Bug 1395759 - Properly declare request payload as ready; r=nchevobbe draft
authorJan Odvarko <odvarko@gmail.com>
Tue, 19 Sep 2017 09:47:20 +0200
changeset 679181 80c3bf2a62adab41a8ec411792daba069ecfd1d8
parent 678462 20d9ad08dd36fe5230ad0ccf6cb3e4865d7851cf
child 679182 794cc6387ad0ede48cc52140da6f84aeb06a5187
push id84146
push userjodvarko@mozilla.com
push dateThu, 12 Oct 2017 11:30:03 +0000
reviewersnchevobbe
bugs1395759
milestone58.0a1
Bug 1395759 - Properly declare request payload as ready; r=nchevobbe MozReview-Commit-ID: FQJpWYjBWmq
devtools/client/netmonitor/src/connector/firefox-data-provider.js
devtools/client/netmonitor/test/browser_net_security-error.js
devtools/client/netmonitor/test/browser_net_simple-request-data.js
devtools/client/netmonitor/test/browser_net_throttle.js
--- a/devtools/client/netmonitor/src/connector/firefox-data-provider.js
+++ b/devtools/client/netmonitor/src/connector/firefox-data-provider.js
@@ -9,55 +9,38 @@ const { EVENTS } = require("../constants
 const { CurlUtils } = require("devtools/client/shared/curl");
 const {
   fetchHeaders,
   formDataURI,
 } = require("../utils/request-utils");
 
 /**
  * This object is responsible for fetching additional HTTP
- * data from the backend.
+ * data from the backend over RDP protocol.
+ *
+ * The object also keeps track of RDP requests in-progress,
+ * so it's possible to determine whether all has been fetched
+ * or not.
  */
 class FirefoxDataProvider {
   constructor({webConsoleClient, actions}) {
     // Options
     this.webConsoleClient = webConsoleClient;
     this.actions = actions;
 
     // Internal properties
     this.payloadQueue = [];
-
-    // Public methods
-    this.addRequest = this.addRequest.bind(this);
-    this.updateRequest = this.updateRequest.bind(this);
+    this.rdpRequestMap = new Map();
 
-    // Internals
-    this.fetchResponseBody = this.fetchResponseBody.bind(this);
-    this.fetchRequestHeaders = this.fetchRequestHeaders.bind(this);
-    this.fetchResponseHeaders = this.fetchResponseHeaders.bind(this);
-    this.fetchPostData = this.fetchPostData.bind(this);
-    this.fetchResponseCookies = this.fetchResponseCookies.bind(this);
-    this.fetchRequestCookies = this.fetchRequestCookies.bind(this);
-    this.getPayloadFromQueue = this.getPayloadFromQueue.bind(this);
-    this.isQueuePayloadReady = this.isQueuePayloadReady.bind(this);
-    this.pushPayloadToQueue = this.pushPayloadToQueue.bind(this);
+    // Fetching data from the backend
     this.getLongString = this.getLongString.bind(this);
-    this.getNetworkRequest = this.getNetworkRequest.bind(this);
 
     // Event handlers
     this.onNetworkEvent = this.onNetworkEvent.bind(this);
     this.onNetworkEventUpdate = this.onNetworkEventUpdate.bind(this);
-    this.onRequestHeaders = this.onRequestHeaders.bind(this);
-    this.onRequestCookies = this.onRequestCookies.bind(this);
-    this.onRequestPostData = this.onRequestPostData.bind(this);
-    this.onSecurityInfo = this.onSecurityInfo.bind(this);
-    this.onResponseHeaders = this.onResponseHeaders.bind(this);
-    this.onResponseCookies = this.onResponseCookies.bind(this);
-    this.onResponseContent = this.onResponseContent.bind(this);
-    this.onEventTimings = this.onEventTimings.bind(this);
   }
 
   /**
    * Add a new network request to application state.
    *
    * @param {string} id request id
    * @param {object} data data payload will be added to application state
    */
@@ -103,49 +86,51 @@ class FirefoxDataProvider {
       responseHeaders,
       requestCookies,
       requestHeaders,
       requestPostData,
     } = data;
 
     // fetch request detail contents in parallel
     let [
-      imageObj,
+      responseContentObj,
       requestHeadersObj,
       responseHeadersObj,
       postDataObj,
       requestCookiesObj,
       responseCookiesObj,
     ] = await Promise.all([
-      this.fetchResponseBody(mimeType, responseContent),
+      this.fetchResponseContent(mimeType, responseContent),
       this.fetchRequestHeaders(requestHeaders),
       this.fetchResponseHeaders(responseHeaders),
       this.fetchPostData(requestPostData),
       this.fetchRequestCookies(requestCookies),
       this.fetchResponseCookies(responseCookies),
     ]);
 
     let payload = Object.assign({},
       data,
-      imageObj,
+      responseContentObj,
       requestHeadersObj,
       responseHeadersObj,
       postDataObj,
       requestCookiesObj,
       responseCookiesObj
     );
 
-    this.pushPayloadToQueue(id, payload);
+    this.pushRequestToQueue(id, payload);
 
-    if (this.actions.updateRequest && this.isQueuePayloadReady(id)) {
-      await this.actions.updateRequest(id, this.getPayloadFromQueue(id).payload, true);
+    if (this.actions.updateRequest && this.isRequestPayloadReady(id)) {
+      let payloadFromQueue = this.getRequestFromQueue(id).payload;
+      this.cleanUpQueue(id);
+      await this.actions.updateRequest(id, payloadFromQueue, true);
     }
   }
 
-  async fetchResponseBody(mimeType, responseContent) {
+  async fetchResponseContent(mimeType, responseContent) {
     let payload = {};
     if (mimeType && responseContent && responseContent.content) {
       let { encoding, text } = responseContent.content;
       let response = await this.getLongString(text);
 
       if (mimeType.includes("image/")) {
         payload.responseContentDataUri = formDataURI(mimeType, encoding, response);
       }
@@ -243,51 +228,52 @@ class FirefoxDataProvider {
   }
 
   /**
    * Access a payload item from payload queue.
    *
    * @param {string} id request id
    * @return {boolean} return a queued payload item from queue.
    */
-  getPayloadFromQueue(id) {
+  getRequestFromQueue(id) {
     return this.payloadQueue.find((item) => item.id === id);
   }
 
   /**
    * 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.
    */
-  isQueuePayloadReady(id) {
-    let queuedPayload = this.getPayloadFromQueue(id);
-
-    // TODO we should find a better solution since it might happen
-    // that eventTimings is not the last update.
-    return queuedPayload && queuedPayload.payload.eventTimings;
+  isRequestPayloadReady(id) {
+    // The payload is ready when there is no RDP request in-progress.
+    return !this.rdpRequestMap.get(id);
   }
 
   /**
-   * Push a request payload into a queue if request doesn't exist. Otherwise update the
-   * request itself.
+   * Merge upcoming networkEventUpdate payload into existing one.
    *
    * @param {string} id request id
    * @param {object} payload request data payload
    */
-  pushPayloadToQueue(id, payload) {
-    let queuedPayload = this.getPayloadFromQueue(id);
-    if (!queuedPayload) {
+  pushRequestToQueue(id, payload) {
+    let request = this.getRequestFromQueue(id);
+    if (!request) {
       this.payloadQueue.push({ id, payload });
     } else {
       // Merge upcoming networkEventUpdate payload into existing one
-      queuedPayload.payload = Object.assign({}, queuedPayload.payload, payload);
+      request.payload = Object.assign({}, request.payload, payload);
     }
   }
 
+  cleanUpQueue(id) {
+    this.payloadQueue = this.payloadQueue.filter(
+      request => request.id != id);
+  }
+
   /**
    * Fetches the network information packet from actor server
    *
    * @param {string} id request id
    * @return {object} networkInfo data packet
    */
   getNetworkRequest(id) {
     return this.webConsoleClient.getNetworkRequest(id);
@@ -343,80 +329,160 @@ class FirefoxDataProvider {
 
   /**
    * The "networkEventUpdate" message type handler.
    *
    * @param {string} type message type
    * @param {object} packet the message received from the server.
    * @param {object} networkInfo the network request information.
    */
-  onNetworkEventUpdate(type, { packet, networkInfo }) {
+  onNetworkEventUpdate(type, data) {
+    let { packet, networkInfo } = data;
     let { actor } = networkInfo;
 
     switch (packet.updateType) {
       case "requestHeaders":
-        this.webConsoleClient.getRequestHeaders(actor, this.onRequestHeaders);
-        emit(EVENTS.UPDATING_REQUEST_HEADERS, actor);
+        this.requestData(actor, packet.updateType).then(response => {
+          this.onRequestHeaders(response);
+          emit(EVENTS.UPDATING_REQUEST_HEADERS, actor);
+        });
         break;
       case "requestCookies":
-        this.webConsoleClient.getRequestCookies(actor, this.onRequestCookies);
-        emit(EVENTS.UPDATING_REQUEST_COOKIES, actor);
+        this.requestData(actor, packet.updateType).then(response => {
+          this.onRequestCookies(response);
+          emit(EVENTS.UPDATING_REQUEST_COOKIES, actor);
+        });
         break;
       case "requestPostData":
-        this.webConsoleClient.getRequestPostData(actor, this.onRequestPostData);
-        emit(EVENTS.UPDATING_REQUEST_POST_DATA, actor);
+        this.requestData(actor, packet.updateType).then(response => {
+          this.onRequestPostData(response);
+          emit(EVENTS.UPDATING_REQUEST_POST_DATA, actor);
+        });
         break;
       case "securityInfo":
         this.updateRequest(actor, {
           securityState: networkInfo.securityInfo,
         }).then(() => {
-          this.webConsoleClient.getSecurityInfo(actor, this.onSecurityInfo);
-          emit(EVENTS.UPDATING_SECURITY_INFO, actor);
+          this.requestData(actor, packet.updateType).then(response => {
+            this.onSecurityInfo(response);
+            emit(EVENTS.UPDATING_SECURITY_INFO, actor);
+          });
         });
         break;
       case "responseHeaders":
-        this.webConsoleClient.getResponseHeaders(actor, this.onResponseHeaders);
-        emit(EVENTS.UPDATING_RESPONSE_HEADERS, actor);
+        this.requestData(actor, packet.updateType).then(response => {
+          this.onResponseHeaders(response);
+          emit(EVENTS.UPDATING_RESPONSE_HEADERS, actor);
+        });
         break;
       case "responseCookies":
-        this.webConsoleClient.getResponseCookies(actor, this.onResponseCookies);
-        emit(EVENTS.UPDATING_RESPONSE_COOKIES, actor);
+        this.requestData(actor, packet.updateType).then(response => {
+          this.onResponseCookies(response);
+          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.webConsoleClient.getResponseContent(actor,
-          this.onResponseContent.bind(this, {
+        this.requestData(actor, packet.updateType).then(response => {
+          this.onResponseContent({
             contentSize: networkInfo.response.bodySize,
             transferredSize: networkInfo.response.transferredSize,
             mimeType: networkInfo.response.content.mimeType
-          }));
-        emit(EVENTS.UPDATING_RESPONSE_CONTENT, actor);
+          }, response);
+          emit(EVENTS.UPDATING_RESPONSE_CONTENT, actor);
+        });
         break;
       case "eventTimings":
         this.updateRequest(actor, { totalTime: networkInfo.totalTime })
           .then(() => {
-            this.webConsoleClient.getEventTimings(actor, this.onEventTimings);
-            emit(EVENTS.UPDATING_EVENT_TIMINGS, actor);
+            this.requestData(actor, packet.updateType).then(response => {
+              this.onEventTimings(response);
+              emit(EVENTS.UPDATING_EVENT_TIMINGS, actor);
+            });
           });
         break;
     }
   }
 
   /**
+   * Wrapper method for requesting HTTP details data from the backend.
+   *
+   * It collects all RDP requests and monitors responses, so it's
+   * possible to determine whether (and when) all requested data
+   * has been fetched from the backend.
+   *
+   * It also nicely returns a promise.
+   *
+   * @param {string} actor actor id (used as request id)
+   * @param {string} method identifier of the data we want to fetch
+   *
+   * @return {Promise} return a promise resolved when data are received.
+   */
+  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;
+  }
+
+  /**
    * Handles additional information received for a "requestHeaders" packet.
    *
    * @param {object} response the message received from the server.
    */
   onRequestHeaders(response) {
     this.updateRequest(response.from, {
       requestHeaders: response
     }).then(() => {
--- a/devtools/client/netmonitor/test/browser_net_security-error.js
+++ b/devtools/client/netmonitor/test/browser_net_security-error.js
@@ -21,16 +21,18 @@ add_task(function* () {
   yield ContentTask.spawn(tab.linkedBrowser, {}, function* () {
     content.wrappedJSObject.performRequests(1, "https://nocert.example.com");
   });
   yield requestsDone;
 
   let securityInfoLoaded = waitForDOM(document, ".security-info-value");
   EventUtils.sendMouseEvent({ type: "click" },
     document.querySelector(".network-details-panel-toggle"));
+
+  yield waitUntil(() => document.querySelector("#security-tab"));
   EventUtils.sendMouseEvent({ type: "click" },
     document.querySelector("#security-tab"));
   yield securityInfoLoaded;
 
   let errormsg = document.querySelector(".security-info-value");
   isnot(errormsg.textContent, "", "Error message is not empty.");
 
   return teardown(monitor);
@@ -45,16 +47,18 @@ add_task(function* () {
       "RECEIVED_REQUEST_HEADERS",
       "UPDATING_REQUEST_COOKIES",
       "RECEIVED_REQUEST_COOKIES",
       "STARTED_RECEIVING_RESPONSE",
       "UPDATING_RESPONSE_CONTENT",
       "RECEIVED_RESPONSE_CONTENT",
       "UPDATING_EVENT_TIMINGS",
       "RECEIVED_EVENT_TIMINGS",
+      "UPDATING_SECURITY_INFO",
+      "RECEIVED_SECURITY_INFO",
     ];
 
     let promises = awaitedEvents.map((event) => {
       return monitor.panelWin.once(EVENTS[event]);
     });
 
     return Promise.all(promises);
   }
--- a/devtools/client/netmonitor/test/browser_net_simple-request-data.js
+++ b/devtools/client/netmonitor/test/browser_net_simple-request-data.js
@@ -314,16 +314,21 @@ function test() {
         SIMPLE_SJS,
         {
           time: true
         }
       );
     });
 
     expectEvent(EVENTS.RECEIVED_EVENT_TIMINGS, async () => {
+      await waitUntil(() => {
+        let requestItem = getSortedRequests(store.getState()).get(0);
+        return requestItem && requestItem.eventTimings;
+      });
+
       let requestItem = getSortedRequests(store.getState()).get(0);
 
       ok(requestItem.eventTimings,
         "There should be a eventTimings data available.");
       is(typeof requestItem.eventTimings.timings.blocked, "number",
         "The eventTimings data has an incorrect |timings.blocked| property.");
       is(typeof requestItem.eventTimings.timings.dns, "number",
         "The eventTimings data has an incorrect |timings.dns| property.");
--- a/devtools/client/netmonitor/test/browser_net_throttle.js
+++ b/devtools/client/netmonitor/test/browser_net_throttle.js
@@ -46,16 +46,21 @@ function* throttleTest(actuallyThrottle)
       resolve(response);
     });
   });
 
   let eventPromise = monitor.panelWin.once(EVENTS.RECEIVED_EVENT_TIMINGS);
   yield triggerActivity(ACTIVITY_TYPE.RELOAD.WITH_CACHE_DISABLED);
   yield eventPromise;
 
+  yield waitUntil(() => {
+    let requestItem = getSortedRequests(store.getState()).get(0);
+    return requestItem && requestItem.eventTimings;
+  });
+
   let requestItem = getSortedRequests(store.getState()).get(0);
   const reportedOneSecond = requestItem.eventTimings.timings.receive > 1000;
   if (actuallyThrottle) {
     ok(reportedOneSecond, "download reported as taking more than one second");
   } else {
     ok(!reportedOneSecond, "download reported as taking less than one second");
   }