Fix scroll regression when specifying an extension id.

chrome/browser/resources/extensions/extension_list.js

Index: chrome/browser/resources/extensions/extension_list.js
diff --git a/chrome/browser/resources/extensions/extension_list.js b/chrome/browser/resources/extensions/extension_list.js
index 637707e0c31e87a92bde6a91c51f53dabc31af1c..f8f19cb3bcfc91500c9660daf3e604d3f975bdc0 100644
--- a/chrome/browser/resources/extensions/extension_list.js
+++ b/chrome/browser/resources/extensions/extension_list.js
@@ -290,6 +290,22 @@ cr.define('extensions', function() {
       return this.extensionsUpdated_;
     },
 
+    /**
+     * Updates any element that needs to be visible to update properly.
+     * @param {boolean} visible Whether the extension list is visible.

[Devlin, 2015/04/14 22:23:55]

Drive-by: What param?

+     */
+    updateVisible: function() {

[not at google - send to devlin, 2015/04/14 21:14:50]

TBH "updateVisible" is still too vague for me. Are these visible elements?
nodes? extensions? Seems more like... well, things, but you can't say that.
Maybe something along the lines of "refresh" is more appropriate? Also: should
callers of updateFocusableElements be using this method instead? The public API
of ExtensionList is growing, and as a consumer of its API I don't know when to
call updateVisible and when to call updateFocusable. Or updateExtensionsData for
that matter.

[Dan Beam, 2015/04/14 21:44:13]

how about:
- updateAndHighlight()
- onExtensionsShown()
- afterUpdate()
- onUpdateFinished()
?

[hcarmona, 2015/04/14 22:06:24]

We can go with onUpdateFinished_() and make it private if there are no
objections to making the list show itself.

updateFocusableElements is different than this because this will potentially
scroll while the updateFocusableElements just updates the elements that can be
focused.

[not at google - send to devlin, 2015/04/14 22:39:25]

Gotcha. Maybe this doesn't make sense, but could there be a single method which
takes a parameter whether to scroll or not?

"no objections to making the list show itself" - in what way is the list showing
itself, and what might the objections be?

[hcarmona, 2015/04/14 23:06:43]

Scrolling should happen when the list is loaded, if we can keep it encapsulated
in the ExtensionList it'll make for a cleaner API
List currently becomes visible after loading via a promise. However, if we use
the same promise, it's not necessary to expose this method at all (see updated
code).

+      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;
@@ -338,14 +354,6 @@ cr.define('extensions', function() {
           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. */