Refactor bootstrapping process and browser.waitForAngular to make them interruptible

[sjelin]

In the bootstrapping process (everything after navigation in browser.get) and browser.waitForAngular we send a few separate commands to selenium-webdriver. Browser initiated navigation (e.g. clicking a link) may cause the first few commands to be executed on one page, and the remaining commands to be executed on a different page. This could end up with us trying to bootstrap or synchronize with entirely the wrong page.

What's more, in #3857 we want to start running bootstrap checks periodically. With the control flow disabled, this could easily lead of race conditions.

Luckily, this is easy enough to solve. We can stick some metadata on the window object at the start of bootstrapping/waitForAngular, and if that metadata goes away, we know navigation occurred. If navigation occurs, we silently give up on the remainder of the bootstrapping/waitForAngular.

Proposed Implementation

We should write a helper function safeExecuteScriptFactory:

interface SafeExecuteScript extends Function {
  (script: string|Function, description: string, ...scriptArgs: any[]): wdpromise.Promise<any>;
  context: string;
  id: string;
  deleteMetadata: () => wdpromise.Promise<void>;
}

class InterruptionError extends Error {
  constructor(public context: string, public id: string) {
    super(`Navigation occurred during ${context} (call id: ${id})`);
  }
}

private safeExecuteScriptFactory(context: string): wdpromise.Promise<SafeExecuteScript> {
  const id = uuid.v4();

  const safeExec = function(script: string|Function, description: string, ...scriptArgs: any[]) {
    // Wrap script
    if (typeof script === 'function') {
      script = `return (${script}).apply(null, arguments);`;
    }
    script = clientSideScripts.wrapScriptForSafeCall(script, id);

    // Run wrapped script & unwrap return value
    scriptArgs.unshift(script, description, id);
    return this.executeScriptWithDescription.apply(this, scriptArgs).then((result: any) => {
      if (result.value) {
        return result.value;
      } else {
        throw new InterruptionError(context, id);
      }
    });
  } as SafeExecuteScript;

  safeExec.bind(this);
  safeExec.context = context;
  safeExec.id = id;
  safeExec.deleteMetadata = () => {
    return this.executeScriptWithDescription(clientSideScripts.deleteSafeCallMetadata,
      `Deleting metadata for ${context} (call id: ${id})`, id);
  };

  return this.executeScriptWithDescription(clientSideScripts.initSafeCallMetadata,
    `Initializing metadata for ${context} (call id: ${id})`, id).then(() => safeExec);
}

With the following additions to clientsidescripts.js:

functions.initSafeCallMetadata = function(safeCallUUID) {
  window['__PROTRACTOR_METADATA_' + safeCallUUID] = {};
}

functions.deleteSafeCallMetadata = function(safeCallUUID) {
  delete window['__PROTRACTOR_METADATA_' + safeCallUUID];
}

exports.wrapScriptForSafeCall = function(script, safeCallUUID) {
  var unwrappedFn = 'function unwrappedFn() {' + script + '}';
  var getCallMetadata = 'function getCallMetadata() {' +
      'return window["__PROTRACTOR_METADATA_' + safeCallUUID + '"];}'
  return wrapWithHelpers(function() {
      if (getCallMetadata()) {
        return { value: unwrappedFn.apply(this, arguments) };
      } else {
        return { error: 'No metadata for ' + id };
      }
  }, unwrappedFn, getCallMetadata);
}

Then, in at the start of bootstrapping in browser.get (code link in v5.1) we do:

        .then(() => {
          return this.safeExecuteScriptFactory(msg('bootstrapping'));
        })
        .then((safeExec) => {
          safeExecuteScriptWithDescription = safeExec; // declared at the top of the file
        })
          ... // Proceed as before but use `safeExecuteScriptWithDescription`
          ... // instead of `this.executeScriptWithDescription`
          ...

        // at the end of the promise chain, delete the metadata
        .then(() => {
          return safeExecuteScriptWithDescription.deleteMetadata();
        }, (error) => {
          return safeExecuteScriptWithDescription.deleteMetadata().then(() => {
            if (!(error instanceOf InterruptionError)) {
              throw error;
            }
          }, (newError) => {
            throw (error instanceOf InterruptionError) ? newError : error;
          });
        });       

And then we do essentially the same thing in waitForAngular as well. This way, if some kind of navigation happens, we just stop.

Edge Case: browser.executeAsyncScript_

We can't use safeExecuteScriptWithDescription in place of browser.executeAsyncScript_ because safeExecuteScriptWithDescription doesn't take asynchronous scripts. However, as per #4053, we probably want to get rid of browser.executeAsyncScript_ anyway.

Edge Case: Blocking Proxy

Blocking Proxy has its own GitHub issue (angular/blocking-proxy#32) about sending clear error messages when navigation interrupts stabilization. We will simply catch this error to detect interruptions.

We could have a race condition in waitForAngular where navigation could occur in between plugin stability checks and blocking proxy checks. However, waitForAngular is only called at the start of commands like el.click(), and so well written tests shouldn't run into this problem.

Edge Case: Plugins

The plugin functions onPageLoad, onPageStable, onPageLoad and onPageStable also need to be interruptible. This is mostly going to be up to the plugin authors, but we can help them out by providing them with safeExecuteScriptWithDescription.

[sjelin]

Note: when we implement #3857, we'll want to change from "give up if navigation occurs" to "start from the beginning if navigation occurs". This will also make more sense because bootstrapping will be done at the top of every waitForAngular call.