feat(restart): browser.restart should return a promise (#4008)

Also allows `browser.restart` to work when the control flow is disabled, and
fixes it for forked browsers.

Closes #3899 and
#3896

Author: sjelin

Diff for: lib/browser.ts

@@ -317,11 +317,16 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver {
   params: any;
 
   /**
+   * Resolved when the browser is ready for use.  Resolves to the browser, so
+   * you can do:
+   *
+   *   forkedBrowser = await browser.forkNewDriverInstance().ready;
+   *
    * Set by the runner.
    *
-   * @type {q.Promise} Done when the new browser is ready for use
+   * @type {webdriver.promise.Promise.<ProtractorBrowser>}
    */
-  ready: wdpromise.Promise<any>;
+  ready: wdpromise.Promise<ProtractorBrowser>;
 
   /*
    * Set by the runner.
@@ -499,7 +504,15 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver {
   /**
    * Fork another instance of browser for use in interactive tests.
    *
-   * Set by the runner.
+   * @example
+   * // Running with control flow enabled
+   * var fork = browser.forkNewDriverInstance();
+   * fork.get('page1'); // 'page1' gotten by forked browser
+   *
+   * @example
+   * // Running with control flow disabled
+   * var forked = await browser.forkNewDriverInstance().ready;
+   * await forked.get('page1'); // 'page1' gotten by forked browser
    *
    * @param {boolean} opt_useSameUrl Whether to navigate to current url on
    * creation
@@ -513,11 +526,85 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver {
   }
 
   /**
-   * Restart the browser instance.
+   * Restart the browser.  This is done by closing this browser instance and creating a new one.
+   * A promise resolving to the new instance is returned, and if this function was called on the
+   * global `browser` instance then Protractor will automatically overwrite the global `browser`
+   * variable.
+   *
+   * When restarting a forked browser, it is the caller's job to overwrite references to the old
+   * instance.
+   *
+   * This function behaves slightly differently depending on if the webdriver control flow is
+   * enabled.  If the control flow is enabled, the global `browser` object is synchronously
+   * replaced. If the control flow is disabled, the global `browser` is replaced asynchronously
+   * after the old driver quits.
    *
    * Set by the runner.
+   *
+   * @example
+   * // Running against global browser, with control flow enabled
+   * browser.get('page1');
+   * browser.restart();
+   * browser.get('page2'); // 'page2' gotten by restarted browser
+   *
+   * @example
+   * // Running against global browser, with control flow disabled
+   * await browser.get('page1');
+   * await browser.restart();
+   * await browser.get('page2'); // 'page2' gotten by restarted browser
+   *
+   * @example
+   * // Running against forked browsers, with the control flow enabled
+   * // In this case, you may prefer `restartSync` (documented below)
+   * var forked = browser.forkNewDriverInstance();
+   * fork.get('page1');
+   * fork.restart().then(function(fork) {
+   *   fork.get('page2'); // 'page2' gotten by restarted fork
+   * });
+   *
+   * @example
+   * // Running against forked browsers, with the control flow disabled
+   * var forked = await browser.forkNewDriverInstance().ready;
+   * await fork.get('page1');
+   * fork = await fork.restart();
+   * await fork.get('page2'); // 'page2' gotten by restarted fork
+   *
+   * @example
+   * // Unexpected behavior can occur if you save references to the global `browser`
+   * var savedBrowser = browser;
+   * browser.get('foo').then(function() {
+   *   console.log(browser === savedBrowser); // false
+   * });
+   * browser.restart();
+   *
+   * @returns {webdriver.promise.Promise<ProtractorBrowser>} A promise resolving to the restarted
+   *   browser
    */
-  restart() {
+  restart(): wdpromise.Promise<ProtractorBrowser> {
+    return;
+  }
+
+  /**
+   * Like `restart`, but instead of returning a promise resolving to the new browser instance,
+   * returns the new browser instance directly.  Can only be used when the control flow is enabled.
+   *
+   * @example
+   * // Running against global browser
+   * browser.get('page1');
+   * browser.restartSync();
+   * browser.get('page2'); // 'page2' gotten by restarted browser
+   *
+   * @example
+   * // Running against forked browsers
+   * var forked = browser.forkNewDriverInstance();
+   * fork.get('page1');
+   * fork = fork.restartSync();
+   * fork.get('page2'); // 'page2' gotten by restarted fork
+   *
+   * @throws {TypeError} Will throw an error if the control flow is not enabled 
+   * @returns {ProtractorBrowser} The restarted browser
+   */
+  restartSync(): ProtractorBrowser {
     return;
   }
 
@@ -1138,4 +1225,18 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver {
     };
     this.debugHelper.init(debuggerClientPath, onStartFn, opt_debugPort);
   }
+
+  /**
+   * Determine if the control flow is enabled.
+   *
+   * @returns true if the control flow is enabled, false otherwise.
+   */
+  controlFlowIsEnabled() {
+    if ((wdpromise as any).USE_PROMISE_MANAGER !== undefined) {
+      return (wdpromise as any).USE_PROMISE_MANAGER;
+    } else {
+      // True for old versions of `selenium-webdriver`, probably false in >=5.0.0
+      return !!wdpromise.ControlFlow;
+    }
+  }
 }

Diff for: lib/runner.ts

@@ -115,8 +115,7 @@ export class Runner extends EventEmitter {
     let ret: q.Promise<void>;
     this.frameworkUsesAfterEach = true;
     if (this.config_.restartBrowserBetweenTests) {
-      // TODO(sjelin): remove the `|| q()` once `restart()` returns a promise
-      this.restartPromise = this.restartPromise || protractor.browser.restart() || q();
+      this.restartPromise = this.restartPromise || q(protractor.browser.restart());
       ret = this.restartPromise;
       this.restartPromise = undefined;
     }
@@ -249,7 +248,8 @@ export class Runner extends EventEmitter {
       browser_.ng12Hybrid = config.ng12Hybrid;
     }
 
-    browser_.ready = driver.manage().timeouts().setScriptTimeout(config.allScriptsTimeout);
+    browser_.ready =
+        driver.manage().timeouts().setScriptTimeout(config.allScriptsTimeout).then(() => browser_);
 
     browser_.getProcessedConfig = () => {
       return wdpromise.fulfilled(config);
@@ -268,12 +268,35 @@ export class Runner extends EventEmitter {
       return newBrowser;
     };
 
+    let replaceBrowser = () => {
+      let newBrowser = browser_.forkNewDriverInstance(false, true);
+      if (browser_ === protractor.browser) {
+        this.setupGlobals_(newBrowser);
+      }
+      return newBrowser;
+    };
+
     browser_.restart = () => {
       // Note: because tests are not paused at this point, any async
       // calls here are not guaranteed to complete before the tests resume.
+
+      // Seperate solutions depending on if the control flow is enabled (see lib/browser.ts)
+      if (browser_.controlFlowIsEnabled()) {
+        return browser_.restartSync().ready;
+      } else {
+        return this.driverprovider_.quitDriver(browser_.driver)
+            .then(replaceBrowser)
+            .then(newBrowser => newBrowser.ready);
+      }
+    };
+
+    browser_.restartSync = () => {
+      if (!browser_.controlFlowIsEnabled()) {
+        throw TypeError('Unable to use `browser.restartSync()` when the control flow is disabled');
+      }
+
       this.driverprovider_.quitDriver(browser_.driver);
-      browser_ = browser_.forkNewDriverInstance(false, true);
-      this.setupGlobals_(browser_);
+      return replaceBrowser();
     };
 
     return browser_;
@@ -365,8 +388,7 @@ export class Runner extends EventEmitter {
             // TODO(sjelin): replace with warnings once `afterEach` support is required
             let restartDriver = () => {
               if (!this.frameworkUsesAfterEach) {
-                // TODO(sjelin): remove the `|| q()` once `restart()` returns a promise
-                this.restartPromise = browser_.restart() || q();
+                this.restartPromise = q(browser_.restart());
               }
             };
             this.on('testPass', restartDriver);