Fix Regression in Unpacked Extension Load Error UI

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.
 
 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;
     div.init();
     return div;
   }
 
-  /**
-   * Construct a Failure.

[not at google - send to devlin, 2014/07/25 16:03:10]

what was wrong with a Failure type?

[Devlin, 2014/07/25 16:15:41]

Well, depends on your semantics ;)  Since JS is "typeless" (I know it's not
really true), there *is* no failure type.  So, really, the problem was we had an
unused function (since we never once _constructed_ a failure.  All pertinent
documentation on the failure object was moved to the failures_ array comment.

[not at google - send to devlin, 2014/07/25 16:19:09]

if we never constructed a Failure that is a problem. we should construct
Failures instead of appending objects to |failures_| :)

JS resources are going to start being type-checked with closure (there is a
chromium-dev thread about it) so type information is nice.

[Devlin, 2014/07/25 16:22:44]

Well, the failure object is primarily constructed in C++ and sent to us as an
object with 3/4 params (filePath, error, manifest).  Would you prefer to
explicitly construct a failure from an existing object, instead of appending the
missing param?  Might be worth it, for the closure compiler, but also seems
kinda... wasteful.

[not at google - send to devlin, 2014/07/25 16:44:13]

Yes I think that would be better. why is it wasteful?

[Devlin, 2014/07/25 17:17:25]

Because of all the extra work we do to copy those expensive references into a
new object!! ;)  Mostly just because it looks a bit excessive in the code, to
me.  But I don't feel strongly.  Done.

-   * @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.
-   * @param {HTMLLIElement} listElement The HTML element used for displaying the
-   *     failure path for the additional failures UI.
-   * @constructor
-   */
-  function Failure(filePath, reason, manifest, listElement) {
-    this.path = filePath;
-    this.reason = reason;
-    this.manifest = manifest;
-    this.listElement = listElement;
-  }
-
   ExtensionLoadError.prototype = {
     __proto__: HTMLDivElement.prototype,
 
     /**
      * Initialize the ExtensionLoadError div.
      */
     init: function() {
       /**
        * The element which displays the path of the extension.
        * @type {HTMLSpanElement}
@@ skipping 19 matching lines @@
       /**
        * The element which displays information about additional errors.
        * @type {HTMLULElement}
        * @private
        */
       this.additional_ = this.querySelector('#extension-load-error-additional');
       this.additional_.list = this.additional_.getElementsByTagName('ul')[0];
 
       /**
        * An array of Failures for keeping track of multiple active failures.
-       * @type {Array.<Failure>}
+       * Each failure object contains the following:
+       * filePath (string) The path to the unpacked extension.
+       * error (string) The reason the extension failed to load.
+       * manifest (object) 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.
+       * listElement (HTMLLIElement) The HTML element used for displaying the
+       * @type {Array.<Object>}
        * @private
        */
       this.failures_ = [];
 
       this.querySelector('#extension-load-error-retry-button').addEventListener(
           'click', function(e) {
         chrome.send('extensionLoaderRetry');
         this.remove_();
       }.bind(this));
 
@@ skipping 50 matching lines @@
     /**
      * Display the load error to the user. The last failure gets its manifest
      * and error displayed, while additional failures have their path names
      * displayed in the additional failures element.
      * @private
      */
     show_: function() {
       assert(this.failures_.length >= 1);
       var failure = this.failures_[this.failures_.length - 1];
       this.path_.textContent = failure.path;
-      this.reason_.textContent = failure.reason;
+      this.reason_.textContent = failure.error;
 
-      failure.manifest.message = failure.reason;
+      failure.manifest.message = failure.error;
       this.manifest_.populate(
           failure.manifest,
           loadTimeData.getString('extensionLoadCouldNotLoadManifest'));
       this.hidden = false;
       this.manifest_.scrollToError();
 
       this.additional_.hidden = this.failures_.length == 1;
     }
   };
 
@@ skipping 40 matching lines @@
    * @see ExtensionLoader.notifyFailed
    */
   ExtensionLoader.notifyLoadFailed = function(failures) {
     ExtensionLoader.getInstance().notifyFailed(failures);
   };
 
   return {
     ExtensionLoader: ExtensionLoader
   };
 });