Surface SCT (Signed Certificate Timestamp) counts in the Security panel.

third_party/WebKit/Source/devtools/front_end/security/SecurityPanel.js

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 /**
  * @constructor
  * @extends {WebInspector.PanelWithSidebar}
  * @implements {WebInspector.TargetManager.Observer}
  */
 WebInspector.SecurityPanel = function()
@@ skipping 160 matching lines @@
             }
         } else {
             // TODO(lgarron): Store a (deduplicated) list of different security details we have seen. https://crbug.com/503170
             var originState = {};
             originState.securityState = securityState;
 
             var securityDetails = request.securityDetails();
             if (securityDetails) {
                 originState.securityDetails = securityDetails;
                 originState.certificateDetailsPromise = request.target().networkManager.certificateDetailsPromise(securityDetails.certificateId);
+
+                if (securityDetails.certificateValidationDetails)
+                    originState.certificateTransparencySummaryPromise = request.target().networkManager.certificateTransparencySummaryPromise(securityDetails.certificateValidationDetails);

[pfeldman, 2016/01/19 17:54:49]

You issue the request here, but don't claim the result. We typically don't do
that.

[lgarron, 2016/01/22 22:51:27]

I want to issue the request to the proper target for the request. I thought
copying the behaviour of certificateDetailsPromise made the most sense.

Alternatives I see:
- Have the originState explicitly hold on to the target (for the request whose
CT summary we care about).
- Wrap the promise call into a closure so that we can issue it later in the code
where we're about to claim it.
- Make it a documented DevTools protocol API guarantee (for now) that
certificateTransparencySummaryPromise() is target-independent, and use
WebInspector.multitargetNetworkManager below. I don't like this as much, but I'm
okay with it (until a need arises to change it, which may or may not happen in
the near future as CT policy becomes more mature).

Do you have a preference?

             }
 
             this._origins.set(origin, originState);
 
             this._sidebarTree.addOrigin(origin, securityState);
 
             // Don't construct the origin view yet (let it happen lazily).
         }
     },
 
@@ skipping 593 matching lines @@
         /**
          * @this {WebInspector.SecurityOriginView}
          * @param {?NetworkAgent.CertificateDetails} certificateDetails
          */
         function displayCertificateDetails(certificateDetails)
         {
             var sanDiv = this._createSanDiv(certificateDetails.subject);
             var validFromString = new Date(1000 * certificateDetails.validFrom).toUTCString();
             var validUntilString = new Date(1000 * certificateDetails.validTo).toUTCString();
 
+            var sctValue = WebInspector.UIString("-");
+            if (originState.securityDetails.certificateValidationDetails) {
+
+                var sctTypeList = [];
+                var validationDetails = originState.securityDetails.certificateValidationDetails;

[pfeldman, 2016/01/19 17:54:49]

Extract it above the check.

[lgarron, 2016/01/22 22:51:28]

Done.

+                if (validationDetails.numValidScts > 0)

[pfeldman, 2016/01/19 17:54:49]

if (validationDetails.numValidScts)
   ...

[lgarron, 2016/01/22 22:51:27]

*grumble* Okay, done.

+                    sctTypeList.push(WebInspector.UIString("%d valid SCT%s", validationDetails.numValidScts, (validationDetails.numValidScts > 1) ? "s" : ""));
+                if (validationDetails.numInvalidScts > 0)
+                    sctTypeList.push(WebInspector.UIString("%d invalid SCT%s", validationDetails.numInvalidScts, (validationDetails.numInvalidScts > 1) ? "s" : ""));
+                if (validationDetails.numUnknownScts > 0)
+                    sctTypeList.push(WebInspector.UIString("%d SCT%s from unknown logs", validationDetails.numUnknownScts, (validationDetails.numUnknownScts > 1) ? "s" : ""));
+
+                var sctTypeCounts = (sctTypeList.length == 0) ? WebInspector.UIString("0 SCTs") : sctTypeList.join(", ");

[pfeldman, 2016/01/19 17:54:49]

sctTypeList.length ? ...

[lgarron, 2016/01/22 22:51:28]

I am checking if the list (of strings) I'm concatenating is empty and showing
another string in that case. Would you prefer something else? e.g.

    var sctTypeCounts = (sctTypeList.length) ? sctTypeList.join(", ") :
WebInspector.UIString("0 SCTs");

or

    var sctTypeCounts = sctTypeList.join(", ");
    if (sctTypeCounts === "") {
      WebInspector.UIString("0 SCTs")
    }

+
+                if (originState.certificateTransparencySummaryPromise) {

[pfeldman, 2016/01/19 17:54:49]

Issue the request here instead.

[lgarron, 2016/01/22 22:51:28]

See above.

+
+                    /**
+                     * @param {function(string)} resolve
+                     * @param {function()} reject - Unused
+                     */
+                    function executor(resolve, reject)
+                    {
+                        /**
+                         * @param {function(string)} summary
+                         */
+                        function innerResolve(summary)
+                        {
+                            resolve(WebInspector.UIString("%s (%s)", summary, sctTypeCounts));
+                        }
+                        function innerReject()
+                        {
+                            resolve(sctTypeCounts);
+                        }
+                        originState.certificateTransparencySummaryPromise.then(innerResolve, innerReject);
+                    }
+                    sctValue = new Promise(executor);

[pfeldman, 2016/01/19 17:54:49]

Why do you use promise here?

[lgarron, 2016/01/22 22:51:28]

Removed in a refactor.

+                } else {
+                    // Fall back and just display the counts.
+                    sctValue = sctTypeCounts;
+                }
+            }
+
             var table = new WebInspector.SecurityDetailsTable();
             certificateSection.appendChild(table.element());
             table.addRow("Subject", certificateDetails.subject.name);
             table.addRow("SAN", sanDiv);
             table.addRow("Valid From", validFromString);
             table.addRow("Valid Until", validUntilString);
             table.addRow("Issuer", certificateDetails.issuer);
+            table.addRow("SCTs", sctValue);

[pfeldman, 2016/01/19 17:54:49]

You should only pass values into the UI components.

[lgarron, 2016/01/22 22:51:28]

I wanted to satisfy two design constraints:

- Be able to insert this row before/after whatever rows we'd like,
deterministically.
- Construct most of the table without waiting for the promise for this
particular row to resolve.

I've handled this in the next patch by modifying the addRow() function.

             table.addRow("", WebInspector.SecurityPanel.createCertificateViewerButton(WebInspector.UIString("Open full certificate details"), originState.securityDetails.certificateId));
-            // TODO(lgarron): Make SCT status available in certificate details and show it here.
         }
 
         function displayCertificateDetailsUnavailable ()
         {
             certificateSection.createChild("div").textContent = WebInspector.UIString("Certificate details unavailable.");
         }
 
         originState.certificateDetailsPromise.then(displayCertificateDetails.bind(this), displayCertificateDetailsUnavailable);
 
         var noteSection = this.element.createChild("div", "origin-view-section");
@@ skipping 83 matching lines @@
     /**
      * @return: {!Element}
      */
     element: function()
     {
         return this._element;
     },
 
     /**
      * @param {string} key
-     * @param {string|!Node} value
+     * @param {string|!Node|!Promise<string>} value

[pfeldman, 2016/01/19 17:54:49]

Resolve value prior to getting here, we try to not use | unless necessary.

[lgarron, 2016/01/22 22:51:28]

Rather than allowing a promise to introduce asynchronicity inside this function,
I've opted to return `valueDiv` so that it can be modified by the caller later.
Let me know if you think this is not a good solution.

      */
     addRow: function(key, value)
     {
         var row = this._element.createChild("div", "details-table-row");
         row.createChild("div").textContent = key;
 
         var valueDiv = row.createChild("div");
-        if (typeof value === "string") {
-            valueDiv.textContent = value;
-        } else {
-            valueDiv.appendChild(value);
+
+        /**
+         * @param {string|!Node} value
+         */
+        function resolve(value) {

[pfeldman, 2016/01/19 17:54:49]

By now you are formatting {} poorly.

[lgarron, 2016/01/22 22:51:28]

Thanks for catching; some habits die hard without a linter/formatter. In any
case, this was the only place I made the mistake, and it's gone in the next
patch.

+            if (typeof value === "string") {
+                valueDiv.textContent = value;
+            } else {
+                valueDiv.appendChild(value);
+            }
         }
+
+        function reject() {
+            valueDiv.textContent = WebInspector.UIString("-");
+        }
+
+        // We have to modify the signature of `resolve` to work around a type checker limitation.
+        Promise.resolve(value).then(/** @type {function((string|!Node|!Promise<string>))} */(resolve), reject);
     }
 }