Apps on NTP should be in order of installation

chrome/browser/extensions/pending_extension_manager.cc

Index: chrome/browser/extensions/pending_extension_manager.cc
diff --git a/chrome/browser/extensions/pending_extension_manager.cc b/chrome/browser/extensions/pending_extension_manager.cc
index 85e5705c51cc1d1478f3a48a1bcbc74ceaac6c29..1f1442ddd4d5fd68d56e65e446e2cb6b259af5fa 100644
--- a/chrome/browser/extensions/pending_extension_manager.cc
+++ b/chrome/browser/extensions/pending_extension_manager.cc
@@ -2,10 +2,12 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "chrome/browser/extensions/pending_extension_manager.h"

[Aaron Boodman, 2012/05/01 15:50:22]

Thanks for fixing this

+
 #include "base/logging.h"
 #include "base/stl_util.h"
 #include "chrome/browser/extensions/extension_service.h"
-#include "chrome/browser/extensions/pending_extension_manager.h"
+#include "chrome/browser/extensions/pending_extension_info.h"
 #include "chrome/common/extensions/extension.h"
 #include "content/public/browser/browser_thread.h"
 
@@ -27,25 +29,43 @@ PendingExtensionManager::PendingExtensionManager(
 
 PendingExtensionManager::~PendingExtensionManager() {}
 
-bool PendingExtensionManager::GetById(
-    const std::string& id,
-    PendingExtensionInfo* out_pending_extension_info) const {
-
-  PendingExtensionMap::const_iterator it = pending_extension_map_.find(id);
-  if (it != pending_extension_map_.end()) {
-    *out_pending_extension_info = it->second;
-    return true;
+const PendingExtensionInfo* PendingExtensionManager::GetById(
+    const std::string& id) const {
+  std::list<PendingExtensionInfo>::const_iterator iter;
+  for (iter = pending_extension_list_.begin();
+       iter != pending_extension_list_.end();

[Aaron Boodman, 2012/05/01 15:50:22]

Nit: You should bring the typedef back that was in the .h file so you don't have
to repeat this definition so much.

[mitchellwrosen, 2012/05/11 05:45:03]

Done.

+       ++iter) {
+    if (!id.compare(iter->id()))

[Aaron Boodman, 2012/05/01 15:50:22]

Nit: We don't usually use compare() with strings. Just use the == operator.

[mitchellwrosen, 2012/05/11 05:45:03]

Done.

+      return &(*iter);
   }
 
-  return false;
+  return NULL;
 }
 
-void PendingExtensionManager::Remove(const std::string& id) {
-  pending_extension_map_.erase(id);
+bool PendingExtensionManager::Remove(const std::string& id) {

[Aaron Boodman, 2012/05/01 15:50:22]

Same comments as GetById

[mitchellwrosen, 2012/05/11 05:45:03]

Done.

[mitchellwrosen, 2012/05/11 05:45:03]

Done.

+  std::list<PendingExtensionInfo>::iterator iter;
+  for (iter = pending_extension_list_.begin();
+       iter != pending_extension_list_.end();
+       ++iter) {
+    if (!id.compare(iter->id())) {
+      pending_extension_list_.erase(iter);
+      return true;
+    }
+  }
+
+  return false;
 }
 
 bool PendingExtensionManager::IsIdPending(const std::string& id) const {
-  return ContainsKey(pending_extension_map_, id);
+  std::list<PendingExtensionInfo>::const_iterator iter;
+  for (iter = pending_extension_list_.begin();

[Aaron Boodman, 2012/05/01 15:50:22]

Same comments as GetById

[mitchellwrosen, 2012/05/11 05:45:03]

Done.

+       iter != pending_extension_list_.end();
+       ++iter) {
+    if (!id.compare(iter->id()))
+      return true;
+  }
+
+  return false;
 }
 
 bool PendingExtensionManager::AddFromSync(
@@ -131,12 +151,12 @@ bool PendingExtensionManager::AddFromExternalFile(
 }
 
 void PendingExtensionManager::GetPendingIdsForUpdateCheck(
-    std::set<std::string>* out_ids_for_update_check) const {
-  PendingExtensionMap::const_iterator iter;
-  for (iter = pending_extension_map_.begin();
-       iter != pending_extension_map_.end();
+    std::list<std::string>* out_ids_for_update_check) const {
+  std::list<PendingExtensionInfo>::const_iterator iter;
+  for (iter = pending_extension_list_.begin();
+       iter != pending_extension_list_.end();
        ++iter) {
-    Extension::Location install_source = iter->second.install_source();
+    Extension::Location install_source = iter->install_source();
 
     // Some install sources read a CRX from the filesystem.  They can
     // not be fetched from an update URL, so don't include them in the
@@ -145,7 +165,7 @@ void PendingExtensionManager::GetPendingIdsForUpdateCheck(
         install_source == Extension::EXTERNAL_REGISTRY)
       continue;
 
-    out_ids_for_update_check->insert(iter->first);
+    out_ids_for_update_check->push_back(iter->id());
   }
 }
 
@@ -161,44 +181,54 @@ bool PendingExtensionManager::AddExtensionImpl(
   // Will add a pending extension record unless this variable is set to false.
   bool should_add_pending_record = true;
 
-  if (ContainsKey(pending_extension_map_, id)) {
-    // Bugs in this code will manifest as sporadic incorrect extension
-    // locations in situations where multiple install sources run at the
-    // same time. For example, on first login to a chrome os machine, an
-    // extension may be requested by sync and the default extension set.
-    // The following logging will help diagnose such issues.
-    VLOG(1) << "Extension id " << id
-            << " was entered for update more than once."
-            << "  old location: " << pending_extension_map_[id].install_source()
-            << "  new location: " << install_source;
-
-    Extension::Location higher_priority_location =
-        Extension::GetHigherPriorityLocation(
-            install_source, pending_extension_map_[id].install_source());
-
-    if (higher_priority_location == install_source) {
-      VLOG(1) << "Overwrite existing record.";
-
-    } else {
-      VLOG(1) << "Keep existing record.";
-      should_add_pending_record = false;
+  std::list<PendingExtensionInfo>::iterator iter;
+  for (iter = pending_extension_list_.begin();

[Aaron Boodman, 2012/05/01 15:50:22]

if (GetById(id)) ?

[mitchellwrosen, 2012/05/11 05:45:03]

Done.

+       iter != pending_extension_list_.end();
+       ++iter) {
+    if (!id.compare(iter->id())) {
+      // Bugs in this code will manifest as sporadic incorrect extension
+      // locations in situations where multiple install sources run at the
+      // same time. For example, on first login to a chrome os machine, an
+      // extension may be requested by sync and the default extension set.
+      // The following logging will help diagnose such issues.
+      VLOG(1) << "Extension id " << id
+              << " was entered for update more than once."
+              << "  old location: " << iter->install_source()
+              << "  new location: " << install_source;
+
+      Extension::Location higher_priority_location =
+          Extension::GetHigherPriorityLocation(
+              install_source, iter->install_source());
+
+      if (higher_priority_location == install_source) {
+        VLOG(1) << "Overwrite existing record.";
+
+      } else {
+        VLOG(1) << "Keep existing record.";
+        should_add_pending_record = false;
+      }
+
+      break;
     }
   }
 
   if (should_add_pending_record) {
-    pending_extension_map_[id] = PendingExtensionInfo(
-        update_url,
-        should_allow_install,
-        is_from_sync,
-        install_silently,
-        install_source);
+    PendingExtensionInfo new_pending_record(id,
+                                            update_url,
+                                            should_allow_install,
+                                            is_from_sync,
+                                            install_silently,
+                                            install_source);
+
+    pending_extension_list_.push_back(new_pending_record);
     return true;
   }
+
   return false;
 }
 
 void PendingExtensionManager::AddForTesting(
     const std::string& id,
     const PendingExtensionInfo& pending_extension_info) {
-  pending_extension_map_[id] = pending_extension_info;
+  pending_extension_list_.push_back(pending_extension_info);
 }