Bug 1518777 - ensure that netmonitor and console tests wait for initialization and open requests tofinish before shutting down; r=nchevobbe
authoryulia <ystartsev@mozilla.com>
Thu, 24 Jan 2019 11:47:39 +0000
changeset 515277 2bc02c7054fee3b0ac24cda88bff6704b209b858
parent 515276 a6cc9b15b1e3f464589488b0f490eadd9bb20f74
child 515278 9c646afb94b2e8b64433cf5ddfb314fee6190f20
push id1953
push userffxbld-merge
push dateMon, 11 Mar 2019 12:10:20 +0000
treeherdermozilla-release@9c35dcbaa899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnchevobbe
bugs1518777
milestone66.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1518777 - ensure that netmonitor and console tests wait for initialization and open requests tofinish before shutting down; r=nchevobbe Several of our tests relied on the timing of the shutdown of the webconsole. I updated one test that fired off an extra rdp request so that it is waiting until it resolves before shutting down. For the rest, I started tracking the async behavior of initialization of the web-console-proxy, and waiting for initialization to finish before destroying. Differential Revision: https://phabricator.services.mozilla.com/D16177
devtools/client/framework/target.js
devtools/client/netmonitor/test/browser_net_columns_reset.js
devtools/client/shared/test/browser_dbg_target-scoped-actor-02.js
devtools/client/webconsole/webconsole-connection-proxy.js
devtools/client/webconsole/webconsole-frame.js
--- a/devtools/client/framework/target.js
+++ b/devtools/client/framework/target.js
@@ -826,32 +826,36 @@ class Target extends EventEmitter {
 
   /**
    * Log an error of some kind to the tab's console.
    *
    * @param {String} text
    *                 The text to log.
    * @param {String} category
    *                 The category of the message.  @see nsIScriptError.
+   * @returns {Promise}
    */
   logErrorInPage(text, category) {
     if (this.activeTab && this.activeTab.traits.logInPage) {
       const errorFlag = 0;
-      this.activeTab.logInPage({ text, category, flags: errorFlag });
+      return this.activeTab.logInPage({ text, category, flags: errorFlag });
     }
+    return Promise.resolve();
   }
 
   /**
    * Log a warning of some kind to the tab's console.
    *
    * @param {String} text
    *                 The text to log.
    * @param {String} category
    *                 The category of the message.  @see nsIScriptError.
+   * @returns {Promise}
    */
   logWarningInPage(text, category) {
     if (this.activeTab && this.activeTab.traits.logInPage) {
       const warningFlag = 1;
-      this.activeTab.logInPage({ text, category, flags: warningFlag });
+      return this.activeTab.logInPage({ text, category, flags: warningFlag });
     }
+    return Promise.resolve();
   }
 }
 exports.Target = Target;
--- a/devtools/client/netmonitor/test/browser_net_columns_reset.js
+++ b/devtools/client/netmonitor/test/browser_net_columns_reset.js
@@ -13,16 +13,62 @@ add_task(async function() {
   const { document, parent, windowRequire } = monitor.panelWin;
   const { Prefs } = windowRequire("devtools/client/netmonitor/src/utils/prefs");
 
   const prefBefore = Prefs.visibleColumns;
 
   await hideColumn(monitor, "status");
   await hideColumn(monitor, "waterfall");
 
+  const onRequestsFinished = waitForRequestsFinished(monitor);
   EventUtils.sendMouseEvent({ type: "contextmenu" },
     document.querySelector("#requests-list-contentSize-button"));
 
   parent.document.querySelector("#request-list-header-reset-columns").click();
+  await onRequestsFinished;
 
   ok(JSON.stringify(prefBefore) === JSON.stringify(Prefs.visibleColumns),
      "Reset columns item should reset columns pref");
 });
+
+/**
+ * Helper function for waiting for all events to fire before resolving a promise.
+ *
+ * @param monitor subject
+ *        The event emitter object that is being listened to.
+ * @return {Promise}
+ *        Returns a promise that resolves upon the last event being fired.
+ */
+function waitForRequestsFinished(monitor, event) {
+  const window = monitor.panelWin;
+
+  return new Promise(resolve => {
+    // Key is the request id, value is a boolean - is request finished or not?
+    const requests = new Map();
+
+    function onRequest(id) {
+      info(`Request ${id} not yet done, keep waiting...`);
+      requests.set(id, false);
+    }
+
+    function onEventRequest(id) {
+      info(`Request ${id} `);
+      requests.set(id, true);
+      maybeResolve();
+    }
+
+    function maybeResolve() {
+      // Have all the requests in the map finished yet?
+      if ([...requests.values()].some(finished => !finished)) {
+        return;
+      }
+
+      // All requests are done - unsubscribe from events and resolve!
+      window.api.off(EVENTS.NETWORK_EVENT, onRequest);
+      window.api.off(EVENTS.RECEIVED_EVENT_TIMINGS, onEventRequest);
+      info("All requests finished");
+      resolve();
+    }
+
+    window.api.on(EVENTS.NETWORK_EVENT, onRequest);
+    window.api.on(EVENTS.RECEIVED_EVENT_TIMINGS, onEventRequest);
+  });
+}
--- a/devtools/client/shared/test/browser_dbg_target-scoped-actor-02.js
+++ b/devtools/client/shared/test/browser_dbg_target-scoped-actor-02.js
@@ -39,15 +39,18 @@ async function testTargetScopedActor(cli
     "testOneActor's actorPrefix should be used.");
 
   const response = await client.request({ to: form.testOneActor, type: "ping" });
   is(response.pong, "pong",
      "Actor should respond to requests.");
 }
 
 async function closeTab(client, form) {
-  await removeTab(gBrowser.selectedTab);
-  await Assert.rejects(
+  // We need to start listening for the rejection before removing the tab
+  /* eslint-disable-next-line mozilla/rejects-requires-await*/
+  const onReject = Assert.rejects(
     client.request({ to: form.testOneActor, type: "ping" }),
     err => err.message === `'ping' active request packet to '${form.testOneActor}' ` +
                            `can't be sent as the connection just closed.`,
     "testOneActor went away.");
+  await removeTab(gBrowser.selectedTab);
+  await onReject;
 }
--- a/devtools/client/webconsole/webconsole-connection-proxy.js
+++ b/devtools/client/webconsole/webconsole-connection-proxy.js
@@ -74,16 +74,22 @@ WebConsoleConnectionProxy.prototype = {
 
   /**
    * Tells if the connection is established.
    * @type boolean
    */
   connected: false,
 
   /**
+   * Tells if the console is attached.
+   * @type boolean
+   */
+  isAttached: null,
+
+  /**
    * Timer used for the connection.
    * @private
    * @type object
    */
   _connectTimer: null,
 
   _connectDefer: null,
   _disconnecter: null,
@@ -123,17 +129,17 @@ WebConsoleConnectionProxy.prototype = {
                        this._onLastPrivateContextExited);
 
     this.target.on("will-navigate", this._onTabWillNavigate);
     this.target.on("navigate", this._onTabNavigated);
 
     if (this.target.isBrowsingContext) {
       this.webConsoleFrame.onLocationChange(this.target.url, this.target.title);
     }
-    this._attachConsole();
+    this.isAttached = this._attachConsole();
 
     return connPromise;
   },
 
   /**
    * Connection timeout handler.
    * @private
    */
@@ -144,25 +150,26 @@ WebConsoleConnectionProxy.prototype = {
     };
 
     this._connectDefer.reject(error);
   },
 
   /**
    * Attach to the Web Console actor.
    * @private
+   * @returns Promise
    */
   _attachConsole: function() {
     const listeners = ["PageError", "ConsoleAPI", "NetworkActivity"];
     // Enable the forwarding of console messages to the parent process
     // when we open the Browser Console or Toolbox.
     if (this.target.chrome && !this.target.isAddon) {
       listeners.push("ContentProcessMessages");
     }
-    this.webConsoleClient.startListeners(listeners)
+    return this.webConsoleClient.startListeners(listeners)
       .then(this._onAttachConsole, error => {
         console.error("attachConsole failed: " + error);
         this._connectDefer.reject(error);
       });
   },
 
   /**
    * The "attachConsole" response handler.
@@ -223,17 +230,17 @@ WebConsoleConnectionProxy.prototype = {
 
   /**
    * The "cachedMessages" response handler.
    *
    * @private
    * @param object response
    *        The JSON response object received from the server.
    */
-  _onCachedMessages: function(response) {
+  _onCachedMessages: async function(response) {
     if (response.error) {
       console.error("Web Console getCachedMessages error: " + response.error +
                     " " + response.message);
       this._connectDefer.reject(response);
       return;
     }
 
     if (!this._connectTimer) {
@@ -243,17 +250,17 @@ WebConsoleConnectionProxy.prototype = {
     }
 
     const messages =
       response.messages.concat(...this.webConsoleClient.getNetworkEvents());
     messages.sort((a, b) => a.timeStamp - b.timeStamp);
 
     this.dispatchMessagesAdd(messages);
     if (!this.webConsoleClient.hasNativeConsoleAPI) {
-      this.webConsoleFrame.logWarningAboutReplacedAPI();
+      await this.webConsoleFrame.logWarningAboutReplacedAPI();
     }
 
     this.connected = true;
     this._connectDefer.resolve(this);
   },
 
   /**
    * The "pageError" message type handler. We redirect any page errors to the UI
@@ -390,23 +397,27 @@ WebConsoleConnectionProxy.prototype = {
   },
 
   /**
    * Disconnect the Web Console from the remote server.
    *
    * @return object
    *         A promise object that is resolved when disconnect completes.
    */
-  disconnect: function() {
+  disconnect: async function() {
     if (this._disconnecter) {
       return this._disconnecter.promise;
     }
 
     this._disconnecter = defer();
 
+    // allow the console to finish attaching if it started.
+    if (this.isAttached) {
+      await this.isAttached;
+    }
     if (!this.client) {
       this._disconnecter.resolve(null);
       return this._disconnecter.promise;
     }
 
     this.client.removeListener("logMessage", this._onLogMessage);
     this.client.removeListener("pageError", this._onPageError);
     this.client.removeListener("consoleAPICall", this._onConsoleAPICall);
--- a/devtools/client/webconsole/webconsole-frame.js
+++ b/devtools/client/webconsole/webconsole-frame.js
@@ -149,17 +149,17 @@ WebConsoleFrame.prototype = {
     return this.consoleOutput;
   },
 
   _onUpdateListeners() {
 
   },
 
   logWarningAboutReplacedAPI() {
-    this.owner.target.logWarningInPage(l10n.getStr("ConsoleAPIDisabled"),
+    return this.owner.target.logWarningInPage(l10n.getStr("ConsoleAPIDisabled"),
       "ConsoleAPIDisabled");
   },
 
   handleNetworkEventUpdate() {
 
   },
 
   /**