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

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

Closes angular#3899 and
angular#3896

Author: sjelin

lib/browser.ts

@@ -76,10 +76,12 @@ export class AbstractExtendedWebDriver extends AbstractWebDriver {
  * `ElementFinder` will be unwrapped to their underlying `WebElement` instance
  *
  * @private
- * @param {Object} to
- * @param {Object} from
- * @param {string} fnName
- * @param {function=} setupFn
+ * @param {Object} to The object to copy a function to
+ * @param {string|Object} from Where to get the function from.  If an object, the object containing
+ *   the function to copy over. If a string, the name of a property on `to` which is an object
+ *   containing the function.
+ * @param {string} fnName The name of the function to copy
+ * @param {function=} setupFn An optional function to run before the copied function
  */
 function ptorMixin(to: any, from: any, fnName: string, setupFn?: Function) {
   to[fnName] = function() {
@@ -91,7 +93,8 @@ function ptorMixin(to: any, from: any, fnName: string, setupFn?: Function) {
     if (setupFn) {
       setupFn();
     }
-    return from[fnName].apply(from, arguments);
+    let fromObj = typeof from === 'string' ? to[from] : from;
+    return fromObj[fnName].apply(fromObj, arguments);
   };
 };
 
@@ -236,9 +239,11 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver {
   params: any;
 
   /**
-   * Set by the runner.
+   * Resolved when the browser is ready for use
+   *
+   * Set partially by `setDriver_()`, partially by the runner.
    *
-   * @type {q.Promise} Done when the new browser is ready for use
+   * @type {webdriver.promise.Promise}
    */
   ready: wdpromise.Promise<any>;
 
@@ -306,31 +311,24 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver {
       webdriverInstance: WebDriver, opt_baseUrl?: string, opt_rootElement?: string,
       opt_untrackOutstandingTimeouts?: boolean, opt_blockingProxyUrl?: string) {
     super();
+    this.setDriver_(webdriverInstance);
+
     // These functions should delegate to the webdriver instance, but should
     // wait for Angular to sync up before performing the action. This does not
     // include functions which are overridden by protractor below.
     let methodsToSync = ['getCurrentUrl', 'getPageSource', 'getTitle'];
-    let extendWDInstance: ExtendedWebDriver;
-    try {
-      extendWDInstance = extendWD(webdriverInstance);
-    } catch (e) {
-      // Probably not a driver that can be extended (e.g. gotten using
-      // `directConnect: true` in the config)
-      extendWDInstance = webdriverInstance as ExtendedWebDriver;
-    }
 
     // Mix all other driver functionality into Protractor.
     Object.getOwnPropertyNames(WebDriver.prototype).forEach(method => {
-      if (!this[method] && typeof(extendWDInstance as any)[method] === 'function') {
+      if (!this[method] && typeof(this.driver as any)[method] === 'function') {
         if (methodsToSync.indexOf(method) !== -1) {
-          ptorMixin(this, extendWDInstance, method, this.waitForAngular.bind(this));
+          ptorMixin(this, 'driver', method, this.waitForAngular.bind(this));
         } else {
-          ptorMixin(this, extendWDInstance, method);
+          ptorMixin(this, 'driver', method);
         }
       }
     });
 
-    this.driver = extendWDInstance;
     if (opt_blockingProxyUrl) {
       logger.info('Starting BP client for ' + opt_blockingProxyUrl);
       this.bpClient = new BPClient(opt_blockingProxyUrl);
@@ -343,7 +341,6 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver {
     this.ignoreSynchronization = false;
     this.getPageTimeout = DEFAULT_GET_PAGE_TIMEOUT;
     this.params = {};
-    this.ready = null;
     this.plugins_ = new Plugins({});
     this.resetUrl = DEFAULT_RESET_URL;
     this.debugHelper = new DebugHelper(this);
@@ -365,7 +362,33 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver {
         ng12Hybrid_ = ng12Hybrid;
       }
     });
-    this.driver.getCapabilities().then((caps: Capabilities) => {
+    this.trackOutstandingTimeouts_ = !opt_untrackOutstandingTimeouts;
+    this.mockModules_ = [];
+    this.addBaseMockModules_();
+
+    // set up expected conditions
+    this.ExpectedConditions = new ProtractorExpectedConditions(this);
+  }
+
+  /**
+   * Sets the `WebDriver` instance which `ProtractorBrowser` wraps around
+   *
+   * @param {WebDriver} webdriverInstance The WebDriver instance to wrap around
+   */
+  setDriver_(webdriverInstance: WebDriver) {
+    // First extend the driver
+    let extendWDInstance: ExtendedWebDriver;
+    try {
+      extendWDInstance = extendWD(webdriverInstance);
+    } catch (e) {
+      // Probably not a driver that can be extended (e.g. gotten using
+      // `directConnect: true` in the config)
+      extendWDInstance = webdriverInstance as ExtendedWebDriver;
+    }
+
+    this.driver = extendWDInstance;
+
+    this.ready = this.driver.getCapabilities().then((caps) => {
       // Internet Explorer does not accept data URLs, which are the default
       // reset URL for Protractor.
       // Safari accepts data urls, but SafariDriver fails after one is used.
@@ -376,13 +399,6 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver {
         this.resetUrl = 'about:blank';
       }
     });
-
-    this.trackOutstandingTimeouts_ = !opt_untrackOutstandingTimeouts;
-    this.mockModules_ = [];
-    this.addBaseMockModules_();
-
-    // set up expected conditions
-    this.ExpectedConditions = new ProtractorExpectedConditions(this);
   }
 
   /**
@@ -434,7 +450,17 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver {
   /**
    * Restart the browser instance.
    *
+   * Replaces a browser's `.driver` property, so if you've saved a reference directly to
+   * `browser.driver`, make sure to update it.
+   *
+   * Behaves slightly differently depending on if the webdriver control flow is enabled.  If the
+   * control flow is enabled, `browser.driver` is synchronously replaced.  If the control flow is
+   * disabled, `browser.driver` is replaced asynchronously after the old driver quits.
+   *
    * Set by the runner.
+   *
+   * @returns {webdriver.promise.Promise.<void>} A promise which resolves once the browser has
+   *   restarted.
    */
   restart() {
     return;

lib/runner.ts

@@ -10,6 +10,7 @@ import {Logger} from './logger';
 import {Plugins} from './plugins';
 import {protractor} from './ptor';
 import * as helper from './util';
+import * as wdpromiseUtil from './wdpromiseUtil';
 
 declare let global: any;
 declare let process: any;
@@ -234,7 +235,9 @@ export class Runner extends EventEmitter {
       browser_.ng12Hybrid = config.ng12Hybrid;
     }
 
-    browser_.ready = driver.manage().timeouts().setScriptTimeout(config.allScriptsTimeout);
+    browser_.ready = browser_.ready.then(() => {
+      return driver.manage().timeouts().setScriptTimeout(config.allScriptsTimeout);
+    });
 
     browser_.getProcessedConfig = () => {
       return wdpromise.fulfilled(config);
@@ -256,9 +259,19 @@ export class Runner extends EventEmitter {
     browser_.restart = () => {
       // Note: because tests are not paused at this point, any async
       // calls here are not guaranteed to complete before the tests resume.
-      this.driverprovider_.quitDriver(browser_.driver);
-      browser_ = browser_.forkNewDriverInstance(false, true);
-      this.setupGlobals_(browser_);
+
+      // Seperate solutions for if the control flow is on or off, see
+      // https://github.com/angular/protractor/issues/3899
+      if (wdpromiseUtil.usePromiseManager()) {
+        this.driverprovider_.quitDriver(browser_.driver);
+        browser_.setDriver_(this.driverprovider_.getNewDriver());
+        return browser_.ready;
+      } else {
+        return this.driverprovider_.quitDriver(browser_.driver).then(() => {
+          browser_.setDriver_(this.driverprovider_.getNewDriver());
+          return browser_.ready;
+        });
+      }
     };
 
     return browser_;

lib/wdpromiseUtil.ts

@@ -0,0 +1,9 @@
+import {promise as wdpromise} from 'selenium-webdriver';
+
+export function usePromiseManager() {
+  if ((wdpromise as any).USE_PROMISE_MANAGER !== undefined) {
+    return (wdpromise as any).USE_PROMISE_MANAGER;
+  } else {
+    return !!wdpromise.ControlFlow;
+  }
+}