Fix scroll regression when specifying an extension id.

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 217 matching lines @@
      */
     enableAppInfoDialog_: false,
 
     /**
      * Initializes the list.
      */
     initialize: function() {
       /** @private {!Array<ExtensionInfo>} */
       this.extensions_ = [];
 
+      /**
+       * |loadFinished| should be used for testing purposes and will be
+       * fulfilled when this list has finished loading the first time.
+       * @type {Promise}
+       * */
+      this.loadFinished = new Promise(function(resolve, reject) {
+        /** @private {function(?)} */
+        this.onLoadFinish_ = resolve;
+      }.bind(this));
+
       chrome.developerPrivate.onItemStateChanged.addListener(
           function(eventData) {
         var EventType = chrome.developerPrivate.EventType;
         switch (eventData.event_type) {
           case EventType.VIEW_REGISTERED:
           case EventType.VIEW_UNREGISTERED:
             // For now, view notifications are handled through the WebUI.
             // TODO(devlin): Transition these.
             break;
           case EventType.INSTALLED:
@@ skipping 30 matching lines @@
       this.extensionsUpdated_ = new Promise(function(resolve, reject) {
         chrome.developerPrivate.getExtensionsInfo(
             {includeDisabled: true, includeTerminated: true},
             function(extensions) {
           // Sort in order of unpacked vs. packed, followed by name, followed by
           // id.
           extensions.sort(compareExtensions);
           this.extensions_ = extensions;
           this.showExtensionNodes_();
           resolve();
+
+          // |resolve| is async so it's necessary to use |then| here in order to
+          // do work after other |then|s have finished.
+          this.extensionsUpdated_.then(function() {

[Dan Beam, 2015/04/18 01:28:40]

why not:

  this.loadFinished_ = this.extensionUpdated_.then(...);

?

[hcarmona, 2015/04/20 17:51:08]

|updateExtensionsData| is called async, so the test wouldn't have a
reference to the |loadFinished_| promise. Also, by assigning
|loadFinished_| in |initialize| the promise will only resolve once.

[Dan Beam, 2015/04/20 17:56:00]

ok, makes sense.  thanks for explanation.

+            // |onUpdateFinished_| should be called after the list becomes
+            // visible in extensions.js.
+            this.onUpdateFinished_();
+
+            // |onLoadFinish_| will resolve the |loadFinished| promise for
+            // testing purposes.
+            this.onLoadFinish_();
+          }.bind(this));
         }.bind(this));
       }.bind(this));
       return this.extensionsUpdated_;
     },
 
+    /** Updates elements that need to be visible in order to update properly. */

[Dan Beam, 2015/04/18 01:28:41]

/**
 * Updates ...
 * @private
 */

[hcarmona, 2015/04/20 17:51:08]

Done.

+    onUpdateFinished_: function() {
+      // Cannot focus or highlight a extension if there are none.
+      if (this.extensions_.length == 0)
+        return;
+
+      assert(!this.hidden);
+      assert(!this.parentElement.hidden);
+
+      this.updateFocusableElements();
+
+      var idToHighlight = this.getIdQueryParam_();
+      if (idToHighlight && $(idToHighlight))
+        this.scrollToNode_(idToHighlight);
+
+      var idToOpenOptions = this.getOptionsQueryParam_();
+      if (idToOpenOptions && $(idToOpenOptions))
+        this.showEmbeddedExtensionOptions_(idToOpenOptions, true);
+    },
+
     /** @return {number} The number of extensions being displayed. */
     getNumExtensions: function() {
       return this.extensions_.length;
     },
 
     getIdQueryParam_: function() {
       return parseQueryParams(document.location)['id'];
     },
 
     getOptionsQueryParam_: function() {
@@ skipping 29 matching lines @@
               focusableNode.getEquivalentElement(
                   document.activeElement).focus();
             }
           }
 
           node.parentElement.removeChild(node);
           // Unregister the removed node from events.
           assertInstanceof(node, ExtensionFocusRow).destroy();
         }
       }
-
-      var idToHighlight = this.getIdQueryParam_();
-      if (idToHighlight && $(idToHighlight))
-        this.scrollToNode_(idToHighlight);
-
-      var idToOpenOptions = this.getOptionsQueryParam_();
-      if (idToOpenOptions && $(idToOpenOptions))
-        this.showEmbeddedExtensionOptions_(idToOpenOptions, true);
     },
 
     /** Updates each row's focusable elements without rebuilding the grid. */
     updateFocusableElements: function() {
       var rows = document.querySelectorAll('.extension-list-item-wrapper[id]');
       for (var i = 0; i < rows.length; ++i) {
         assertInstanceof(rows[i], ExtensionFocusRow).updateFocusableElements();
       }
     },
 
@@ skipping 698 matching lines @@
         var nextExt = this.extensions_[this.extensions_.indexOf(extension) + 1];
         this.createNode_(extension, nextExt ? $(nextExt.id) : null);
       }
     }
   };
 
   return {
     ExtensionList: ExtensionList
   };
 });