Stop rebuilding all elements in chrome://extensions to preserve focus on refresh

chrome/browser/resources/extensions/extension_list.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.
 
 <include src="extension_error.js">
 
 /**
  * The type of the extension data object. The definition is based on
  * chrome/browser/ui/webui/extensions/extension_basic_info.cc
  * and
@@ skipping 59 matching lines @@
 
   /**
    * Creates a new list of extensions.
    * @param {Object=} opt_propertyBag Optional properties.
    * @constructor
    * @extends {HTMLDivElement}
    */
   var ExtensionsList = cr.ui.define('div');
 
   /**
-   * @type {Object.<string, boolean>} A map from extension id to a boolean
-   *     indicating whether the incognito warning is showing. This persists
-   *     between calls to decorate.
-   */
-  var butterBarVisibility = {};
-
-  /**
    * @type {Object.<string, number>} A map from extension id to last reloaded
    *     timestamp. The timestamp is recorded when the user click the 'Reload'
    *     link. It is used to refresh the icon of an unpacked extension.
    *     This persists between calls to decorate.
    */
   var extensionReloadedTimestamp = {};
 
   ExtensionsList.prototype = {
     __proto__: HTMLDivElement.prototype,
 
     /**
      * Indicates whether an embedded options page that was navigated to through
      * the '?options=' URL query has been shown to the user. This is necessary
      * to prevent showExtensionNodes_ from opening the options more than once.
      * @type {boolean}
      * @private
      */
     optionsShown_: false,
 
+    /**
+     * Necessary to only show the butterbar once.
+     * @private {boolean}
+     */
+    butterbarShown_: false,
+
     decorate: function() {
-      this.textContent = '';
-
       this.showExtensionNodes_();
     },
 
     getIdQueryParam_: function() {
       return parseQueryParams(document.location)['id'];
     },
 
     getOptionsQueryParam_: function() {
       return parseQueryParams(document.location)['options'];
     },
 
     /**
      * Creates all extension items from scratch.
      * @private
      */
     showExtensionNodes_: function() {
+      // Any node that is not updated will be deleted.
+      var nodes = document.querySelectorAll('.extension-list-item-wrapper');
+      for (var i = 0; i < nodes.length; ++i) {
+        nodes[i].classList.add('to-remove');

[not at google - send to devlin, 2015/02/11 19:25:35]

It seems odd to be modifying a node's class to mark it for deletion; at least,
how about setting it on the JS wrapper itself? I.e. nodes[i].toRemove = true.

It looks simpler to me than that though. The only time 'to-remove' is added is
here, and the only time it's removed is in updateNode_. You could write it as:

var seenIds = [];

this.data_.extensions.forEach(function(extension) {
  var node = $(extension.id);
  seenIds.push(extension.id);
  ...
});

var nodes = document.querySelectorAll('.extension-list-item-wrapper');
// Note: calling slice() to take a copy of |nodes|, since NodeLists
// are live.
[].slice.call(nodes).forEach(function(node) {
  if (seenIds.indexOf(node.getAttribute('id') == 0)
    node.parentNode.removeChild(node);
});

You could even use a Set() for seenIds if you feel inclined.

[hcarmona, 2015/02/11 23:36:47]

Moving to the JS wrapper simplifies the code so we don't need to update
|toRemove| in |updateNode_|. It makes more sense to only check the list of
pre-existing nodes.

[not at google - send to devlin, 2015/02/12 01:04:50]

Alright. Well, I prefer the version which doesn't have this extra variable
hanging off the node, regardless of whether it's pure JS or a class. Is it
something to do with only checking the list of pre-existing nodes...? There's
going to be at most 1 more node than before, so it's hardly any different, and
less code the way I suggested. Eh.

[Dan Beam, 2015/02/12 01:08:32]

what if we just make a superset of all the IDs in the DOM and from the data and
iterate them?

if we find the ID in the DOM but not data, remove.  if we find the ID in the
data but not the DOM, create.  else update?

[hcarmona, 2015/02/12 02:33:12]

Since we're only ever adding or removing 1 extension at a time, then why don't
we add a class to the extension we're removing and query for it here? This way
we don't need to keep track of what's been visited because we know when the
trash button was clicked and we know what extension. We just need to verify that
the extension is no longer in the data set.

[not at google - send to devlin, 2015/02/12 18:34:39]

What do you mean by a "class"? A DOM class (like you had originally) or
something populated from the webui data?

[Dan Beam, 2015/02/12 20:46:19]

^ ping

+      }
+
       // Iterate over the extension data and add each item to the list.
-      this.data_.extensions.forEach(this.createNode_, this);
+      this.data_.extensions.forEach(function(extension) {
+        var node = $(extension.id);
+
+        if (node)
+          this.updateNode_(extension, node);
+        else
+          this.createNode_(extension);
+      }, this);
+
+      // Delete any node that was not updated
+      var toRemove = document.querySelectorAll(
+          '.extension-list-item-wrapper.to-remove');

[Dan Beam, 2015/02/11 20:09:43]

.extension-list-item-wrapper.to-remove[id]

[hcarmona, 2015/02/11 23:36:46]

Done.

+      for (var i = 0; i < toRemove.length; ++i) {
+        var node = toRemove[i];
+        if (node.id && node.parentElement)

[Dan Beam, 2015/02/11 20:09:43]

document.qSA() doesn't work without a parentElement, if I had to guess

[hcarmona, 2015/02/11 23:36:46]

Done.

+          node.parentElement.removeChild(node);

[not at google - send to devlin, 2015/02/11 19:25:35]

I think this is a bug. A NodeList (which is what querySelectorAll returns) is
live, so if you remove an element from the DOM it removes it from the list as
well (i.e. if you have 2 adjacent nodes which need to be removed - highly
unlikely - only the first will be). This is fixed with the code above.

[not at google - send to devlin, 2015/02/11 19:29:30]

Ok, apparenetly querySelectorAll returns a "static NodeList" in this case, so
you're good, and don't need to take the .slice() copy first.

[hcarmona, 2015/02/11 23:36:47]

Acknowledged.

+      }
 
       var idToHighlight = this.getIdQueryParam_();
       if (idToHighlight && $(idToHighlight))
         this.scrollToNode_(idToHighlight);
 
       var idToOpenOptions = this.getOptionsQueryParam_();
       if (idToOpenOptions && $(idToOpenOptions))
         this.showEmbeddedExtensionOptions_(idToOpenOptions, true);
 
       var noExtensions = this.data_.extensions.length == 0;
@@ skipping 21 matching lines @@
      * given in |extension|.
      * @param {ExtensionData} extension A dictionary of extension metadata.
      * @private
      */
     createNode_: function(extension) {
       var template = $('template-collection').querySelector(
           '.extension-list-item-wrapper');
       var node = template.cloneNode(true);
       node.id = extension.id;
 
-      if (!extension.enabled || extension.terminated)
-        node.classList.add('inactive-extension');
+      // The 'Show Browser Action' button.
+      this.addListener_('click', node, '.show-button', function(e) {
+        chrome.send('extensionSettingsShowButton', [extension.id]);
+      });
 
+      // The 'allow in incognito' checkbox.
+      this.addListener_('change', node, '.incognito-control input',
+                        function(e) {
+        var butterBar = node.querySelector('.butter-bar');
+        var checked = e.target.checked;
+        if (!this.butterbarShown_) {
+          butterBar.hidden = !checked || extension.is_hosted_app;
+          this.butterbarShown_ = !butterBar.hidden;
+        } else
+          butterBar.hidden = true;
+        chrome.send('extensionSettingsEnableIncognito',
+                    [extension.id, String(checked)]);
+      }.bind(this));
+
+      // The 'collect errors' checkbox. This should only be visible if the
+      // error console is enabled - we can detect this by the existence of the
+      // |errorCollectionEnabled| property.
+      this.addListener_('change', node, '.error-collection-control input',
+                        function(e) {
+        chrome.send('extensionSettingsEnableErrorCollection',
+                    [extension.id, String(e.target.checked)]);
+      });
+
+      // The 'allow on all urls' checkbox. This should only be visible if
+      // active script restrictions are enabled. If they are not enabled, no
+      // extensions should want all urls.
+      this.addListener_('click', node, '.all-urls-control', function(e) {
+        chrome.send('extensionSettingsAllowOnAllUrls',
+                    [extension.id, String(e.target.checked)]);
+      });
+
+      // The 'allow file:// access' checkbox.
+      this.addListener_('click', node, '.file-access-control', function(e) {
+        chrome.send('extensionSettingsAllowFileAccess',
+                    [extension.id, String(e.target.checked)]);
+      });
+
+      // The 'Options' button or link, depending on its behaviour.
+      // Set an href to get the correct mouse-over appearance (link,
+      // footer) - but the actual link opening is done through chrome.send
+      // with a preventDefault().
+      node.querySelector('.options-link').setAttribute(

[Dan Beam, 2015/02/11 20:09:43]

nit: `.href =` is shorter

[hcarmona, 2015/02/11 23:36:46]

Done.

+          'href', extension.optionsPageHref);
+      this.addListener_('click', node, '.options-link', function(e) {
+        chrome.send('extensionSettingsOptions', [extension.id]);
+        e.preventDefault();
+      });
+
+      this.addListener_('click', node, '.options-button', function(e) {
+        this.showEmbeddedExtensionOptions_(extension.id, false);
+        e.preventDefault();
+      }.bind(this));
+
+      // The 'Permissions' link.
+      this.addListener_('click', node, '.permissions-link', function(e) {
+        chrome.send('extensionSettingsPermissions', [extension.id]);
+        e.preventDefault();
+      });
+
+      // The 'Reload' link.
+      this.addListener_('click', node, '.reload-link', function(e) {
+        chrome.send('extensionSettingsReload', [extension.id]);
+        extensionReloadedTimestamp[extension.id] = Date.now();
+      });
+
+      // The 'Launch' link.
+      this.addListener_('click', node, '.launch-link', function(e) {
+        chrome.send('extensionSettingsLaunch', [extension.id]);
+      });
+
+      // The 'Reload' terminated link.
+      this.addListener_('click', node, '.terminated-reload-link', function(e) {
+        chrome.send('extensionSettingsReload', [extension.id]);
+      });
+
+      // The 'Repair' corrupted link.
+      this.addListener_('click', node, '.corrupted-repair-button', function(e) {
+        chrome.send('extensionSettingsRepair', [extension.id]);
+      });
+
+      // The 'Enabled' checkbox.
+      this.addListener_('click', node, '.enable-checkbox input', function(e) {

[Dan Beam, 2015/02/11 20:09:43]

why click and not 'change'?

[hcarmona, 2015/02/11 23:36:47]

Done.

+        var checked = e.target.checked;
+        chrome.send('extensionSettingsEnable', [extension.id, String(checked)]);
+
+        // This may seem counter-intuitive (to not set/clear the checkmark)
+        // but this page will be updated asynchronously if the extension
+        // becomes enabled/disabled. It also might not become enabled or
+        // disabled, because the user might e.g. get prompted when enabling
+        // and choose not to.
+        e.preventDefault();
+      });
+
+      // 'Remove' button.
+      var trashTemplate = $('template-collection').querySelector('.trash');
+      var trash = trashTemplate.cloneNode(true);
+      trash.title = loadTimeData.getString('extensionUninstall');
+      trash.addEventListener('click', function(e) {
+        chrome.send('extensionSettingsUninstall', [extension.id]);
+      });
+      node.querySelector('.enable-controls').appendChild(trash);
+
+      // Developer mode ////////////////////////////////////////////////////////
+
+      // The path, if provided by unpacked extension.
+      this.addListener_('click', node, '.load-path a:nth-of-type(1)',

[Dan Beam, 2015/02/11 20:09:43]

:first-of-type

[hcarmona, 2015/02/11 23:36:47]

Done.

+                        function(e) {
+        chrome.send('extensionSettingsShowPath', [String(extension.id)]);
+        e.preventDefault();
+      });
+
+      this.appendChild(node);
+      this.updateNode_(extension, node);
+    },
+
+    /**
+     * Updates an HTML element for the extension metadata given in |extension|.
+     * @param {ExtensionData} extension A dictionary of extension metadata.
+     * @param {Element} node The node that is being updated.
+     * @private
+     */
+    updateNode_: function(extension, node) {
+      node.classList.remove('to-remove');
+      node.classList.toggle('inactive-extension',
+                            !extension.enabled || extension.terminated);
+
+      var toRemove = ['policy-controlled', 'may-not-modify', 'may-not-remove'];
+      node.classList.remove.apply(node.classList, toRemove);

[Dan Beam, 2015/02/11 20:09:43]

node.classList.remove('policy-controlled', 'may-not-modify', 'may-not-remove');

[hcarmona, 2015/02/11 23:36:47]

Done.

       var classes = [];
       if (extension.managedInstall) {
         classes.push('policy-controlled', 'may-not-modify');
       } else if (extension.dependentExtensions.length > 0) {
         classes.push('may-not-remove', 'may-not-modify');
       } else if (extension.recommendedInstall) {
         classes.push('may-not-remove');
       } else if (extension.suspiciousInstall ||
                  extension.corruptInstall ||
                  extension.updateRequiredByPolicy) {
         classes.push('may-not-modify');
       }
       node.classList.add.apply(node.classList, classes);
 
-      var idToHighlight = this.getIdQueryParam_();
-      if (node.id == idToHighlight)
-        node.classList.add('extension-highlight');
+      node.classList.toggle('extension-highlight',
+                            node.id == this.getIdQueryParam_());
 
       var item = node.querySelector('.extension-list-item');
       // Prevent the image cache of extension icon by using the reloaded
       // timestamp as a query string. The timestamp is recorded when the user
       // clicks the 'Reload' link. http://crbug.com/159302.
       if (extensionReloadedTimestamp[extension.id]) {
         item.style.backgroundImage =
             'url(' + extension.icon + '?' +
             extensionReloadedTimestamp[extension.id] + ')';
       } else {
         item.style.backgroundImage = 'url(' + extension.icon + ')';
       }
 
-      var title = node.querySelector('.extension-title');
-      title.textContent = extension.name;
-
-      var version = node.querySelector('.extension-version');
-      version.textContent = extension.version;
-
-      var locationText = node.querySelector('.location-text');
-      locationText.textContent = extension.locationText;
-
-      var blacklistText = node.querySelector('.blacklist-text');
-      blacklistText.textContent = extension.blacklistText;
-
-      var description = document.createElement('span');
-      description.textContent = extension.description;
-      node.querySelector('.extension-description').appendChild(description);
+      this.setTextContent_(node, '.extension-title', extension.name);
+      this.setTextContent_(node, '.extension-version', extension.version);
+      this.setTextContent_(node, '.location-text', extension.locationText);
+      this.setTextContent_(node, '.blacklist-text', extension.blacklistText);
+      this.setTextContent_(node, '.extension-description',
+                           extension.description);
 
       // The 'Show Browser Action' button.
-      if (extension.enable_show_button) {
-        var showButton = node.querySelector('.show-button');
-        showButton.addEventListener('click', function(e) {
-          chrome.send('extensionSettingsShowButton', [extension.id]);
-        });
-        showButton.hidden = false;
-      }
+      this.updateVisibility_(node, '.show-button',
+                             extension.enable_show_button);
 
       // The 'allow in incognito' checkbox.
-      node.querySelector('.incognito-control').hidden =
-          !this.data_.incognitoAvailable;
-      var incognito = node.querySelector('.incognito-control input');
-      incognito.disabled = !extension.incognitoCanBeEnabled;
-      incognito.checked = extension.enabledIncognito;
-      if (!incognito.disabled) {
-        incognito.addEventListener('change', function(e) {
-          var checked = e.target.checked;
-          butterBarVisibility[extension.id] = checked;
-          butterBar.hidden = !checked || extension.is_hosted_app;
-          chrome.send('extensionSettingsEnableIncognito',
-                      [extension.id, String(checked)]);
-        });
-      }
+      this.updateVisibility_(node, '.incognito-control',
+                             this.data_.incognitoAvailable, function(item) {
+        var incognito = item.querySelector('input');
+        incognito.disabled = !extension.incognitoCanBeEnabled;
+        incognito.checked = extension.enabledIncognito;
+      });
+
+      // Hide butterBar if incognito is not enabled for the extension.
       var butterBar = node.querySelector('.butter-bar');
-      butterBar.hidden = !butterBarVisibility[extension.id];
+      butterBar.hidden = butterBar.hidden || !extension.enabledIncognito;
 
       // The 'collect errors' checkbox. This should only be visible if the
       // error console is enabled - we can detect this by the existence of the
       // |errorCollectionEnabled| property.
-      if (extension.wantsErrorCollection) {
-        node.querySelector('.error-collection-control').hidden = false;
-        var errorCollection =
-            node.querySelector('.error-collection-control input');
-        errorCollection.checked = extension.errorCollectionEnabled;
-        errorCollection.addEventListener('change', function(e) {
-          chrome.send('extensionSettingsEnableErrorCollection',
-                      [extension.id, String(e.target.checked)]);
-        });
-      }
+      this.updateVisibility_(node, '.error-collection-control',
+                             extension.wantsErrorCollection, function(item) {
+        item.querySelector('input').checked = extension.errorCollectionEnabled;
+      });
 
       // The 'allow on all urls' checkbox. This should only be visible if
       // active script restrictions are enabled. If they are not enabled, no
       // extensions should want all urls.
-      if (extension.showAllUrls) {
-        var allUrls = node.querySelector('.all-urls-control');
-        allUrls.addEventListener('click', function(e) {
-          chrome.send('extensionSettingsAllowOnAllUrls',
-                      [extension.id, String(e.target.checked)]);
-        });
-        allUrls.querySelector('input').checked = extension.allowAllUrls;
-        allUrls.hidden = false;
-      }
+      this.updateVisibility_(node, '.all-urls-control', extension.showAllUrls,
+                             function(item) {
+        item.querySelector('input').checked = extension.allowAllUrls;
+      });
 
       // The 'allow file:// access' checkbox.
-      if (extension.wantsFileAccess) {
-        var fileAccess = node.querySelector('.file-access-control');
-        fileAccess.addEventListener('click', function(e) {
-          chrome.send('extensionSettingsAllowFileAccess',
-                      [extension.id, String(e.target.checked)]);
-        });
-        fileAccess.querySelector('input').checked = extension.allowFileAccess;
-        fileAccess.hidden = false;
-      }
+      this.updateVisibility_(node, '.file-access-control',
+                             extension.wantsFileAccess, function(item) {
+        item.querySelector('input').checked = extension.allowFileAccess;
+      });
 
       // The 'Options' button or link, depending on its behaviour.
-      if (extension.enabled && extension.optionsUrl) {
-        var options, optionsClickListener;
-        if (extension.optionsOpenInTab) {
-          options = node.querySelector('.options-link');
-          // Set an href to get the correct mouse-over appearance (link,
-          // footer) - but the actual link opening is done through chrome.send
-          // with a preventDefault().
-          options.setAttribute('href', extension.optionsPageHref);
-          optionsClickListener = function() {
-            chrome.send('extensionSettingsOptions', [extension.id]);
-          };
-        } else {
-          options = node.querySelector('.options-button');
-          optionsClickListener = function() {
-            this.showEmbeddedExtensionOptions_(extension.id, false);
-          }.bind(this);
-        }
-        options.addEventListener('click', function(e) {
-          optionsClickListener();
-          e.preventDefault();
-        });
-        options.hidden = false;
-      }
+      var optionsEnabled = extension.enabled && extension.optionsUrl;
+      this.updateVisibility_(node, '.options-link', optionsEnabled &&
+                             extension.optionsOpenInTab);
+      this.updateVisibility_(node, '.options-button', optionsEnabled &&
+                             !extension.optionsOpenInTab);
 
-      // The 'Permissions' link.
-      var permissions = node.querySelector('.permissions-link');
-      permissions.addEventListener('click', function(e) {
-        chrome.send('extensionSettingsPermissions', [extension.id]);
-        e.preventDefault();
+      // The 'View in Web Store/View Web Site' link.
+      var siteLinkEnabled = extension.homepageUrl &&
+                            !extension.enableExtensionInfoDialog;
+      this.updateVisibility_(node, '.site-link', siteLinkEnabled,
+                             function(item) {
+        item.href = extension.homepageUrl;
+        item.textContent = loadTimeData.getString(
+            extension.homepageProvided ? 'extensionSettingsVisitWebsite' :
+                                         'extensionSettingsVisitWebStore');
       });
 
-      // The 'View in Web Store/View Web Site' link.
-      if (extension.homepageUrl && !extension.enableExtensionInfoDialog) {
-        var siteLink = node.querySelector('.site-link');
-        siteLink.href = extension.homepageUrl;
-        siteLink.textContent = loadTimeData.getString(
-                extension.homepageProvided ? 'extensionSettingsVisitWebsite' :
-                                             'extensionSettingsVisitWebStore');
-        siteLink.hidden = false;
-      }
+      // The 'Reload' link.
+      this.updateVisibility_(node, '.reload-link', extension.allow_reload);
 
-      if (extension.allow_reload) {
-        // The 'Reload' link.
-        var reload = node.querySelector('.reload-link');
-        reload.addEventListener('click', function(e) {
-          chrome.send('extensionSettingsReload', [extension.id]);
-          extensionReloadedTimestamp[extension.id] = Date.now();
-        });
-        reload.hidden = false;
+      // The 'Launch' link.
+      this.updateVisibility_(node, '.launch-link', extension.allow_reload &&
+                             extension.is_platform_app);
 
-        if (extension.is_platform_app) {
-          // The 'Launch' link.
-          var launch = node.querySelector('.launch-link');
-          launch.addEventListener('click', function(e) {
-            chrome.send('extensionSettingsLaunch', [extension.id]);
-          });
-          launch.hidden = false;
-        }
-      }
+      // The 'Reload' terminated link.
+      var isTerminated = extension.terminated;
+      this.updateVisibility_(node, '.terminated-reload-link', isTerminated);
 
-      if (extension.terminated) {
-        var terminatedReload = node.querySelector('.terminated-reload-link');
-        terminatedReload.hidden = false;
-        terminatedReload.onclick = function() {
-          chrome.send('extensionSettingsReload', [extension.id]);
-        };
-      } else if (extension.corruptInstall && extension.isFromStore) {
-        var repair = node.querySelector('.corrupted-repair-button');
-        repair.hidden = false;
-        repair.onclick = function() {
-          chrome.send('extensionSettingsRepair', [extension.id]);
-        };
-      } else {
-        // The 'Enabled' checkbox.
-        var enable = node.querySelector('.enable-checkbox');
-        enable.hidden = false;
+      // The 'Repair' corrupted link.
+      var canRepair = !isTerminated && extension.corruptInstall &&
+                      extension.isFromStore;
+      this.updateVisibility_(node, '.corrupted-repair-button', canRepair);
+
+      // The 'Enabled' checkbox.
+      var isOK = !isTerminated && !canRepair;
+      this.updateVisibility_(node, '.enable-checkbox', isOK, function(item) {
         var enableCheckboxDisabled = extension.managedInstall ||
                                      extension.suspiciousInstall ||
                                      extension.corruptInstall ||
                                      extension.updateRequiredByPolicy ||
                                      extension.dependentExtensions.length > 0;
-        enable.querySelector('input').disabled = enableCheckboxDisabled;
+        item.querySelector('input').disabled = enableCheckboxDisabled;
+        item.querySelector('input').checked = extension.enabled;
+      });
 
-        if (extension.managedInstall) {
-          var indicator = new cr.ui.ControlledIndicator();
-          indicator.classList.add('controlled-extension-indicator');
-          indicator.setAttribute('controlled-by', 'policy');
-          var textPolicy = extension.policyText || '';
-          indicator.setAttribute('textpolicy', textPolicy);
-          indicator.image.setAttribute('aria-label', textPolicy);
-          node.querySelector('.enable-controls').appendChild(indicator);
-        }
+      // Button for extensions controlled by policy.
+      var controlNode = node.querySelector('.enable-controls');
+      var indicator =
+          controlNode.querySelector('.controlled-extension-indicator');
+      var needsIndicator = isOK && extension.managedInstall;
 
-        if (!enableCheckboxDisabled) {
-          enable.addEventListener('click', function(e) {
-            // When e.target is the label instead of the checkbox, it doesn't
-            // have the checked property and the state of the checkbox is
-            // left unchanged.
-            var checked = e.target.checked;
-            if (checked == undefined)
-              checked = !e.currentTarget.querySelector('input').checked;
-            chrome.send('extensionSettingsEnable',
-                        [extension.id, checked ? 'true' : 'false']);
-
-            // This may seem counter-intuitive (to not set/clear the checkmark)
-            // but this page will be updated asynchronously if the extension
-            // becomes enabled/disabled. It also might not become enabled or
-            // disabled, because the user might e.g. get prompted when enabling
-            // and choose not to.
-            e.preventDefault();
-          });
-        }
-
-        enable.querySelector('input').checked = extension.enabled;
-      }
-
-      // 'Remove' button.
-      var trashTemplate = $('template-collection').querySelector('.trash');
-      var trash = trashTemplate.cloneNode(true);
-      trash.title = loadTimeData.getString('extensionUninstall');
-      trash.addEventListener('click', function(e) {
-        butterBarVisibility[extension.id] = false;
-        chrome.send('extensionSettingsUninstall', [extension.id]);
-      });
-      node.querySelector('.enable-controls').appendChild(trash);
+      if (needsIndicator && !indicator) {
+        indicator = new cr.ui.ControlledIndicator();
+        indicator.classList.add('controlled-extension-indicator');
+        indicator.setAttribute('controlled-by', 'policy');
+        var textPolicy = extension.policyText || '';
+        indicator.setAttribute('textpolicy', textPolicy);
+        indicator.image.setAttribute('aria-label', textPolicy);
+        controlNode.appendChild(indicator);
+      } else if (!needsIndicator && indicator)
+        controlNode.removeChild(indicator);
 
       // Developer mode ////////////////////////////////////////////////////////
 
       // First we have the id.
       var idLabel = node.querySelector('.extension-id');
       idLabel.textContent = ' ' + extension.id;
 
       // Then the path, if provided by unpacked extension.
-      if (extension.isUnpacked) {
-        var loadPath = node.querySelector('.load-path');
-        loadPath.hidden = false;
-        var pathLink = loadPath.querySelector('a:nth-of-type(1)');
-        pathLink.textContent = ' ' + extension.prettifiedPath;
-        pathLink.addEventListener('click', function(e) {
-          chrome.send('extensionSettingsShowPath', [String(extension.id)]);
-          e.preventDefault();
-        });
-      }
+      this.updateVisibility_(node, '.load-path', extension.isUnpacked,
+                             function(item) {
+        item.querySelector('a:nth-of-type(1)').textContent =

[Dan Beam, 2015/02/11 20:09:43]

:first-of-type

[hcarmona, 2015/02/11 23:36:46]

Done.

+            ' ' + extension.prettifiedPath;
+      });
 
       // Then the 'managed, cannot uninstall/disable' message.
-      if (extension.managedInstall || extension.recommendedInstall) {
-        node.querySelector('.managed-message').hidden = false;
-      } else {
-        if (extension.suspiciousInstall) {
-          // Then the 'This isn't from the webstore, looks suspicious' message.
-          node.querySelector('.suspicious-install-message').hidden = false;
-        }
-        if (extension.corruptInstall) {
-          // Then the 'This is a corrupt extension' message.
-          node.querySelector('.corrupt-install-message').hidden = false;
-        }
-      }
+      // We would like to hide managed installed message since this
+      // extension is disabled.
+      var isRequired = extension.managedInstall || extension.recommendedInstall;
+      this.updateVisibility_(node, '.managed-message', isRequired &&
+                             !extension.updateRequiredByPolicy);
+
+      // Then the 'This isn't from the webstore, looks suspicious' message.
+      this.updateVisibility_(node, '.suspicious-install-message', !isRequired &&
+                             extension.suspiciousInstall);
+
+      // Then the 'This is a corrupt extension' message.
+      this.updateVisibility_(node, '.corrupt-install-message', !isRequired &&
+                             extension.corruptInstall);
 
       // Then the 'An update required by enterprise policy' message. Note that
       // a force-installed extension might be disabled due to being outdated
       // as well.
-      if (extension.updateRequiredByPolicy) {
-        node.querySelector('.update-required-message').hidden = false;
-        // We would like to hide managed installed message since this
-        // extension is disabled.
-        node.querySelector('.managed-message').hidden = true;
-      }
+      this.updateVisibility_(node, '.update-required-message',
+                             extension.updateRequiredByPolicy);
 
-      if (extension.dependentExtensions.length > 0) {
-        node.classList.add('developer-extras');
-        var dependentMessage =
-            node.querySelector('.dependent-extensions-message');
-        dependentMessage.hidden = false;
-        var dependentList = dependentMessage.querySelector('ul');
+      // The 'following extensions depend on this extension' list.
+      var hasDependents = extension.dependentExtensions.length > 0;
+      node.classList.toggle('developer-extras', hasDependents);
+      this.updateVisibility_(node, '.dependent-extensions-message',
+                             hasDependents, function(item) {
+        var dependentList = item.querySelector('ul');
+        dependentList.textContent = '';
         var dependentTemplate = $('template-collection').querySelector(
             '.dependent-list-item');
         extension.dependentExtensions.forEach(function(elem) {
           var depNode = dependentTemplate.cloneNode(true);
           depNode.querySelector('.dep-extension-title').textContent = elem.name;
           depNode.querySelector('.dep-extension-id').textContent = elem.id;
           dependentList.appendChild(depNode);
         });
-      }
+      });
 
-      // Then active views.
-      if (extension.views.length > 0) {
-        var activeViews = node.querySelector('.active-views');
-        activeViews.hidden = false;
-        var link = activeViews.querySelector('a');
+      // The active views.
+      this.updateVisibility_(node, '.active-views', extension.views.length > 0,
+                             function(item) {
+        var link = item.querySelector('a');
+
+        // Link needs to be an only child before the list is updated.
+        while (link.nextElementSibling)
+          item.removeChild(link.nextElementSibling);
+
+        // Link needs to be cleaned up if it was used before.
+        link.textContent = '';
+        if (link.clickHandler)
+          link.removeEventListener('click', link.clickHandler);
 
         extension.views.forEach(function(view, i) {
           var displayName = view.generatedBackgroundPage ?
               loadTimeData.getString('backgroundPage') : view.path;
           var label = displayName +
               (view.incognito ?
                   ' ' + loadTimeData.getString('viewIncognito') : '') +
               (view.renderProcessId == -1 ?
                   ' ' + loadTimeData.getString('viewInactive') : '');
           link.textContent = label;
-          link.addEventListener('click', function(e) {
+          link.clickHandler = function(e) {
             // TODO(estade): remove conversion to string?
             chrome.send('extensionSettingsInspect', [
               String(extension.id),
               String(view.renderProcessId),
               String(view.renderViewId),
               view.incognito
             ]);
-          });
+          };
+          link.addEventListener('click', link.clickHandler);
 
           if (i < extension.views.length - 1) {
             link = link.cloneNode(true);
-            activeViews.appendChild(link);
+            item.appendChild(link);
           }
         });
-      }
+      });
 
       // The extension warnings (describing runtime issues).
-      if (extension.warnings) {
-        var panel = node.querySelector('.extension-warnings');
-        panel.hidden = false;
-        var list = panel.querySelector('ul');
+      this.updateVisibility_(node, '.extension-warnings', extension.warnings,
+                             function(item) {
+        var warningList = item.querySelector('ul');
+        warningList.textContent = '';
         extension.warnings.forEach(function(warning) {
-          list.appendChild(document.createElement('li')).innerText = warning;
+          var li = document.createElement('li');
+          warningList.appendChild(li).innerText = warning;
         });
-      }
+      });
 
       // If the ErrorConsole is enabled, we should have manifest and/or runtime
       // errors. Otherwise, we may have install warnings. We should not have
       // both ErrorConsole errors and install warnings.
-      if (extension.manifestErrors) {
-        var panel = node.querySelector('.manifest-errors');
-        panel.hidden = false;
-        panel.appendChild(new extensions.ExtensionErrorList(
-            extension.manifestErrors));
-      }
-      if (extension.runtimeErrors) {
-        var panel = node.querySelector('.runtime-errors');
-        panel.hidden = false;
-        panel.appendChild(new extensions.ExtensionErrorList(
-            extension.runtimeErrors));
-      }
-      if (extension.installWarnings) {
-        var panel = node.querySelector('.install-warnings');
-        panel.hidden = false;
-        var list = panel.querySelector('ul');
-        extension.installWarnings.forEach(function(warning) {
-          var li = document.createElement('li');
-          li.innerText = warning.message;
-          list.appendChild(li);
-        });
-      }
+      // Errors.
+      this.updateErrors_(node.querySelector('.manifest-errors'),
+                        extension.manifestErrors);
+      this.updateErrors_(node.querySelector('.runtime-errors'),
+                        extension.runtimeErrors);
 
-      this.appendChild(node);
+      // Install warnings.
+      this.updateVisibility_(node, '.install-warnings',
+                             extension.installWarnings, function(item) {
+        var installWarningList = item.querySelector('ul');
+        installWarningList.textContent = '';
+        if (extension.installWarnings) {
+          extension.installWarnings.forEach(function(warning) {
+            var li = document.createElement('li');
+            li.innerText = warning.message;
+            installWarningList.appendChild(li);
+          });
+        }
+      });
+
       if (location.hash.substr(1) == extension.id) {
         // Scroll beneath the fixed header so that the extension is not
         // obscured.
         var topScroll = node.offsetTop - $('page-header').offsetHeight;
         var pad = parseInt(window.getComputedStyle(node, null).marginTop, 10);
         if (!isNaN(pad))
           topScroll -= pad / 2;
         setScrollTopForDocument(document, topScroll);
       }
     },
 
     /**
+     * Adds a listener to an element.
+     * @param {string} type The type of listener to add.
+     * @param {Element} node Ancestor of the element specified by |query|.
+     * @param {string} query A query to select an element in |node|.
+     * @param {function({Event} e)} onclick The function that should be called
+     *     on click.
+     * @private
+     */
+    addListener_: function(type, node, query, onclick) {

[Dan Beam, 2015/02/11 20:09:43]

onclick -> handler

[hcarmona, 2015/02/11 23:36:47]

Done.

+      node.querySelector(query).addEventListener(type, onclick);
+    },
+
+    /**
+     * Updates an element's textContent.
+     * @param {Element} node Ancestor of the element specified by |query|.
+     * @param {string} query A query to select an element in |node|.
+     * @param {string} textContent
+     * @private
+     */
+    setTextContent_: function(node, query, textContent) {

[Dan Beam, 2015/02/11 20:09:43]

nit: setText_

[hcarmona, 2015/02/11 23:36:46]

Done.

+      node.querySelector(query).textContent = textContent;
+    },
+
+    /**
+     * Updates an element's visibility and calls |callback| if it is visible.
+     * @param {Element} node Ancestor of the element specified by |query|.
+     * @param {string} query A query to select an element in |node|.
+     * @param {boolean} visible Whether the element should be visible or not.
+     * @param {function({Element} item)} callback Callback if the element
+     *     is visible. |item| is the element specified by |query|.

[Dan Beam, 2015/02/11 20:09:43]

this is kind of an odd API...

[hcarmona, 2015/02/11 23:36:47]

Should we rename it?

+     * @private
+     */
+    updateVisibility_: function(node, query, visible, callback) {

[Dan Beam, 2015/02/11 20:09:43]

nit: showingCallback or shownCallback or something

[hcarmona, 2015/02/11 23:36:47]

Done.

+      var item = node.querySelector(query);

[Dan Beam, 2015/02/11 20:09:43]

nit: var item = assert(node.querySelector(query));

[hcarmona, 2015/02/11 23:36:47]

Done.

+      assert(item);
+      item.hidden = !visible;
+      if (visible && callback)
+        callback(item);
+    },
+
+    /**
+     * Updates an element to show a list of errors.
+     * @param {Element} panel An element to hold the errors.
+     * @param {?Array<RuntimeError>} errors The errors to be displayed.
+     * @private
+     */
+    updateErrors_: function(panel, errors) {
+      // TODO(hcarmona): Look into updating the ExtensionErrorList rather than
+      // rebuilding it every time.
+      panel.hidden = !errors;

[Dan Beam, 2015/02/11 20:09:43]

is this meant to only hide the pane if |errors| is null or if errors.length == 0
as well?

[hcarmona, 2015/02/11 23:36:47]

Acknowledged.

+      panel.textContent = '';
+      if (errors)

[Dan Beam, 2015/02/11 20:09:42]

i think this will work when errors == [].  do you want that?

[hcarmona, 2015/02/11 23:36:47]

Changed to hide the panels if there's a list of 0 errors as well, since it
doesn't make sense to create an ExtensionErrorList to show no errors.

+        panel.appendChild(new extensions.ExtensionErrorList(errors));
+    },
+
+    /**
      * Opens the extension options overlay for the extension with the given id.
      * @param {string} extensionId The id of extension whose options page should
      *     be displayed.
      * @param {boolean} scroll Whether the page should scroll to the extension
      * @private
      */
     showEmbeddedExtensionOptions_: function(extensionId, scroll) {
       if (this.optionsShown_)
         return;
 
@@ skipping 33 matching lines @@
       // TODO(dbeam): why do we need to focus <extensionoptions> before and
       // after its showing animation? Makes very little sense to me.
       overlay.setInitialFocus();
     },
   };
 
   return {
     ExtensionsList: ExtensionsList
   };
 });