Bug 1404929 - Security info should be loaded lazily;r=Honza

MozReview-Commit-ID: JIwepd7qdOB --HG-- extra : rebase_source : 9fa7971412798e1f2f580a44543f1dd4e946d823
2017-11-15 12:50:47 +08:00 · 2017-11-15 12:50:47 +08:00 · 8083de770a
--- a/devtools/client/netmonitor/src/components/SecurityPanel.js
+++ b/devtools/client/netmonitor/src/components/SecurityPanel.js
@ -4,7 +4,10 @@

 "use strict";

-const { createFactory } = require("devtools/client/shared/vendor/react");
+const {
+  Component,
+  createFactory,
+} = require("devtools/client/shared/vendor/react");
 const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
 const dom = require("devtools/client/shared/vendor/react-dom-factories");
 const { L10N } = require("../utils/l10n");
@ -47,128 +50,157 @@ const SHA1_FINGERPRINT_LABEL = L10N.getStr("certmgr.certdetail.sha1fingerprint")
 * This contains details about the secure connection used including the protocol,
 * the cipher suite, and certificate details
 */
-function SecurityPanel({
-  openLink,
-  request,
-}) {
-  const { securityInfo, url } = request;
-
-  if (!securityInfo || !url) {
-    return null;
-  }
-
-  let object;
-
-  if (securityInfo.state === "secure" || securityInfo.state === "weak") {
-    const { subject, issuer, validity, fingerprint } = securityInfo.cert;
-    const HOST_HEADER_LABEL = L10N.getFormatStr("netmonitor.security.hostHeader",
-      getUrlHost(url));
-
-    object = {
-      [CONNECTION_LABEL]: {
-        [PROTOCOL_VERSION_LABEL]:
-          securityInfo.protocolVersion || NOT_AVAILABLE,
-        [CIPHER_SUITE_LABEL]:
-          securityInfo.cipherSuite || NOT_AVAILABLE,
-        [KEA_GROUP_LABEL]:
-          securityInfo.keaGroupName || NOT_AVAILABLE,
-        [SIGNATURE_SCHEME_LABEL]:
-          securityInfo.signatureSchemeName || NOT_AVAILABLE,
-      },
-      [HOST_HEADER_LABEL]: {
-        [HSTS_LABEL]:
-          securityInfo.hsts ? ENABLED_LABEL : DISABLED_LABEL,
-        [HPKP_LABEL]:
-          securityInfo.hpkp ? ENABLED_LABEL : DISABLED_LABEL,
-      },
-      [CERTIFICATE_LABEL]: {
-        [SUBJECT_INFO_LABEL]: {
-          [CERT_DETAIL_COMMON_NAME_LABEL]:
-            subject.commonName || NOT_AVAILABLE,
-          [CERT_DETAIL_ORG_LABEL]:
-            subject.organization || NOT_AVAILABLE,
-          [CERT_DETAIL_ORG_UNIT_LABEL]:
-            subject.organizationUnit || NOT_AVAILABLE,
-        },
-        [ISSUER_INFO_LABEL]: {
-          [CERT_DETAIL_COMMON_NAME_LABEL]:
-            issuer.commonName || NOT_AVAILABLE,
-          [CERT_DETAIL_ORG_LABEL]:
-            issuer.organization || NOT_AVAILABLE,
-          [CERT_DETAIL_ORG_UNIT_LABEL]:
-            issuer.organizationUnit || NOT_AVAILABLE,
-        },
-        [PERIOD_OF_VALIDITY_LABEL]: {
-          [BEGINS_LABEL]:
-            validity.start || NOT_AVAILABLE,
-          [EXPIRES_LABEL]:
-            validity.end || NOT_AVAILABLE,
-        },
-        [FINGERPRINTS_LABEL]: {
-          [SHA256_FINGERPRINT_LABEL]:
-            fingerprint.sha256 || NOT_AVAILABLE,
-          [SHA1_FINGERPRINT_LABEL]:
-            fingerprint.sha1 || NOT_AVAILABLE,
-        },
-      },
-    };
-  } else {
-    object = {
-      [ERROR_LABEL]:
-        new DOMParser().parseFromString(securityInfo.errorMessage, "text/html")
-          .body.textContent || NOT_AVAILABLE
+class SecurityPanel extends Component {
+  static get propTypes() {
+    return {
+      connector: PropTypes.object.isRequired,
+      openLink: PropTypes.func,
+      request: PropTypes.object.isRequired,
    };
  }

-  return div({ className: "panel-container security-panel" },
-    PropertiesView({
-      object,
-      renderValue: (props) => renderValue(props, securityInfo.weaknessReasons),
-      enableFilter: false,
-      expandedNodes: TreeViewClass.getExpandedNodes(object),
-      openLink,
-    })
-  );
-}
-
-SecurityPanel.displayName = "SecurityPanel";
-
-SecurityPanel.propTypes = {
-  request: PropTypes.object.isRequired,
-  openLink: PropTypes.func,
-};
-
-function renderValue(props, weaknessReasons = []) {
-  const { member, value } = props;
-
-  // Hide object summary
-  if (typeof member.value === "object") {
-    return null;
+  /**
+   * `componentDidMount` is called when opening the SecurityPanel for the first time
+   */
+  componentDidMount() {
+    this.maybeFetchSecurityInfo(this.props);
  }

-  return span({ className: "security-info-value" },
-    member.name === ERROR_LABEL ?
-      // Display multiline text for security error
-      value
-      :
-      // Display one line selectable text for security details
-      input({
-        className: "textbox-input",
-        readOnly: "true",
-        value,
+  /**
+   * `componentWillReceiveProps` is the only method called when switching between two
+   * requests while the security panel is displayed.
+   */
+  componentWillReceiveProps(nextProps) {
+    this.maybeFetchSecurityInfo(nextProps);
+  }
+
+  /**
+   * When switching to another request, lazily fetch securityInfo
+   * from the backend. The Security Panel will first be empty and then
+   * display the content.
+   */
+  maybeFetchSecurityInfo(props) {
+    if (!props.request.securityInfo) {
+      // This method will set `props.request.securityInfo`
+      // asynchronously and force another render.
+      props.connector.requestData(props.request.id, "securityInfo");
+    }
+  }
+
+  renderValue(props, weaknessReasons = []) {
+    const { member, value } = props;
+
+    // Hide object summary
+    if (typeof member.value === "object") {
+      return null;
+    }
+
+    return span({ className: "security-info-value" },
+      member.name === ERROR_LABEL ?
+        // Display multiline text for security error
+        value
+        :
+        // Display one line selectable text for security details
+        input({
+          className: "textbox-input",
+          readOnly: "true",
+          value,
+        })
+      ,
+      weaknessReasons.indexOf("cipher") !== -1 &&
+      member.name === CIPHER_SUITE_LABEL ?
+        // Display an extra warning icon after the cipher suite
+        div({
+          id: "security-warning-cipher",
+          className: "security-warning-icon",
+          title: WARNING_CIPHER_LABEL,
+        })
+        :
+        null
+    );
+  }
+
+  render() {
+    let { openLink, request } = this.props;
+    const { securityInfo, url } = request;
+
+    if (!securityInfo || !url) {
+      return null;
+    }
+
+    let object;
+
+    if (securityInfo.state === "secure" || securityInfo.state === "weak") {
+      const { subject, issuer, validity, fingerprint } = securityInfo.cert;
+      const HOST_HEADER_LABEL = L10N.getFormatStr("netmonitor.security.hostHeader",
+        getUrlHost(url));
+
+      object = {
+        [CONNECTION_LABEL]: {
+          [PROTOCOL_VERSION_LABEL]:
+            securityInfo.protocolVersion || NOT_AVAILABLE,
+          [CIPHER_SUITE_LABEL]:
+            securityInfo.cipherSuite || NOT_AVAILABLE,
+          [KEA_GROUP_LABEL]:
+            securityInfo.keaGroupName || NOT_AVAILABLE,
+          [SIGNATURE_SCHEME_LABEL]:
+            securityInfo.signatureSchemeName || NOT_AVAILABLE,
+        },
+        [HOST_HEADER_LABEL]: {
+          [HSTS_LABEL]:
+            securityInfo.hsts ? ENABLED_LABEL : DISABLED_LABEL,
+          [HPKP_LABEL]:
+            securityInfo.hpkp ? ENABLED_LABEL : DISABLED_LABEL,
+        },
+        [CERTIFICATE_LABEL]: {
+          [SUBJECT_INFO_LABEL]: {
+            [CERT_DETAIL_COMMON_NAME_LABEL]:
+              subject.commonName || NOT_AVAILABLE,
+            [CERT_DETAIL_ORG_LABEL]:
+              subject.organization || NOT_AVAILABLE,
+            [CERT_DETAIL_ORG_UNIT_LABEL]:
+              subject.organizationUnit || NOT_AVAILABLE,
+          },
+          [ISSUER_INFO_LABEL]: {
+            [CERT_DETAIL_COMMON_NAME_LABEL]:
+              issuer.commonName || NOT_AVAILABLE,
+            [CERT_DETAIL_ORG_LABEL]:
+              issuer.organization || NOT_AVAILABLE,
+            [CERT_DETAIL_ORG_UNIT_LABEL]:
+              issuer.organizationUnit || NOT_AVAILABLE,
+          },
+          [PERIOD_OF_VALIDITY_LABEL]: {
+            [BEGINS_LABEL]:
+              validity.start || NOT_AVAILABLE,
+            [EXPIRES_LABEL]:
+              validity.end || NOT_AVAILABLE,
+          },
+          [FINGERPRINTS_LABEL]: {
+            [SHA256_FINGERPRINT_LABEL]:
+              fingerprint.sha256 || NOT_AVAILABLE,
+            [SHA1_FINGERPRINT_LABEL]:
+              fingerprint.sha1 || NOT_AVAILABLE,
+          },
+        },
+      };
+    } else {
+      object = {
+        [ERROR_LABEL]:
+          new DOMParser().parseFromString(securityInfo.errorMessage, "text/html")
+            .body.textContent || NOT_AVAILABLE
+      };
+    }
+
+    return div({ className: "panel-container security-panel" },
+      PropertiesView({
+        object,
+        renderValue: (props) => this.renderValue(props, securityInfo.weaknessReasons),
+        enableFilter: false,
+        expandedNodes: TreeViewClass.getExpandedNodes(object),
+        openLink,
      })
-    ,
-    weaknessReasons.indexOf("cipher") !== -1 &&
-    member.name === CIPHER_SUITE_LABEL ?
-      // Display an extra warning icon after the cipher suite
-      div({
-        id: "security-warning-cipher",
-        className: "security-warning-icon",
-        title: WARNING_CIPHER_LABEL,
-      })
-      :
-      null
-  );
+    );
+  }
 }

 module.exports = SecurityPanel;
--- a/devtools/client/netmonitor/src/components/TabboxPanel.js
+++ b/devtools/client/netmonitor/src/components/TabboxPanel.js
@ -96,7 +96,11 @@ function TabboxPanel({
        id: PANELS.SECURITY,
        title: SECURITY_TITLE,
      },
-        SecurityPanel({ request, openLink }),
+        SecurityPanel({
+          connector,
+          openLink,
+          request,
+        }),
      ),
    )
  );
--- a/devtools/client/netmonitor/src/connector/firefox-data-provider.js
+++ b/devtools/client/netmonitor/src/connector/firefox-data-provider.js
@ -257,14 +257,11 @@ class FirefoxDataProvider {
    // The payload is ready when all values in the record are true. (i.e. all data
    // received, but the lazy one. responseContent is the only one for now).
    // Note that we never fetch response header/cookies for request with security issues.
-    // (Be careful, securityState can be undefined, for example for WebSocket requests)
-    // Also note that service worker don't have security info set.
    // Bug 1404917 should simplify this heuristic by making all these field be lazily
    // fetched, only on-demand.
    return record.requestHeaders &&
      record.requestCookies &&
      record.eventTimings &&
-      (record.securityInfo || record.fromServiceWorker) &&
      (
       (record.responseHeaders && record.responseCookies) ||
       payload.securityState == "broken" ||
@ -344,12 +341,7 @@ class FirefoxDataProvider {
      requestCookies: false,
      responseHeaders: false,
      responseCookies: false,
-      securityInfo: false,
      eventTimings: false,
-
-      // This isn't a request data, but we need to know about request being served from
-      // service worker later, from isRequestPayloadReady.
-      fromServiceWorker,
    });

    this.addRequest(actor, {
@ -391,12 +383,10 @@ class FirefoxDataProvider {
      case "responseCookies":
        this.requestPayloadData(actor, updateType);
        break;
+      // (Be careful, securityState can be undefined, for example for WebSocket requests)
+      // Also note that service worker don't have security info set.
      case "securityInfo":
-        this.updateRequest(actor, {
-          securityState: networkInfo.securityInfo,
-        }).then(() => {
-          this.requestPayloadData(actor, updateType);
-        });
+        this.updateRequest(actor, { securityState: networkInfo.securityInfo });
        break;
      case "responseStart":
        this.updateRequest(actor, {
@ -422,10 +412,8 @@ class FirefoxDataProvider {
        });
        break;
      case "eventTimings":
-        this.updateRequest(actor, { totalTime: networkInfo.totalTime })
-          .then(() => {
-            this.requestPayloadData(actor, updateType);
-          });
+        this.pushRequestToQueue(actor, { totalTime: networkInfo.totalTime });
+        this.requestPayloadData(actor, updateType);
        break;
    }

@ -491,7 +479,7 @@ class FirefoxDataProvider {
  /**
   * Public connector API to lazily request HTTP details from the backend.
   *
-   * This is internal method that focus on:
+   * The method focus on:
   * - calling the right actor method,
   * - emitting an event to tell we start fetching some request data,
   * - call data processing method.
--- a/devtools/client/netmonitor/test/browser_net_security-details.js
+++ b/devtools/client/netmonitor/test/browser_net_security-details.js
@ -22,12 +22,12 @@ add_task(function* () {
  });
  yield wait;

-  wait = waitForDOM(document, "#security-panel");
  EventUtils.sendMouseEvent({ type: "click" },
    document.querySelector(".network-details-panel-toggle"));
  EventUtils.sendMouseEvent({ type: "click" },
    document.querySelector("#security-tab"));
-  yield wait;
+  yield waitUntil(() => document.querySelector(
+    "#security-panel .security-info-value"));

  let tabpanel = document.querySelector("#security-panel");
  let textboxes = tabpanel.querySelectorAll(".textbox-input");
--- a/devtools/client/netmonitor/test/browser_net_security-error.js
+++ b/devtools/client/netmonitor/test/browser_net_security-error.js
@ -49,8 +49,6 @@ add_task(function* () {
      "RECEIVED_REQUEST_COOKIES",
      "UPDATING_EVENT_TIMINGS",
      "RECEIVED_EVENT_TIMINGS",
-      "UPDATING_SECURITY_INFO",
-      "RECEIVED_SECURITY_INFO",
    ];

    let promises = awaitedEvents.map((event) => {
--- a/devtools/client/netmonitor/test/browser_net_security-icon-click.js
+++ b/devtools/client/netmonitor/test/browser_net_security-icon-click.js
@ -46,9 +46,9 @@ add_task(function* () {

  function* clickAndTestSecurityIcon() {
    let icon = document.querySelector(".requests-security-state-icon");
-
    info("Clicking security icon of the first request and waiting for panel update.");
    EventUtils.synthesizeMouseAtCenter(icon, {}, monitor.panelWin);
+    yield waitUntil(() => document.querySelector("#security-panel .security-info-value"));

    ok(document.querySelector("#security-tab[aria-selected=true]"),
       "Security tab is selected.");
--- a/devtools/client/netmonitor/test/browser_net_security-tab-visibility.js
+++ b/devtools/client/netmonitor/test/browser_net_security-tab-visibility.js
@ -42,7 +42,6 @@ add_task(function* () {
  for (let testcase of TEST_DATA) {
    info("Testing Security tab visibility for " + testcase.desc);
    let onNewItem = monitor.panelWin.once(EVENTS.NETWORK_EVENT);
-    let onSecurityInfo = monitor.panelWin.once(EVENTS.RECEIVED_SECURITY_INFO);
    let onComplete = testcase.isBroken ?
                       waitForSecurityBrokenNetworkEvent() :
                       waitForNetworkEvents(monitor, 1);
@ -65,12 +64,20 @@ add_task(function* () {
      "Security tab is " + (testcase.visibleOnNewEvent ? "visible" : "hidden") +
      " after new request was added to the menu.");

-    info("Waiting for security information to arrive.");
-    yield onSecurityInfo;
+    if (testcase.visibleOnSecurityInfo) {
+      // click security panel to lazy load the securityState
+      yield waitUntil(() => document.querySelector("#security-tab"));
+      EventUtils.sendMouseEvent({ type: "click" },
+        document.querySelector("#security-tab"));
+      yield waitUntil(() => document.querySelector(
+        "#security-panel .security-info-value"));
+      info("Waiting for security information to arrive.");
+
+      yield waitUntil(() => !!getSelectedRequest(store.getState()).securityState);
+      ok(getSelectedRequest(store.getState()).securityState,
+         "Security state arrived.");
+    }

-    yield waitUntil(() => !!getSelectedRequest(store.getState()).securityState);
-    ok(getSelectedRequest(store.getState()).securityState,
-       "Security state arrived.");
    is(!!document.querySelector("#security-tab"), testcase.visibleOnSecurityInfo,
       "Security tab is " + (testcase.visibleOnSecurityInfo ? "visible" : "hidden") +
       " after security information arrived.");
--- a/devtools/client/netmonitor/test/shared-head.js
+++ b/devtools/client/netmonitor/test/shared-head.js
@ -24,7 +24,6 @@ async function waitForExistingRequests(monitor) {
      // in order to ensure there is no more pending payload requests to be done.
      if (!request.requestHeaders || !request.requestCookies ||
          !request.eventTimings ||
-          (!request.securityInfo && !request.fromServiceWorker) ||
          ((!request.responseHeaders || !request.responseCookies) &&
            request.securityState != "broken" &&
            (!request.responseContentAvailable || request.status))) {