Clean up the JavaScript for net-internals/#hsts.

chrome/browser/resources/net_internals/hsts_view.js

 // Copyright (c) 2012 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.
 
 /**
  * HSTS is HTTPS Strict Transport Security: a way for sites to elect to always
  * use HTTPS. See http://dev.chromium.org/sts
  *
  * This UI allows a user to query and update the browser's list of HSTS domains.
  * It also allows users to query and update the browser's list of public key
@@ skipping 99 matching lines @@
       if (result.result == false) {
         this.queryOutputDiv_.innerHTML = '<b>Not found</b>';
         yellowFade(this.queryOutputDiv_);
         return;
       }
 
       this.queryOutputDiv_.innerHTML = '';
 
       var s = addNode(this.queryOutputDiv_, 'span');
       s.innerHTML = '<b>Found:</b><br/>';
-      var t;
-      var b;
 
-      addTextNode(this.queryOutputDiv_, ' domain: ');
-      t = addNodeWithText(this.queryOutputDiv_, 'tt', result.domain);
-      b = addNode(this.queryOutputDiv_, 'br');
-
-      addTextNode(this.queryOutputDiv_, ' static_upgrade_mode: ');
-      t = addNodeWithText(this.queryOutputDiv_, 'tt',
-                          modeToString(result.static_upgrade_mode));
-      b = addNode(this.queryOutputDiv_, 'br');
-
-      addTextNode(this.queryOutputDiv_, ' static_sts_include_subdomains: ');
-      t = addNodeWithText(this.queryOutputDiv_, 'tt',
-                          result.static_sts_include_subdomains);
-      b = addNode(this.queryOutputDiv_, 'br');
-
-      addTextNode(this.queryOutputDiv_, ' static_pkp_include_subdomains: ');
-      t = addNodeWithText(this.queryOutputDiv_, 'tt',
-                          result.static_pkp_include_subdomains);
-      b = addNode(this.queryOutputDiv_, 'br');
-
-      addTextNode(this.queryOutputDiv_, ' static_sts_observed: ');
-      t = addNodeWithText(this.queryOutputDiv_, 'tt',
-                          result.static_sts_observed);
-      b = addNode(this.queryOutputDiv_, 'br');
-
-      addTextNode(this.queryOutputDiv_, ' static_pkp_observed: ');
-      t = addNodeWithText(this.queryOutputDiv_, 'tt',
-                          result.static_pkp_observed);
-      b = addNode(this.queryOutputDiv_, 'br');
-
-      addTextNode(this.queryOutputDiv_, ' static_spki_hashes: ');
-      t = addNode(this.queryOutputDiv_, 'tt');
+      var keys = [
+        'domain', 'static_upgrade_mode', 'static_sts_include_subdomains',
+        'static_pkp_include_subdomains', 'static_sts_observed',
+        'static_pkp_observed', 'static_spki_hashes', 'dynamic_upgrade_mode',
+        'dynamic_sts_include_subdomains', 'dynamic_pkp_include_subdomains',
+        'dynamic_sts_observed', 'dynamic_pkp_observed', 'dynamic_spki_hashes'
+      ];
 
       // |public_key_hashes| is an old synonym for what is now
       // |preloaded_spki_hashes|, which in turn is a legacy synonym for
       // |static_spki_hashes|.
       if (typeof result.public_key_hashes === 'undefined')
         result.public_key_hashes = '';

[eroman, 2014/05/12 21:29:13]

Modifying the "result" object for display is bad form. See the recommendation
below about getting rid of this all-together.

[palmer, 2014/05/12 21:46:51]

Done.

       if (typeof result.preloaded_spki_hashes === 'undefined')
         result.preloaded_spki_hashes = '';
       if (typeof result.static_spki_hashes === 'undefined')
         result.static_spki_hashes = '';
 
-      var hashes = [];
+      var staticHashes = [];

[eroman, 2014/05/12 21:29:13]

While you are cleaning up, this can be merged with the code right above (i.e.
the "if (typeof result.public_key_hashes = 'undefined')"...).

How about:

var kStaticHashKeys = ['public_key_hashes', 'preloaded_spki_hashes',
'static_spki_hashes'];

var staticHashes = [];
for (var i = 0; i < kStaticHashKeys.length; ++i) {
  var staticHashValue = result[kStaticHashKeys[i]];
  if (staticHashValue != undefined)
    staticHashes.push(staticHashValue);
}

[palmer, 2014/05/12 21:46:51]

Done.

       if (result.public_key_hashes)
-        hashes.push(result.public_key_hashes);
+        staticHashes.push(result.public_key_hashes);
       if (result.preloaded_spki_hashes)
-        hashes.push(result.preloaded_spki_hashes);
+        staticHashes.push(result.preloaded_spki_hashes);
       if (result.static_spki_hashes)
-        hashes.push(result.static_spki_hashes);
+        staticHashes.push(result.static_spki_hashes);
 
-      t.textContent = hashes.join(',');
-      b = addNode(this.queryOutputDiv_, 'br');
+      for (var i = 0; i < keys.length; ++i) {
+        var key = keys[i];
+        addTextNode(this.queryOutputDiv_, ' ' + key + ': ');
 
-      addTextNode(this.queryOutputDiv_, ' dynamic_upgrade_mode: ');
-      t = addNodeWithText(this.queryOutputDiv_, 'tt',
-                          modeToString(result.dynamic_upgrade_mode));
-      b = addNode(this.queryOutputDiv_, 'br');
+        // If there are no static_hashes, do not make it seem like there is a
+        // static PKP policy in place.
+        if (staticHashes.length == 0 && key.indexOf('static_pkp_') == 0) {
+          addNode(this.queryOutputDiv_, 'br');
+          continue;
+        }
 
-      addTextNode(this.queryOutputDiv_, ' dynamic_sts_include_subdomains: ');
-      t = addNodeWithText(this.queryOutputDiv_, 'tt',
-                          result.dynamic_sts_include_subdomains || '');
-      b = addNode(this.queryOutputDiv_, 'br');
-
-      addTextNode(this.queryOutputDiv_, ' dynamic_pkp_include_subdomains: ');
-      t = addNodeWithText(this.queryOutputDiv_, 'tt',
-                          result.dynamic_pkp_include_subdomains || '');
-      b = addNode(this.queryOutputDiv_, 'br');
-
-      addTextNode(this.queryOutputDiv_, ' dynamic_sts_observed: ');
-      t = addNodeWithText(this.queryOutputDiv_, 'tt',
-                          result.dynamic_sts_observed || '');
-      b = addNode(this.queryOutputDiv_, 'br');
-
-      addTextNode(this.queryOutputDiv_, ' dynamic_pkp_observed: ');
-      t = addNodeWithText(this.queryOutputDiv_, 'tt',
-                          result.dynamic_pkp_observed || '');
-      b = addNode(this.queryOutputDiv_, 'br');
-
-      addTextNode(this.queryOutputDiv_, ' dynamic_spki_hashes: ');
-      t = addNodeWithText(this.queryOutputDiv_, 'tt',
-                          result.dynamic_spki_hashes || '');
+        if (key === 'static_spki_hashes') {
+          addNodeWithText(this.queryOutputDiv_, 'tt', staticHashes.join(','));
+        } else if (key.indexOf('_upgrade_mode') >= 0) {
+          addNodeWithText(this.queryOutputDiv_, 'tt',
+                          modeToString(result[key]));
+        } else {
+          addNodeWithText(this.queryOutputDiv_, 'tt', result[key] || '');

[eroman, 2014/05/12 21:29:13]

Be aware that this pattern (a || '') will cause a value of 0 to be masked by ''.
Probably OK but your use case, but just wanted to make sure you are aware of
this. If not, you may consider:

var value = result[key];
 .... (value == undefined ? '' : value) ....

[palmer, 2014/05/12 21:46:51]

Done.

+        }
+        addNode(this.queryOutputDiv_, 'br');
+      }
 
       yellowFade(this.queryOutputDiv_);
     }
   };
 
   function modeToString(m) {
     // These numbers must match those in
     // TransportSecurityState::DomainState::UpgradeMode.
     if (m == 0) {
       return 'STRICT';
     } else if (m == 1) {
       return 'OPPORTUNISTIC';
     } else {
       return 'UNKNOWN';
     }
   }
 
   function yellowFade(element) {
     element.style.webkitTransitionProperty = 'background-color';
     element.style.webkitTransitionDuration = '0';
     element.style.backgroundColor = '#fffccf';
     setTimeout(function() {
       element.style.webkitTransitionDuration = '1000ms';
       element.style.backgroundColor = '#fff';
     }, 0);
   }
 
   return HSTSView;
 })();