[MD Extensions] Fix item sorting

chrome/browser/resources/md_extensions/service.js

Index: chrome/browser/resources/md_extensions/service.js
diff --git a/chrome/browser/resources/md_extensions/service.js b/chrome/browser/resources/md_extensions/service.js
index 5f58f312d1bb75849250bc56575bfd12ff7366f9..423f7f00499269ca796c9a218eab3fb215ff7d2e 100644
--- a/chrome/browser/resources/md_extensions/service.js
+++ b/chrome/browser/resources/md_extensions/service.js
@@ -6,30 +6,6 @@ cr.define('extensions', function() {
   'use strict';
 
   /**
-   * Compares two extensions to determine which should come first in the list.
-   * @param {chrome.developerPrivate.ExtensionInfo} a
-   * @param {chrome.developerPrivate.ExtensionInfo} b
-   * @return {number}
-   */
-  var compareExtensions = function(a, b) {
-    function compare(x, y) {
-      return x < y ? -1 : (x > y ? 1 : 0);
-    }
-    function compareLocation(x, y) {
-      if (x.location == y.location)
-        return 0;
-      if (x.location == chrome.developerPrivate.Location.UNPACKED)
-        return -1;
-      if (y.location == chrome.developerPrivate.Location.UNPACKED)
-        return 1;
-      return 0;
-    }
-    return compareLocation(a, b) ||
-           compare(a.name.toLowerCase(), b.name.toLowerCase()) ||
-           compare(a.id, b.id);
-  };
-
-  /**
    * @constructor
    * @implements {extensions.ItemDelegate}
    * @implements {extensions.SidebarDelegate}
@@ -42,8 +18,10 @@ cr.define('extensions', function() {
 
     /** @param {extensions.Manager} manager */
     managerReady: function(manager) {
+      /** @private {extensions.Manager} */
       this.manager_ = manager;
-      this.sidebar_ = manager.sidebar;
+      /** @private {extensions.Sidebar} */
+      this.sidebar_ = /** @type {extensions.Sidebar} */(manager.sidebar);

[Devlin, 2015/11/18 00:38:57]

nasty cast needed because sidebar is a declared property on manager.

[michaelpg, 2015/11/18 09:08:46]

polymer_pass would normally pick these up if you @type the property.

unless the PolymerPass isn't working because the Manager lives inside a
cr.define?

[Devlin, 2015/11/18 17:16:59]

Fixed with the @type in manager.js.

       this.sidebar_.setDelegate(this);
       chrome.developerPrivate.onProfileStateChanged.addListener(
           this.onProfileStateChanged_.bind(this));
@@ -52,7 +30,7 @@ cr.define('extensions', function() {
       chrome.developerPrivate.getExtensionsInfo(
           {includeDisabled: true, includeTerminated: true},
           function(extensions) {
-        extensions.sort(compareExtensions);
+        /** @private {Array<chrome.developerPrivate.ExtensionInfo>} */
         this.extensions_ = extensions;
         for (let extension of extensions)
           this.manager_.addItem(extension, this);

[michaelpg, 2015/11/18 09:08:46]

so... this is essentially bubble sort now, right?

[Devlin, 2015/11/18 17:16:59]

Hmm... I think it's actually insertion sort, right?  Which is still O(n^2), but
it's worth noting that the data set here is small (i.e., anything over 50 and
the user probably has other problems), and on reasonably small data sets,
insertion sort's actually decently fast.

[michaelpg, 2015/11/18 20:01:17]

ok, insertion sort, but still not a native sort() or even a native compare
function :-P

as a user with >50 extensions, what other problems do I have? Having a bunch of
apps on CrOS is not abnormal.

all that being said, with some quick and dirty empirical testing it looks like
this would add ~20ms for 200 extensions. sooo, not worth optimizing for.

[Devlin, 2015/11/18 23:23:10]

Was mostly referring to extensions, since apps (globally) are pretty uncommon -
and with >50 extensions, your browsing experience is almost certainly suffering.
 >50 apps isn't as bad, but is also pretty uncommon.  (And, like you said, the
~20ms probably isn't worth the work. :))

@@ -66,6 +44,7 @@ cr.define('extensions', function() {
      * @private
      */
     onProfileStateChanged_: function(profileInfo) {
+      /** @private {chrome.developerPrivate.ProfileInfo} */
       this.profileInfo_ = profileInfo;
       this.sidebar_.inDevMode = profileInfo.inDeveloperMode;
       this.manager_.forEachItem(function(item) {
@@ -104,13 +83,9 @@ cr.define('extensions', function() {
               return extension.id == eventData.extensionInfo.id;
             });
             assert(currentIndex == -1);
-            var newIndex = this.extensions_.findIndex(function(extension) {
-              return compareExtensions(extension,
-                                       assert(eventData.extensionInfo)) > 0;
-            });
-            newIndex = newIndex == -1 ? this.extensions_.length : newIndex;
-            this.extensions_.splice(newIndex, 0, eventData.extensionInfo);
-            this.manager_.addItem(eventData.extensionInfo, this, newIndex);
+
+            this.extensions_.push(eventData.extensionInfo);
+            this.manager_.addItem(eventData.extensionInfo, this);
           }
           break;
         case EventType.UNINSTALLED: