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

chrome/browser/resources/extensions/update_node.js

+// TODO(hcarmona): Remove this temporary file.
+
+    /**
+     * 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.toggle('inactive-extension',
+                            !extension.enabled || extension.terminated);
+
+      node.classList.toggle('policy-controlled', extension.managedInstall);
+
+      node.classList.toggle('may-not-remove',
+                            extension.dependentExtensions.length > 0 ||
+                            extension.recommendedInstall);
+
+      node.classList.toggle('may-not-modify',
+                            extension.managedInstall ||
+                            extension.dependentExtensions.length > 0 ||
+                            extension.suspiciousInstall ||
+                            extension.corruptInstall ||
+                            extension.updateRequiredByPolicy);
+
+      node.classList.toggle('extension-highlight',
+                            node.id == this.getIdQueryParam_());

[not at google - send to devlin, 2015/02/03 17:41:18]

What was wrong with the old class setup code?

[hcarmona, 2015/02/03 19:26:10]

This code will toggle the classes because the node is being updated, while the
previous code would only add to the classes.

If we want to reuse the previous code we can remove the classes that can be
added and then continue with previous logic that will add the necessary classes.

[not at google - send to devlin, 2015/02/03 19:33:40]

Yes I think that would be preferable, at least for now for the sake of reducing
the diff in logic.

[hcarmona, 2015/02/04 23:38:59]

Done.

+
+      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;
+      var descriptionHolder = node.querySelector('.extension-description');
+      descriptionHolder.textContent = '';
+      descriptionHolder.appendChild(description);
+
+      // The 'Show Browser Action' button.
+      var showButton = node.querySelector('.show-button');
+      this.toggleEventListener_(showButton, extension.enable_show_button,

[not at google - send to devlin, 2015/02/03 17:41:18]

Any reason why you need to toggle these listeners? Can you just show/hide the
elements, the event listeners won't fire if they're hidden.

[hcarmona, 2015/02/03 19:26:10]

Previous logic only adds listeners when they're going to be used. I read that
having a lot of unused listeners can make the code slow. If there's no
performance overhead when the elements are hidden, then we can move all the
listeners to the createNode_ function.

[not at google - send to devlin, 2015/02/03 19:33:40]

Maybe Dan can comment on that, but I feel the complexity added to support this
is non trivial, and we're talking about only a small number of listeners really
(plus, it's no worse than how it was before, right..?)

[Dan Beam, 2015/02/03 19:35:06]

i think unless there's a metrics-driven or anecdotal speed difference to back up
this claim, just write the simplest code possible.

[hcarmona, 2015/02/04 23:38:59]

Page feels snappier (compared to stable) when toggling between developer and
normal mode when there are several extensions due to not rebuilding the list.
Didn't notice any difference between toggling the event listener.

Code is less complicated now. :-)

+                                'click', showButton.showButtonHandler);
+      showButton.hidden = !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;
+      this.toggleEventListener_(incognito, !incognito.disabled,
+                                'change', incognito.changeHandler);
+      var butterBar = node.querySelector('.butter-bar');
+      butterBar.hidden = !butterBarVisibility[extension.id];
+
+      // 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.
+      node.querySelector('.error-collection-control').hidden =
+          !extension.wantsErrorCollection;
+      var errorCollection =
+          node.querySelector('.error-collection-control input');
+      errorCollection.checked = extension.wantsErrorCollection &&
+                                extension.errorCollectionEnabled;
+      this.toggleEventListener_(errorCollection, extension.wantsErrorCollection,
+                                'change', errorCollection.changeHandler);
+
+      // 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.
+      var allUrls = node.querySelector('.all-urls-control');
+      this.toggleEventListener_(allUrls, extension.wantsAllUrls,
+                                'click', allUrls.clickHandler);
+      allUrls.querySelector('input').checked = extension.wantsAllUrls &&
+                                               extension.allowAllUrls;
+      allUrls.hidden = !extension.wantsAllUrls;
+
+      // The 'allow file:// access' checkbox.
+      var fileAccess = node.querySelector('.file-access-control');
+      this.toggleEventListener_(fileAccess, extension.wantsFileAccess,
+                                'click', fileAccess.clickHandler);
+      fileAccess.querySelector('input').checked = extension.wantsFileAccess &&
+                                                  extension.allowFileAccess;
+      fileAccess.hidden = !extension.wantsFileAccess;
+
+      // The 'Options' button or link, depending on its behaviour.
+      var optionsEnabled = extension.enabled && extension.optionsUrl;
+
+      var optionsLink = node.querySelector('.options-link');
+      var optionsLinkEnabled = optionsEnabled && extension.optionsOpenInTab;
+      this.addEventListener(optionsLink, optionsLinkEnabled,
+                            'click', optionsLink.clickHandler);
+      optionsLink.hidden = !optionsLinkEnabled;
+
+      var optionsButton = options = node.querySelector('.options-button');
+      var optionsButtonEnabled = optionsEnabled && !extension.optionsOpenInTab;
+      this.addEventListener(optionsButton, optionsButtonEnabled,
+                            'click', optionsButton.clickHandler);
+      optionsButton.hidden = !optionsButtonEnabled;
+
+      // The 'View in Web Store/View Web Site' link.
+      var siteLinkEnabled = extension.homepageUrl &&
+                            !extension.enableExtensionInfoDialog;
+      var siteLink = node.querySelector('.site-link');
+      siteLink.href = extension.homepageUrl;
+      siteLink.textContent = loadTimeData.getString(
+          extension.homepageProvided ? 'extensionSettingsVisitWebsite' :
+                                       'extensionSettingsVisitWebStore');
+      siteLink.hidden = !siteLinkEnabled;
+
+      // The 'Reload' link.
+      var reload = node.querySelector('.reload-link');
+      this.toggleEventListener_(reload, extension.allow_reload,
+                                'click', reload.clickHandler);
+      reload.hidden = !extension.allow_reload;
+
+      var canLaunch = extension.allow_reload && extension.is_platform_app;
+      // The 'Launch' link.
+      var launch = node.querySelector('.launch-link');
+      this.toggleEventListener_(launch, canLaunch,
+                                'click', launch.clickHandler);
+      launch.hidden = !canLaunch;
+
+      // The 'Reload' terminated link.
+      var isTerminated = extension.terminated;
+      var terminatedReload = node.querySelector('.terminated-reload-link');
+      this.toggleEventListener_(terminatedReload, isTerminated,
+                                'click', terminatedReload.clickHandler);
+      terminatedReload.hidden = !isTerminated;
+
+      // The 'Repair' corrupted link.
+      var canRepair = !isTerminated && extension.corruptInstall &&
+                      extension.isFromStore;
+      var repair = node.querySelector('.corrupted-repair-button');
+      this.toggleEventListener_(repair, canRepair,
+                                'click', repair.clickHandler);
+      repair.hidden = !canRepair;
+
+      // The 'Enabled' checkbox.
+      var isOK = !isTerminated && !canRepair;
+      var enable = node.querySelector('.enable-checkbox');
+      enable.hidden = !isOK;
+      var enableCheckboxDisabled = extension.managedInstall ||
+                                   extension.suspiciousInstall ||
+                                   extension.corruptInstall ||
+                                   extension.updateRequiredByPolicy ||
+                                   extension.dependentExtensions.length > 0;
+      enable.querySelector('input').disabled = enableCheckboxDisabled;
+      this.toggleEventListener_(enable, isOK && !enableCheckboxDisabled,
+                                'click', enable.clickHandler);
+
+      enable.querySelector('input').checked = isOK && extension.enabled;
+
+      // Button for extensions controlled by policy.
+      var controlNode = node.querySelector('.enable-controls');
+      if (extension.managedInstall &&
+          !controlNode.querySelector('.controlled-extension-indicator')) {
+        var indicator = new cr.ui.ControlledIndicator();
+        indicator.classList.add('controlled-extension-indicator');
+        indicator.setAttribute('controlled-by', 'policy');
+        indicator.setAttribute('textpolicy', extension.policyText || '');
+        controlNode.appendChild(indicator);
+      } else if (!extension.managedInstall) {
+        var indicator = controlNode.querySelector(
+            '.controlled-extension-indicator');
+        if (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.
+      var loadPath = node.querySelector('.load-path');
+      loadPath.hidden = !extension.isUnpacked;
+      var pathLink = loadPath.querySelector('a:nth-of-type(1)');
+      pathLink.textContent = ' ' + extension.prettifiedPath;
+      this.toggleEventListener_(pathLink, extension.isUnpacked,
+                                'click', pathLink.clickHandler);
+
+      // Then the 'managed, cannot uninstall/disable' message.
+      var isRequired = extension.managedInstall || extension.recommendedInstall;
+      node.querySelector('.managed-message').hidden = !isRequired;
+
+      // Then the 'This isn't from the webstore, looks suspicious' message.
+      var isSuspicious = !isRequired && extension.suspiciousInstall;
+      node.querySelector('.suspicious-install-message').hidden = !isSuspicious;
+
+      // Then the 'This is a corrupt extension' message.
+      var isCorrupt = !isRequired && extension.corruptInstall;
+      node.querySelector('.corrupt-install-message').hidden = !isCorrupt;
+
+      // Then the 'An update required by enterprise policy' message. Note that
+      // a force-installed extension might be disabled due to being outdated
+      // as well.
+      node.querySelector('.update-required-message').hidden =
+          !extension.updateRequiredByPolicy;
+      // We would like to hide managed installed message since this
+      // extension is disabled.
+      node.querySelector('.managed-message').hidden =
+          extension.updateRequiredByPolicy;
+
+      // The 'following extensions depend on this extension' list.
+      var hasDependents = extension.dependentExtensions.length > 0;
+      node.classList.toggle('developer-extras', hasDependents);
+      var dependentMessage =
+          node.querySelector('.dependent-extensions-message');
+      dependentMessage.hidden = !hasDependents;
+      var dependentList = dependentMessage.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);
+      });
+
+      // The active views.
+      var hasActiveViews = extension.views.length > 0;
+      var activeViews = node.querySelector('.active-views');
+      activeViews.hidden = !hasActiveViews;
+
+      var link = activeViews.querySelector('a');
+
+      // Link needs to be an only child before the list is updated.
+      while (link.nextElementSibling)
+        activeViews.removeChild(link.nextElementSibling);
+
+      // Link needs to be cleaned up if it was used before.
+      link.textContent = '';
+      if (link.clickHandler)
+        link.removeEventListener('click', link.clickHandler);
+
+      if (hasActiveViews) {
+        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.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);
+          }
+        });
+      }
+
+      // The extension warnings (describing runtime issues).
+      var warningPanel = node.querySelector('.extension-warnings');
+      warningPanel.hidden = !extension.warnings;
+      var warningList = warningPanel.querySelector('ul');
+      warningList.textContent = '';
+      if (extension.warnings) {
+        extension.warnings.forEach(function(warning) {
+          warningList.appendChild(document.createElement('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.
+      // Errors.
+      this.updateErrors_(node.querySelector('.manifest-errors'),
+                        extension.manifestErrors);
+      this.updateErrors_(node.querySelector('.runtime-errors'),
+                        extension.runtimeErrors);
+      // Install warnings.
+      var installWarningPanel = node.querySelector('.install-warnings');
+      installWarningPanel.hidden = !extension.installWarnings;
+      var installWarningList = installWarningPanel.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);
+      }
+    },