Show alert failure for reloading unpacked extensions with bad manifest

chrome/browser/resources/extensions/extension_loader.js

 // Copyright 2014 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.
 
+window.addEventListener('beforeunload', function() {
+  chrome.send('extensionLoaderSetDisplayLoading');
+});
+
 cr.define('extensions', function() {
   'use strict';
 
   /**
    * Construct an ExtensionLoadError around the given |div|.
    * @param {HTMLDivElement} div The HTML div for the extension load error.
    * @constructor
    */
   function ExtensionLoadError(div) {
     div.__proto__ = ExtensionLoadError.prototype;
@@ skipping 23 matching lines @@
       this.reason_ = this.querySelector('#extension-load-error-reason');
 
       /**
        * The element which displays the manifest code.
        * @type {ExtensionCode}
        * @private
        */
       this.manifest_ = new extensions.ExtensionCode(
           this.querySelector('#extension-load-error-manifest'));
 
+      /**
+       * The element which displays information about additional errors.
+       * @type {HTMLPreElement}
+       * @private
+       */
+      this.additional_ = this.querySelector('#extension-load-error-additional');
+
+      /**
+       * An array of failures for keeping track of multiple active failures.
+       * @type {Array}

[Devlin, 2014/07/01 22:14:17]

Array of whats? (document)

[gpdavis, 2014/07/01 23:48:09]

An array of failures!  Just kidding.

Added in a Failure constructor and capitalized Failures in the comment to be
more precise.

+       * @private
+       */
+      this.failures_ = [];
+
       this.querySelector('#extension-load-error-retry-button').addEventListener(
           'click', function(e) {
-        chrome.send('extensionLoaderRetry');
-        this.hide_();
+        chrome.send('extensionLoaderRetry',
+                    [this.failures_[this.failures_.length - 1].path_]);

[Devlin, 2014/07/01 22:14:17]

We can't do this.  We deliberately do not allow WebUI to choose a file path to
load as a security measure.  File selection must be done in C++ only.

[gpdavis, 2014/07/01 23:48:10]

Rats.  I suppose I'll add a vector of file paths and an OnLoadSuccess to the
observer so that the cc file knows what path to load when it receives the
message.

+        this.remove_();
       }.bind(this));
 
       this.querySelector('#extension-load-error-give-up-button').
           addEventListener('click', function(e) {
-        this.hide_();
+        this.remove_();
       }.bind(this));
+
+      chrome.send('extensionLoaderDisplayFailures');
     },
 
     /**
-     * Display the load error to the user.
+     * Add a failure to failures_ array.  If there is already a displayed

[Devlin, 2014/07/01 22:14:17]

nit: one space after a period (Clint will never forget this one ;))

[gpdavis, 2014/07/01 23:48:10]

He most certainly has not ;) Done.

+     *    failure, display the additional failures element.
      * @param {string} path The path from which the extension was loaded.
      * @param {string} reason The reason the extension failed to load.
      * @param {string} manifest The manifest object, with highlighted regions.
      */
-    show: function(path, reason, manifest) {
-      this.path_.textContent = path;
-      this.reason_.textContent = reason;
+    add: function(path, reason, manifest) {
+      if (this.failures_.length == 1) {

[Devlin, 2014/07/01 22:14:17]

nit: no brackets around single-line if

[gpdavis, 2014/07/01 23:48:10]

Done.

+        this.additional_.hidden = false;
+      }
 
-      manifest.message = reason;
+      this.failures_.push({
+        path_: path,
+        reason_: reason,
+        manifest_: manifest
+      });
+
+      this.show_();
+    },
+
+    /**
+     * Remove a failure from failures_ array.  If this was the last failure,
+     *    hide the error UI.  If this was the last additional failure, hide
+     *    the additional failures UI.
+     * @private
+     */
+    remove_: function() {
+      this.failures_.pop();
+      if (this.failures_.length == 0) {
+        this.hidden = true;
+      } else {
+        if (this.failures_.length == 1) {

[Devlin, 2014/07/01 22:14:17]

nit: brackets.

[gpdavis, 2014/07/01 23:48:10]

Done.

+          this.additional_.hidden = true;
+        }
+        this.show_();
+      }
+    },
+
+    /**
+     * Display the load error to the user. The last failure gets its manifest
+     *    and error displayed, while additional failures have their path names

[Devlin, 2014/07/01 22:14:17]

I don't think we wrap like this for these comments... are you copying style from
elsewhere?

[gpdavis, 2014/07/01 23:48:10]

See line 176.  Should I do it a different way?

[Devlin, 2014/07/02 17:46:49]

I think that we only line-wrap for param declarations, etc, not for the full
function comment.

+     *    displayed in the additional failures element.
+     * @private
+     */
+    show_: function() {
+      var failure = this.failures_[this.failures_.length - 1];
+      this.path_.textContent = failure.path_;
+      this.reason_.textContent = failure.reason_;
+
+      failure.manifest_.message = failure.reason_;
       this.manifest_.populate(
-          manifest,
+          failure.manifest_,
           loadTimeData.getString('extensionLoadCouldNotLoadManifest'));
       this.hidden = false;
       this.manifest_.scrollToError();
-    },
 
-    /**
-     * Hide the extension load error.
-     * @private
-     */
-    hide_: function() {
-      this.hidden = true;
+      this.additional_.textContent = 'Additional failures:';

[Devlin, 2014/07/01 22:14:17]

this will still have to be i18n'd. ;)

[gpdavis, 2014/07/01 23:48:10]

How do in JS?

[gpdavis, 2014/07/02 00:49:36]

This was a premature request for help.  Got it figured out (I think).

+      for (var i = 0; i < this.failures_.length - 1; ++i) {
+        this.additional_.textContent += '\n' + this.failures_[i].path_;

[Devlin, 2014/07/01 22:14:17]

nit: no brackets around 1-line for loops

[gpdavis, 2014/07/01 23:48:09]

Done.

+      }
     }
   };
 
   /**
    * The ExtensionLoader is the class in charge of loading unpacked extensions.
    * @constructor
    */
   function ExtensionLoader() {
     /**
      * The ExtensionLoadError to show any errors from loading an unpacked
@@ skipping 10 matching lines @@
     /**
      * Begin the sequence of loading an unpacked extension. If an error is
      * encountered, this object will get notified via notifyFailed().
      */
     loadUnpacked: function() {
       chrome.send('extensionLoaderLoadUnpacked');
     },
 
     /**
      * Notify the ExtensionLoader that loading an unpacked extension failed.
-     * Show the ExtensionLoadError.
+     * Add the failure to failures_ and show the ExtensionLoadError.
      * @param {string} filePath The path to the unpacked extension.
      * @param {string} reason The reason the extension failed to load.
      * @param {Object} manifest An object with three strings: beforeHighlight,
      *     afterHighlight, and highlight. These represent three portions of the
      *     file's content to display - the portion which is most relevant and
      *     should be emphasized (highlight), and the parts both before and after
      *     this portion. These may be empty.
      */
     notifyFailed: function(filePath, reason, manifest) {
-      this.loadError_.show(filePath, reason, manifest);
-    }
+      this.loadError_.add(filePath, reason, manifest);
+    },
   };
 
   /*
    * A static forwarding function for ExtensionLoader.notifyFailed.
    * @param {string} filePath The path to the unpacked extension.
    * @param {string} reason The reason the extension failed to load.
    * @param {Object} manifest The manifest of the failed extension.
    * @see ExtensionLoader.notifyFailed
    */
   ExtensionLoader.notifyLoadFailed = function(filePath, reason, manifest) {
     ExtensionLoader.getInstance().notifyFailed(filePath, reason, manifest);
   };
 
   return {
     ExtensionLoader: ExtensionLoader
   };
 });