Bug 1395759 - Properly wait on payload ready in tests; r=nchevobbe draft
authorJan Odvarko <odvarko@gmail.com>
Thu, 12 Oct 2017 13:27:22 +0200
changeset 679182 794cc6387ad0ede48cc52140da6f84aeb06a5187
parent 679181 80c3bf2a62adab41a8ec411792daba069ecfd1d8
child 735535 545b14e3d817da6d253ef4e96c8a4d3041031970
push id84146
push userjodvarko@mozilla.com
push dateThu, 12 Oct 2017 11:30:03 +0000
reviewersnchevobbe
bugs1395759
milestone58.0a1
Bug 1395759 - Properly wait on payload ready in tests; r=nchevobbe MozReview-Commit-ID: Mvhwoihk53
devtools/client/netmonitor/src/connector/firefox-data-provider.js
devtools/client/netmonitor/src/constants.js
devtools/client/netmonitor/test/browser_net_copy_headers.js
devtools/client/netmonitor/test/head.js
devtools/client/webconsole/new-console-output/new-console-output-wrapper.js
devtools/client/webconsole/new-console-output/store.js
devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_network_messages_expand.js
--- a/devtools/client/netmonitor/src/connector/firefox-data-provider.js
+++ b/devtools/client/netmonitor/src/connector/firefox-data-provider.js
@@ -112,22 +112,16 @@ class FirefoxDataProvider {
       requestHeadersObj,
       responseHeadersObj,
       postDataObj,
       requestCookiesObj,
       responseCookiesObj
     );
 
     this.pushRequestToQueue(id, payload);
-
-    if (this.actions.updateRequest && this.isRequestPayloadReady(id)) {
-      let payloadFromQueue = this.getRequestFromQueue(id).payload;
-      this.cleanUpQueue(id);
-      await this.actions.updateRequest(id, payloadFromQueue, true);
-    }
   }
 
   async fetchResponseContent(mimeType, responseContent) {
     let payload = {};
     if (mimeType && responseContent && responseContent.content) {
       let { encoding, text } = responseContent.content;
       let response = await this.getLongString(text);
 
@@ -239,18 +233,25 @@ class FirefoxDataProvider {
 
   /**
    * Return true if payload is ready (all data fetched from the backend)
    *
    * @param {string} id request id
    * @return {boolean} return whether a specific networkEvent has been updated completely.
    */
   isRequestPayloadReady(id) {
-    // The payload is ready when there is no RDP request in-progress.
-    return !this.rdpRequestMap.get(id);
+    let record = this.rdpRequestMap.get(id);
+    if (!record) {
+      return false;
+    }
+
+    // The payload is ready when all values in the record are true.
+    // (i.e. all data received).
+    let props = Object.getOwnPropertyNames(record);
+    return props.every(prop => record[prop] === true);
   }
 
   /**
    * Merge upcoming networkEventUpdate payload into existing one.
    *
    * @param {string} id request id
    * @param {object} payload request data payload
    */
@@ -309,16 +310,24 @@ class FirefoxDataProvider {
       isXHR,
       request: {
         method,
         url,
       },
       startedDateTime,
     } = networkInfo;
 
+    // Create tracking record for this request.
+    this.rdpRequestMap.set(actor, {
+      requestHeaders: false,
+      requestCookies: false,
+      eventTimings: false,
+      responseContent: false,
+    });
+
     this.addRequest(actor, {
       cause,
       fromCache,
       fromServiceWorker,
       isXHR,
       method,
       startedDateTime,
       url,
@@ -332,85 +341,93 @@ class FirefoxDataProvider {
    *
    * @param {string} type message type
    * @param {object} packet the message received from the server.
    * @param {object} networkInfo the network request information.
    */
   onNetworkEventUpdate(type, data) {
     let { packet, networkInfo } = data;
     let { actor } = networkInfo;
+    let { updateType } = packet;
 
-    switch (packet.updateType) {
+    switch (updateType) {
       case "requestHeaders":
-        this.requestData(actor, packet.updateType).then(response => {
-          this.onRequestHeaders(response);
+        this.requestData(actor, updateType).then(response => {
+          this.onRequestHeaders(response)
+            .then(() => this.onDataReceived(actor, updateType));
           emit(EVENTS.UPDATING_REQUEST_HEADERS, actor);
         });
         break;
       case "requestCookies":
-        this.requestData(actor, packet.updateType).then(response => {
-          this.onRequestCookies(response);
+        this.requestData(actor, updateType).then(response => {
+          this.onRequestCookies(response)
+            .then(() => this.onDataReceived(actor, updateType));
           emit(EVENTS.UPDATING_REQUEST_COOKIES, actor);
         });
         break;
       case "requestPostData":
-        this.requestData(actor, packet.updateType).then(response => {
-          this.onRequestPostData(response);
+        this.requestData(actor, updateType).then(response => {
+          this.onRequestPostData(response)
+            .then(() => this.onDataReceived(actor, updateType));
           emit(EVENTS.UPDATING_REQUEST_POST_DATA, actor);
         });
         break;
       case "securityInfo":
         this.updateRequest(actor, {
           securityState: networkInfo.securityInfo,
         }).then(() => {
-          this.requestData(actor, packet.updateType).then(response => {
-            this.onSecurityInfo(response);
+          this.requestData(actor, updateType).then(response => {
+            this.onSecurityInfo(response)
+              .then(() => this.onDataReceived(actor, updateType));
             emit(EVENTS.UPDATING_SECURITY_INFO, actor);
           });
         });
         break;
       case "responseHeaders":
-        this.requestData(actor, packet.updateType).then(response => {
-          this.onResponseHeaders(response);
+        this.requestData(actor, updateType).then(response => {
+          this.onResponseHeaders(response)
+            .then(() => this.onDataReceived(actor, updateType));
           emit(EVENTS.UPDATING_RESPONSE_HEADERS, actor);
         });
         break;
       case "responseCookies":
-        this.requestData(actor, packet.updateType).then(response => {
-          this.onResponseCookies(response);
+        this.requestData(actor, updateType).then(response => {
+          this.onResponseCookies(response)
+            .then(() => this.onDataReceived(actor, updateType));
           emit(EVENTS.UPDATING_RESPONSE_COOKIES, actor);
         });
         break;
       case "responseStart":
         this.updateRequest(actor, {
           httpVersion: networkInfo.response.httpVersion,
           remoteAddress: networkInfo.response.remoteAddress,
           remotePort: networkInfo.response.remotePort,
           status: networkInfo.response.status,
           statusText: networkInfo.response.statusText,
           headersSize: networkInfo.response.headersSize
         }).then(() => {
           emit(EVENTS.STARTED_RECEIVING_RESPONSE, actor);
         });
         break;
       case "responseContent":
-        this.requestData(actor, packet.updateType).then(response => {
+        this.requestData(actor, updateType).then(response => {
           this.onResponseContent({
             contentSize: networkInfo.response.bodySize,
             transferredSize: networkInfo.response.transferredSize,
             mimeType: networkInfo.response.content.mimeType
-          }, response);
+          }, response).then(() => this.onDataReceived(actor, updateType));
           emit(EVENTS.UPDATING_RESPONSE_CONTENT, actor);
         });
         break;
       case "eventTimings":
         this.updateRequest(actor, { totalTime: networkInfo.totalTime })
           .then(() => {
-            this.requestData(actor, packet.updateType).then(response => {
-              this.onEventTimings(response);
+            this.requestData(actor, updateType).then(response => {
+              this.onEventTimings(response)
+                .then(() => this.onDataReceived(actor, updateType));
               emit(EVENTS.UPDATING_EVENT_TIMINGS, actor);
             });
           });
         break;
     }
   }
 
   /**
@@ -429,157 +446,169 @@ class FirefoxDataProvider {
    */
   requestData(actor, method) {
     let record = this.rdpRequestMap.get(actor);
 
     // All RDP requests related to the given actor will be collected
     // in the same record.
     if (!record) {
       record = {};
-      this.rdpRequestMap.set(actor, record);
     }
 
     // If data has been already requested return the same promise.
     if (record.method) {
       return record.method;
     }
 
     // Calculate real name of the client getter.
     let realMethodName = "get" + method.charAt(0).toUpperCase() +
       method.slice(1);
 
     // Request data from the backend.
     let promise = new Promise((resolve, reject) => {
       if (typeof this.webConsoleClient[realMethodName] == "function") {
         this.webConsoleClient[realMethodName](actor, response => {
-          // Remove resolved method from the record.
-          delete record[method];
-
-          // If the record is empty (i.e. there are no RDP requests
-          // for this actor in-progress), remove the entire record.
-          // Further data might be requested in the future, but none
-          // is in-progress at this point.
-          let props = Object.getOwnPropertyNames(record);
-          if (!props.length) {
-            this.rdpRequestMap.delete(actor);
-          }
-
           // Resolve incoming HTTP details data-promise.
           resolve(response);
         });
       } else {
         reject(new Error("Error: No such client method!"));
       }
     });
 
     // Store the promise in order to know about RDP requests
     // in progress.
     record[method] = promise;
 
     return promise;
   }
 
   /**
+   * Executed when new data are received from the backend.
+   */
+  async onDataReceived(actor, type) {
+    let record = this.rdpRequestMap.get(actor);
+    if (record) {
+      record[type] = true;
+    }
+
+    if (this.isRequestPayloadReady(actor)) {
+      let payloadFromQueue = this.getRequestFromQueue(actor).payload;
+
+      // Clean up
+      this.cleanUpQueue(actor);
+      this.rdpRequestMap.delete(actor);
+
+      let { updateRequest } = this.actions;
+      if (updateRequest) {
+        await updateRequest(actor, payloadFromQueue, true);
+      }
+
+      emit(EVENTS.PAYLOAD_READY, actor);
+    }
+  }
+
+  /**
    * Handles additional information received for a "requestHeaders" packet.
    *
    * @param {object} response the message received from the server.
    */
   onRequestHeaders(response) {
-    this.updateRequest(response.from, {
+    return this.updateRequest(response.from, {
       requestHeaders: response
     }).then(() => {
       emit(EVENTS.RECEIVED_REQUEST_HEADERS, response.from);
     });
   }
 
   /**
    * Handles additional information received for a "requestCookies" packet.
    *
    * @param {object} response the message received from the server.
    */
   onRequestCookies(response) {
-    this.updateRequest(response.from, {
+    return this.updateRequest(response.from, {
       requestCookies: response
     }).then(() => {
       emit(EVENTS.RECEIVED_REQUEST_COOKIES, response.from);
     });
   }
 
   /**
    * Handles additional information received for a "requestPostData" packet.
    *
    * @param {object} response the message received from the server.
    */
   onRequestPostData(response) {
-    this.updateRequest(response.from, {
+    return this.updateRequest(response.from, {
       requestPostData: response
     }).then(() => {
       emit(EVENTS.RECEIVED_REQUEST_POST_DATA, response.from);
     });
   }
 
   /**
    * Handles additional information received for a "securityInfo" packet.
    *
    * @param {object} response the message received from the server.
    */
   onSecurityInfo(response) {
-    this.updateRequest(response.from, {
+    return this.updateRequest(response.from, {
       securityInfo: response.securityInfo
     }).then(() => {
       emit(EVENTS.RECEIVED_SECURITY_INFO, response.from);
     });
   }
 
   /**
    * Handles additional information received for a "responseHeaders" packet.
    *
    * @param {object} response the message received from the server.
    */
   onResponseHeaders(response) {
-    this.updateRequest(response.from, {
+    return this.updateRequest(response.from, {
       responseHeaders: response
     }).then(() => {
       emit(EVENTS.RECEIVED_RESPONSE_HEADERS, response.from);
     });
   }
 
   /**
    * Handles additional information received for a "responseCookies" packet.
    *
    * @param {object} response the message received from the server.
    */
   onResponseCookies(response) {
-    this.updateRequest(response.from, {
+    return this.updateRequest(response.from, {
       responseCookies: response
     }).then(() => {
       emit(EVENTS.RECEIVED_RESPONSE_COOKIES, response.from);
     });
   }
 
   /**
    * Handles additional information received for a "responseContent" packet.
    *
    * @param {object} data the message received from the server event.
    * @param {object} response the message received from the server.
    */
   onResponseContent(data, response) {
     let payload = Object.assign({ responseContent: response }, data);
-    this.updateRequest(response.from, payload).then(() => {
+    return this.updateRequest(response.from, payload).then(() => {
       emit(EVENTS.RECEIVED_RESPONSE_CONTENT, response.from);
     });
   }
 
   /**
    * Handles additional information received for a "eventTimings" packet.
    *
    * @param {object} response the message received from the server.
    */
   onEventTimings(response) {
-    this.updateRequest(response.from, {
+    return this.updateRequest(response.from, {
       eventTimings: response
     }).then(() => {
       emit(EVENTS.RECEIVED_EVENT_TIMINGS, response.from);
     });
   }
 }
 
 /**
--- a/devtools/client/netmonitor/src/constants.js
+++ b/devtools/client/netmonitor/src/constants.js
@@ -88,16 +88,19 @@ const EVENTS = {
 
   // When response content begins, updates and finishes receiving.
   STARTED_RECEIVING_RESPONSE: "NetMonitor:NetworkEventUpdating:ResponseStart",
   UPDATING_RESPONSE_CONTENT: "NetMonitor:NetworkEventUpdating:ResponseContent",
   RECEIVED_RESPONSE_CONTENT: "NetMonitor:NetworkEventUpdated:ResponseContent",
 
   // Fired once the connection is established
   CONNECTED: "connected",
+
+  // When request payload (HTTP details data) are fetched from the backend.
+  PAYLOAD_READY: "NetMonitor:PayloadReady",
 };
 
 const UPDATE_PROPS = [
   "method",
   "url",
   "remotePort",
   "remoteAddress",
   "status",
--- a/devtools/client/netmonitor/test/browser_net_copy_headers.js
+++ b/devtools/client/netmonitor/test/browser_net_copy_headers.js
@@ -9,31 +9,37 @@
 
 add_task(function* () {
   let { tab, monitor } = yield initNetMonitor(SIMPLE_URL);
   info("Starting test... ");
 
   let { document, store, windowRequire } = monitor.panelWin;
   let {
     getSortedRequests,
+    getSelectedRequest,
   } = windowRequire("devtools/client/netmonitor/src/selectors/index");
 
   let wait = waitForNetworkEvents(monitor, 1);
   tab.linkedBrowser.reload();
   yield wait;
 
   EventUtils.sendMouseEvent({ type: "mousedown" },
     document.querySelectorAll(".request-list-item")[0]);
 
   let requestItem = getSortedRequests(store.getState()).get(0);
   let { method, httpVersion, status, statusText } = requestItem;
 
   EventUtils.sendMouseEvent({ type: "contextmenu" },
     document.querySelectorAll(".request-list-item")[0]);
 
+  let selectedRequest = getSelectedRequest(store.getState());
+  is(selectedRequest, requestItem, "Proper request is selected");
+  ok(selectedRequest.requestHeaders, "Selected request should have request headers");
+  ok(selectedRequest.responseHeaders, "Selected request should have response headers");
+
   const EXPECTED_REQUEST_HEADERS = [
     `${method} ${SIMPLE_URL} ${httpVersion}`,
     "Host: example.com",
     "User-Agent: " + navigator.userAgent + "",
     "Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8",
     "Accept-Language: " + navigator.languages.join(",") + ";q=0.5",
     "Accept-Encoding: gzip, deflate",
     "Connection: keep-alive",
--- a/devtools/client/netmonitor/test/head.js
+++ b/devtools/client/netmonitor/test/head.js
@@ -202,23 +202,23 @@ function waitForAllRequestsFinished(moni
     function maybeResolve() {
       // Have all the requests in the map finished yet?
       if (![...requests.values()].every(finished => finished)) {
         return;
       }
 
       // All requests are done - unsubscribe from events and resolve!
       window.off(EVENTS.NETWORK_EVENT, onRequest);
-      window.off(EVENTS.RECEIVED_EVENT_TIMINGS, onTimings);
+      window.off(EVENTS.PAYLOAD_READY, onTimings);
       info("All requests finished");
       resolve();
     }
 
     window.on(EVENTS.NETWORK_EVENT, onRequest);
-    window.on(EVENTS.RECEIVED_EVENT_TIMINGS, onTimings);
+    window.on(EVENTS.PAYLOAD_READY, onTimings);
   });
 }
 
 function initNetMonitor(url, enableCache) {
   info("Initializing a network monitor pane.");
 
   return Task.spawn(function* () {
     let tab = yield addTab(url);
@@ -291,32 +291,34 @@ function waitForNetworkEvents(monitor, g
   return new Promise((resolve) => {
     let panel = monitor.panelWin;
     let { windowRequire } = panel;
     let { getNetworkRequest } =
       windowRequire("devtools/client/netmonitor/src/connector/index");
     let progress = {};
     let genericEvents = 0;
     let postEvents = 0;
+    let payloadReady = 0;
     let awaitedEventsToListeners = [
       ["UPDATING_REQUEST_HEADERS", onGenericEvent],
       ["RECEIVED_REQUEST_HEADERS", onGenericEvent],
       ["UPDATING_REQUEST_COOKIES", onGenericEvent],
       ["RECEIVED_REQUEST_COOKIES", onGenericEvent],
       ["UPDATING_REQUEST_POST_DATA", onPostEvent],
       ["RECEIVED_REQUEST_POST_DATA", onPostEvent],
       ["UPDATING_RESPONSE_HEADERS", onGenericEvent],
       ["RECEIVED_RESPONSE_HEADERS", onGenericEvent],
       ["UPDATING_RESPONSE_COOKIES", onGenericEvent],
       ["RECEIVED_RESPONSE_COOKIES", onGenericEvent],
       ["STARTED_RECEIVING_RESPONSE", onGenericEvent],
       ["UPDATING_RESPONSE_CONTENT", onGenericEvent],
       ["RECEIVED_RESPONSE_CONTENT", onGenericEvent],
       ["UPDATING_EVENT_TIMINGS", onGenericEvent],
-      ["RECEIVED_EVENT_TIMINGS", onGenericEvent]
+      ["RECEIVED_EVENT_TIMINGS", onGenericEvent],
+      ["PAYLOAD_READY", onPayloadReady]
     ];
 
     function initProgressForURL(url) {
       if (progress[url]) {
         return;
       }
       progress[url] = {};
       awaitedEventsToListeners.forEach(function ([e]) {
@@ -346,32 +348,45 @@ function waitForNetworkEvents(monitor, g
         // Must have been related to reloading document to disable cache.
         // Ignore the event.
         return;
       }
       postEvents++;
       maybeResolve(event, actor, networkInfo);
     }
 
+    function onPayloadReady(event, actor) {
+      let networkInfo = getNetworkRequest(actor);
+      if (!networkInfo) {
+        // Must have been related to reloading document to disable cache.
+        // Ignore the event.
+        return;
+      }
+
+      payloadReady++;
+      maybeResolve(event, actor, networkInfo);
+    }
+
     function maybeResolve(event, actor, networkInfo) {
       info("> Network events progress: " +
         genericEvents + "/" + ((getRequests + postRequests) * 13) + ", " +
         postEvents + "/" + (postRequests * 2) + ", " +
         "got " + event + " for " + actor);
 
       let url = networkInfo.request.url;
       updateProgressForURL(url, event);
 
       // Uncomment this to get a detailed progress logging (when debugging a test)
       // info("> Current state: " + JSON.stringify(progress, null, 2));
 
       // There are 15 updates which need to be fired for a request to be
       // considered finished. The "requestPostData" packet isn't fired for
       // non-POST requests.
-      if (genericEvents >= (getRequests + postRequests) * 13 &&
+      if (payloadReady >= (getRequests + postRequests) &&
+        genericEvents >= (getRequests + postRequests) * 13 &&
         postEvents >= postRequests * 2) {
         awaitedEventsToListeners.forEach(([e, l]) => panel.off(EVENTS[e], l));
         executeSoon(resolve);
       }
     }
 
     awaitedEventsToListeners.forEach(([e, l]) => panel.on(EVENTS[e], l));
   });
--- a/devtools/client/webconsole/new-console-output/new-console-output-wrapper.js
+++ b/devtools/client/webconsole/new-console-output/new-console-output-wrapper.js
@@ -268,17 +268,17 @@ NewConsoleOutputWrapper.prototype = {
     this.throttledDispatchTimeout = setTimeout(() => {
       this.throttledDispatchTimeout = null;
 
       store.dispatch(actions.messagesAdd(this.queuedMessageAdds));
       this.queuedMessageAdds = [];
 
       if (this.queuedMessageUpdates.length > 0) {
         this.queuedMessageUpdates.forEach(({ message, res }) => {
-          store.dispatch(actions.networkMessageUpdate(message));
+          store.dispatch(actions.networkMessageUpdate(message, null, res));
           this.jsterm.hud.emit("network-message-updated", res);
         });
         this.queuedMessageUpdates = [];
       }
       if (this.queuedRequestUpdates.length > 0) {
         this.queuedRequestUpdates.forEach(({ id, data}) => {
           store.dispatch(actions.networkUpdateRequest(id, data));
         });
--- a/devtools/client/webconsole/new-console-output/store.js
+++ b/devtools/client/webconsole/new-console-output/store.js
@@ -172,40 +172,46 @@ function enableNetProvider(hud) {
       if (!dataProvider) {
         dataProvider = new DataProvider({
           actions,
           webConsoleClient: proxy.webConsoleClient
         });
       }
 
       let type = action.type;
+      let newState = reducer(state, action);
 
-      // If network message has been opened, fetch all
-      // HTTP details from the backend.
+      // If network message has been opened, fetch all HTTP details
+      // from the backend. It can happen (especially in test) that
+      // the message is opened before all network event updates are
+      // received. The rest of updates will be handled below, see:
+      // NETWORK_MESSAGE_UPDATE action handler.
       if (type == MESSAGE_OPEN) {
         let message = getMessage(state, action.id);
         if (!message.openedOnce && message.source == "network") {
+          dataProvider.onNetworkEvent(null, message);
           message.updates.forEach(updateType => {
             dataProvider.onNetworkEventUpdate(null, {
               packet: { updateType: updateType },
               networkInfo: message,
             });
           });
         }
       }
 
       // Process all incoming HTTP details packets.
       if (type == NETWORK_MESSAGE_UPDATE) {
-        let open = getAllMessagesUiById(state).includes(action.id);
+        let actor = action.response.networkInfo.actor;
+        let open = getAllMessagesUiById(state).includes(actor);
         if (open) {
           dataProvider.onNetworkEventUpdate(null, action.response);
         }
       }
 
-      return reducer(state, action);
+      return newState;
     }
 
     return next(netProviderEnhancer, initialState, enhancer);
   };
 }
 /**
  * Helper function for releasing backend actors.
  */
--- a/devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_network_messages_expand.js
+++ b/devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_network_messages_expand.js
@@ -29,20 +29,22 @@ add_task(async function task() {
   const documentUrl = TEST_PATH + TEST_FILE;
   await loadDocument(documentUrl);
   info("Document loaded.");
 
   let messageNode = await waitFor(() => findMessage(hud, documentUrl));
   let urlNode = messageNode.querySelector(".url");
   info("Network message found.");
 
+  let updates = waitForNetworkUpdates(toolbox);
+
   // Expand network log
   urlNode.click();
 
-  await waitForNetworkUpdates(toolbox);
+  await updates;
   await testNetworkMessage(messageNode);
 });
 
 async function testNetworkMessage(messageNode) {
   let headersTab = messageNode.querySelector("#headers-tab");
   let cookiesTab = messageNode.querySelector("#cookies-tab");
   let paramsTab = messageNode.querySelector("#params-tab");
   let responseTab = messageNode.querySelector("#response-tab");