[Extensions] Update Error Console UI

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">
 
 ///////////////////////////////////////////////////////////////////////////////
 // ExtensionFocusRow:
 
 /**
@@ skipping 130 matching lines @@
   /**
    * Creates a new list of extensions.
    * @constructor
    * @extends {HTMLDivElement}
    */
   function ExtensionList() {
     var div = document.createElement('div');
     div.__proto__ = ExtensionList.prototype;
     /** @private {!Array<ExtensionInfo>} */
     div.extensions_ = [];
+    chrome.developerPrivate.onItemStateChanged.addListener(
+        div.onItemStateChanged_.bind(div));
     return div;
   }
 
   /**
    * @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 = {};
@@ skipping 280 matching lines @@
       row.setupColumn('.reload-link', 'localReload', 'click', function(e) {
         chrome.developerPrivate.reload(extension.id, {failQuietly: true});
         extensionReloadedTimestamp[extension.id] = Date.now();
       });
 
       // The 'Launch' link.
       row.setupColumn('.launch-link', 'launch', 'click', function(e) {
         chrome.send('extensionSettingsLaunch', [extension.id]);
       });
 
+      row.setupColumn('.errors-link', 'errors', 'click', function(e) {
+        var extensionId = extension.id;
+        var newEx = this.extensions_.filter(function(e) {

[Dan Beam, 2015/04/22 21:02:38]

what if this.extensions_ is empty?

[Devlin, 2015/04/22 23:17:32]

I don't think it can be - there'd be no node to create or click on.  Let's
assert it.

+          return e.state == chrome.developerPrivate.ExtensionState.ENABLED &&
+              e.id == extensionId;
+        })[0];
+        var errors = newEx.manifestErrors.concat(newEx.runtimeErrors);
+        extensions.ExtensionErrorOverlay.getInstance().setErrorsAndShowOverlay(
+            errors, extensionId, newEx.name);
+      }.bind(this));
+
       // The 'Reload' terminated link.
       row.setupColumn('.terminated-reload-link', 'terminatedReload', 'click',
                       function(e) {
         chrome.developerPrivate.reload(extension.id, {failQuietly: true});
       });
 
       // The 'Repair' corrupted link.
       row.setupColumn('.corrupted-repair-button', 'repair', 'click',
                       function(e) {
         chrome.send('extensionSettingsRepair', [extension.id]);
@@ skipping 179 matching lines @@
       var isUnpacked =
           extension.location == chrome.developerPrivate.Location.UNPACKED;
       // The 'Reload' link.
       this.updateVisibility_(row, '.reload-link', isUnpacked);
 
       // The 'Launch' link.
       this.updateVisibility_(
           row, '.launch-link',
           isUnpacked && extension.type ==
                             chrome.developerPrivate.ExtensionType.PLATFORM_APP);
 

[Dan Beam, 2015/04/22 21:02:38]

var errors = extension.runtimeErrors.length || extension.runtimeErrors.length;
this.updateVisibility_(row, '.errors-link', errors, function() {

[Devlin, 2015/04/22 23:17:32]

Done.

+      // The 'Errors' link.
+      this.updateVisibility_(
+          row, '.errors-link',
+          extension.runtimeErrors.length > 0 ||
+              extension.manifestErrors.length > 0,
+          function(item) {
+        var Level = chrome.developerPrivate.ErrorLevel;

[Dan Beam, 2015/04/28 03:47:03]

\n

[Devlin, 2015/04/29 16:08:37]

Done.

+        var map = {};
+        map[Level.LOG] = {weight: 0, name: 'extension-error-info-icon'};
+        map[Level.WARN] = {weight: 1, name: 'extension-error-warning-icon'};
+        map[Level.ERROR] = {weight: 2, name: 'extension-error-fatal-icon'};

[Dan Beam, 2015/04/28 03:47:03]

\n

[Devlin, 2015/04/29 16:08:37]

Done.

+        // Find the highest severity of all the errors; manifest errors all have
+        // a 'warning' level severity.
+        var highSeverity = extension.runtimeErrors.reduce(

[Dan Beam, 2015/04/28 03:47:03]

nit: highestSeverity or mostSevere

[Devlin, 2015/04/29 16:08:37]

Done.

+            function(prev, error) {
+          return map[error.severity].weight > map[prev].weight ?
+              error.severity : prev;
+        }, extension.manifestErrors.length ? Level.WARN : Level.LOG);

[Dan Beam, 2015/04/22 21:02:38]

i have no idea what this does

[Devlin, 2015/04/22 23:17:32]

Comment on line 675. :)

It goes through and looks for the error with the highest severity, passing in an
initial value of WARN if there's a manifest error (since all manifest errors
have WARN level) or LOG (the lowest) otherwise.

+
+        // Adjust the class on the icon.
+        var icon = item.querySelector('.extension-error-icon');
+        icon.className = 'extension-error-icon';

[Dan Beam, 2015/04/22 21:02:38]

and the point of this is... to remove other classes?

[Devlin, 2015/04/22 23:17:32]

Exactly.  We could also remove all the possibles, if that's more clear?
i.e.
Object.keys(map).forEach(function(key) {
  icon.classList.remove(map[key].name);
});
icon.classList.add(map[highSeverity].name);

Preferred?  Or option c?

[Dan Beam, 2015/04/28 03:47:03]

leave as is, maybe add "  // Remove other classes."

[Devlin, 2015/04/29 16:08:37]

Done.

+        icon.classList.add(map[highSeverity].name);
+      });
+
       // The 'Reload' terminated link.
       var isTerminated =
           extension.state == chrome.developerPrivate.ExtensionState.TERMINATED;
       this.updateVisibility_(row, '.terminated-reload-link', isTerminated);
 
       // The 'Repair' corrupted link.
       var canRepair = !isTerminated &&
                       extension.disableReasons.corruptInstall &&
                       extension.location ==
                           chrome.developerPrivate.Location.FROM_STORE;
@@ skipping 165 matching lines @@
                              extension.runtimeWarnings.length > 0,
                              function(item) {
         var warningList = item.querySelector('ul');
         warningList.textContent = '';
         extension.runtimeWarnings.forEach(function(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.
-      // Errors.
-      this.updateErrors_(row.querySelector('.manifest-errors'),
-                         'dev-manifestErrors', extension.manifestErrors);
-      this.updateErrors_(row.querySelector('.runtime-errors'),
-                         'dev-runtimeErrors', extension.runtimeErrors);
-
       // Install warnings.
       this.updateVisibility_(row, '.install-warnings',
                              extension.installWarnings.length > 0,
                              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;
@@ skipping 39 matching lines @@
      * @private
      */
     updateVisibility_: function(node, query, visible, opt_shownCallback) {
       var item = assert(node.querySelector(query));
       item.hidden = !visible;
       if (visible && opt_shownCallback)
         opt_shownCallback(item);
     },
 
     /**
-     * Updates an element to show a list of errors.
-     * @param {Element} panel An element to hold the errors.
-     * @param {string} columnType A tag used to identify the column when
-     *     changing focus.
-     * @param {Array<RuntimeError|ManifestError>|undefined} errors The errors
-     *     to be displayed.
-     * @private
-     */
-    updateErrors_: function(panel, columnType, errors) {
-      // TODO(hcarmona): Look into updating the ExtensionErrorList rather than
-      // rebuilding it every time.
-      panel.hidden = !errors || errors.length == 0;
-      panel.textContent = '';
-
-      if (panel.hidden)
-        return;
-
-      var errorList =
-          new extensions.ExtensionErrorList(assertInstanceof(errors, Array));
-
-      panel.appendChild(errorList);
-
-      var list = errorList.getErrorListElement();
-      if (list)
-        list.setAttribute('column-type', columnType + 'list');
-
-      var button = errorList.getToggleElement();
-      if (button)
-        button.setAttribute('column-type', columnType + 'button');
-    },
-
-    /**
      * 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 29 matching lines @@
       var self = this;
       $('overlay').addEventListener('cancelOverlay', function f() {
         self.optionsShown_ = false;
         $('overlay').removeEventListener('cancelOverlay', f);
       });
 
       // TODO(dbeam): why do we need to focus <extensionoptions> before and
       // after its showing animation? Makes very little sense to me.
       overlay.setInitialFocus();
     },
+
+    /**
+     * Handle an extension changing. Currently this only checks when errors are
+     * deleted.
+     * @param {EventData} e The event data for the extension change.
+     * @private
+     */
+    onItemStateChanged_: function(e) {
+      // TODO(devlin): We should be doing this for far more than just
+      // ERRORS_REMOVED, instead of re-generating all extension data whenever
+      // anything happens.
+      if (e.event_type == chrome.developerPrivate.EventType.ERRORS_REMOVED &&
+          e.info) {
+        var node = /** @type {ExtensionFocusRow} */($(e.info.id));
+        if (node)
+          this.updateNode_(e.info, node);
+      }
+    },
   };
 
   return {
     ExtensionList: ExtensionList
   };
 });