improve behavior of Run All Chunks in notebooks

This change improves the behavior of Run All Chunks in notebook mode,
especially when the setup chunk is run automatically. It does the
following:

1. Avoid calling prepare_for_rmd_chunk_execution once per chunk;
   instead, call it only when the first chunk enters the execution
   queue. Waiting for this RPC to return per chunk caused a lot of
   nondeterministic behavior, since e.g. the first chunk might an
   autosave of the doc first, but the second might not, so it entered
   the execution queue ahead of the first.

2. Sort chunks that enter the execution queue as they arrive. This
   ensures that chunks which arrive out of order will still be executed
   in document order (this includes interactively executed chunks)

3. Improve robustness checks to ensure that the currently executing
   chunk doesn't get re-queued until it's done executing, and the setup
   chunk is marked executed even if triggered manually.

src/gwt/src/org/rstudio/studio/client/workbench/views/source/editors/text/TextEditingTarget.java

@@ -4228,7 +4228,7 @@ public void execute()
  };
 
  // Rmd allows server-side prep for chunk execution
- if (fileType_.isRmd())
+ if (fileType_.isRmd() && !docDisplay_.showChunkOutputInline())
  {
  // ensure source is synced with server
  docUpdateSentinel_.withSavedDoc(new Command() {

src/gwt/src/org/rstudio/studio/client/workbench/views/source/editors/text/rmd/TextEditingTargetNotebook.java

@@ -18,8 +18,9 @@
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.LinkedList;
-import java.util.Queue;
 
+import org.rstudio.core.client.CommandWithArg;
+import org.rstudio.core.client.Debug;
 import org.rstudio.core.client.JsArrayUtil;
 import org.rstudio.core.client.StringUtil;
 import org.rstudio.core.client.command.CommandBinder;
@@ -105,11 +106,12 @@ public interface Binder
  private class ChunkExecQueueUnit
  {
  public ChunkExecQueueUnit(String chunkIdIn, String codeIn, 
- String optionsIn, String setupCrc32In)
+ String optionsIn, int rowIn, String setupCrc32In)
  {
  chunkId = chunkIdIn;
  options = optionsIn;
  code = codeIn;
+ row = rowIn;
  setupCrc32 = setupCrc32In;
  pos = 0;
  linesExecuted = 0;
@@ -119,6 +121,7 @@ public ChunkExecQueueUnit(String chunkIdIn, String codeIn,
  public String code;
  public String setupCrc32;
  public int pos;
+ public int row;
  public int linesExecuted;
  };
 
@@ -309,7 +312,7 @@ public void executeChunk(Scope chunk, String code, String options)
  String setupCrc32 = "";
  if (isSetupChunkScope(chunk))
  {
- setupCrc32 = setupCrc32_;
+ setupCrc32 = getChunkCrc32(chunk);
  chunkId = SETUP_CHUNK_ID;
  }
  else
@@ -322,12 +325,6 @@ public void executeChunk(Scope chunk, String code, String options)
  ensureSetupChunkExecuted();
  }
 
- // decorate the gutter to show the chunk is queued
- docDisplay_.setChunkLineExecState(chunk.getBodyStart().getRow() + 1,
- chunk.getEnd().getRow(), ChunkRowExecState.LINE_QUEUED);
- chunks_.setChunkState(chunk.getPreamble().getRow(), 
- ChunkContextToolbar.STATE_QUEUED);
-
  // check to see if this chunk is already in the execution queue--if so
  // just update the code and leave it queued
  for (ChunkExecQueueUnit unit: chunkExecQueue_)
@@ -337,16 +334,89 @@ public void executeChunk(Scope chunk, String code, String options)
  unit.code = code;
  unit.options = options;
  unit.setupCrc32 = setupCrc32;
+ unit.row = row;
  return;
  }
  }
 
+ // if this is the currently executing chunk, don't queue it again
+ if (executingChunk_ != null && executingChunk_.chunkId == chunkId)
+ return;
+
+ // decorate the gutter to show the chunk is queued
+ docDisplay_.setChunkLineExecState(chunk.getBodyStart().getRow() + 1,
+ chunk.getEnd().getRow(), ChunkRowExecState.LINE_QUEUED);
+ chunks_.setChunkState(chunk.getPreamble().getRow(), 
+ ChunkContextToolbar.STATE_QUEUED);
+
+ // find the appropriate place in the queue for this chunk
+ int idx = chunkExecQueue_.size();
+ for (int i = 0; i < chunkExecQueue_.size(); i++)
+ {
+ if (chunkExecQueue_.get(i).row > row)
+ {
+ idx = i;
+ break;
+ }
+ }
+
  // put it in the queue 
- chunkExecQueue_.add(new ChunkExecQueueUnit(chunkId, code,
- options, setupCrc32));
+ chunkExecQueue_.add(idx, new ChunkExecQueueUnit(chunkId, code,
+ options, row, setupCrc32));
+
+ // if there are other chunks in the queue, let them run
+ if (chunkExecQueue_.size() > 1)
+ return;
 
- // initiate queue processing
- processChunkExecQueue();
+ // if this is the first chunk in the queue, make sure the doc is saved
+ // and check for param eval before we continue. since we suppress the
+ // execution of additional chunks while waiting for param evaluation, we
+ // need to guarantee that this state gets cleaned up correctly on error
+ evaluatingParams_ = true;
+ final Command completeEval = new Command()
+ {
+ @Override
+ public void execute()
+ {
+ evaluatingParams_ = false;
+ processChunkExecQueue();
+ }
+ };
+
+ docUpdateSentinel_.withSavedDoc(new Command()
+ {
+ @Override
+ public void execute()
+ {
+ server_.prepareForRmdChunkExecution(docUpdateSentinel_.getId(), 
+ new ServerRequestCallback<Void>()
+ {
+ @Override
+ public void onResponseReceived(Void v)
+ {
+ completeEval.execute();
+ }
+
+ @Override
+ public void onError(ServerError error)
+ {
+ // if we couldn't evaluate the parameters, process the queue
+ // anyway
+ Debug.logError(error);
+ completeEval.execute();
+ }
+ });
+ }
+ }, 
+ new CommandWithArg<String>()
+ {
+ @Override
+ public void execute(String arg)
+ {
+ Debug.logWarning(arg);
+ completeEval.execute();
+ }
+ });
  }
 
  public void manageCommands()
@@ -629,6 +699,9 @@ public void onInterruptStatus(InterruptStatusEvent event)
  cleanChunkExecState(unit.chunkId);
  }
  chunkExecQueue_.clear();
+
+ // clear currently executing chunk
+ executingChunk_ = null;
  }
 
  @Override
@@ -665,9 +738,15 @@ public void onLineWidgetRemoved(LineWidget widget)
 
  private void processChunkExecQueue()
  {
+ // do nothing if we're currently executing a chunk or there are no chunks
+ // to execute
  if (chunkExecQueue_.isEmpty() || executingChunk_ != null)
  return;
 
+ // do nothing if we're waiting for params to evaluate
+ if (evaluatingParams_)
+ return;
+
  // begin chunk execution
  final ChunkExecQueueUnit unit = chunkExecQueue_.remove();
  executingChunk_ = unit;
@@ -905,38 +984,63 @@ private void ensureSetupChunkExecuted()
  if (!prefs_.autoRunSetupChunk().getValue())
  return;
 
+ // ignore if setup chunk currently running or already in the execution
+ // queue
+ if (executingChunk_ != null &&
+ executingChunk_.chunkId == SETUP_CHUNK_ID)
+ {
+ return;
+ }
+ for (ChunkExecQueueUnit unit: chunkExecQueue_)
+ {
+ if (unit.chunkId == SETUP_CHUNK_ID)
+ {
+ return;
+ }
+ }
+
  // no reason to do work if we don't need to re-validate the setup chunk
  if (!validateSetupChunk_ && !StringUtil.isNullOrEmpty(setupCrc32_))
  return;
  validateSetupChunk_ = false;
-
+ 
  // find the setup chunk
  JsArray<Scope> scopes = docDisplay_.getScopeTree();
  for (int i = 0; i < scopes.length(); i++)
  {
  if (isSetupChunkScope(scopes.get(i)))
  {
- // extract the body of the chunk
- String setupCode = docDisplay_.getCode(
- scopes.get(i).getBodyStart(),
- Position.create(scopes.get(i).getEnd().getRow(), 0));
-
- // hash the body and prefix with the virtual session ID (so all
- // hashes are automatically invalidated when the session changes)
- String crc32 = session_.getSessionInfo().getSessionId() +
- StringUtil.crc32(setupCode);
-
+ String crc32 = getChunkCrc32(scopes.get(i));
+
  // compare with previously known hash; if it differs, re-run the
  // setup chunk
  if (crc32 != setupCrc32_)
  {
  setupCrc32_ = crc32;
- executeChunk(scopes.get(i), setupCode, "");
+ executeChunk(scopes.get(i), getChunkCode(scopes.get(i)), "");
  }
  }
  }
  }
 
+ private String getChunkCode(Scope chunk)
+ {
+ return docDisplay_.getCode(
+ chunk.getBodyStart(),
+ Position.create(chunk.getEnd().getRow(), 0));
+ }
+
+ private String getChunkCrc32(Scope chunk)
+ {
+ // extract the body of the chunk
+ String code = getChunkCode(chunk);
+
+ // hash the body and prefix with the virtual session ID (so all
+ // hashes are automatically invalidated when the session changes)
+ return session_.getSessionInfo().getSessionId() +
+ StringUtil.crc32(code);
+ }
+
  private static boolean isSetupChunkScope(Scope scope)
  {
  if (!scope.isChunk())
@@ -1029,7 +1133,7 @@ private void setAllExpansionStates(int state)
 
  private JsArray<ChunkDefinition> initialChunkDefs_;
  private HashMap<String, ChunkOutputUi> outputs_;
- private Queue<ChunkExecQueueUnit> chunkExecQueue_;
+ private LinkedList<ChunkExecQueueUnit> chunkExecQueue_;
  private ChunkExecQueueUnit executingChunk_;
 
  private final DocDisplay docDisplay_;
@@ -1060,6 +1164,7 @@ private void setAllExpansionStates(int state)
  private int charWidth_ = 65;
  private int pixelWidth_ = 0;
  private boolean executedSinceActivate_ = false;
+ private boolean evaluatingParams_ = false;
 
  private int state_ = STATE_NONE;
 

src/gwt/src/org/rstudio/studio/client/workbench/views/source/model/DocUpdateSentinel.java

@@ -26,6 +26,7 @@
 import com.google.gwt.user.client.Window.ClosingHandler;
 
 import org.rstudio.core.client.Barrier.Token;
+import org.rstudio.core.client.CommandWithArg;
 import org.rstudio.core.client.Debug;
 import org.rstudio.core.client.TimeBufferedCommand;
 import org.rstudio.core.client.js.JsObject;
@@ -191,6 +192,12 @@ public void onError(String message)
  }
 
  public void withSavedDoc(final Command onSaved)
+ {
+ withSavedDoc(onSaved, null);
+ }
+
+ public void withSavedDoc(final Command onSaved, 
+ final CommandWithArg<String> onError)
  {
  if (changeTracker_.hasChanged())
  {
@@ -199,12 +206,18 @@ public void withSavedDoc(final Command onSaved)
  @Override
  public void onCompleted()
  {
- onSaved.execute(); 
+ if (onSaved != null)
+ onSaved.execute(); 
  }
 
+ @Override public void onError(String message) 
+ {
+ if (onError != null)
+ onError.execute(message);
+ }
+
  @Override public void onProgress(String message) {}
  @Override public void onProgress(String message, Operation onCancel) {}
- @Override public void onError(String message) {}
  @Override public void clearProgress() {}
  });
 
@@ -216,7 +229,6 @@ public void onCompleted()
  onSaved.execute();
  }
  }
-
  private boolean maybeAutoSave()
  {
  if (changeTracker_.hasChanged())