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..3856e4c3b74de6542e14b2b3e4bfc9fc12675d08 100644
--- a/chrome/browser/extensions/pending_extension_manager.cc
+++ b/chrome/browser/extensions/pending_extension_manager.cc
@@ -2,10 +2,11 @@
 // 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"
+
 #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/common/extensions/extension.h"
 #include "content/public/browser/browser_thread.h"
 
@@ -30,22 +31,28 @@ 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;
+  PendingExtensionList::const_iterator iter = GetExtensionListConstIterById(id);

[Aaron Boodman, 2012/04/05 22:06:43]

It would be cleaner to make the pending list store linked_ptr<PendingInfo>
rather than raw PendingInfo structures. That way this can just be PendingInfo*
GetPendingInfoByID() and return NULL in the case where one doesn't exist.

Also you could remove the default constructor from PendingInfo which is only
there to support STL.

+  if (iter != pending_extension_list_.end()) {
+    *out_pending_extension_info = iter->second;
     return true;
   }
 
   return false;
 }
 
-void PendingExtensionManager::Remove(const std::string& id) {
-  pending_extension_map_.erase(id);
+bool PendingExtensionManager::Remove(const std::string& id) {
+  PendingExtensionList::iterator iter = GetExtensionListIterById(id);
+  if (iter != pending_extension_list_.end()) {
+    pending_extension_list_.erase(iter);
+    return true;
+  }
+
+  return false;
 }
 
 bool PendingExtensionManager::IsIdPending(const std::string& id) const {
-  return ContainsKey(pending_extension_map_, id);
+  PendingExtensionList::const_iterator iter = GetExtensionListConstIterById(id);
+  return iter != pending_extension_list_.end();
 }
 
 bool PendingExtensionManager::AddFromSync(
@@ -131,10 +138,10 @@ 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 {
+  PendingExtensionList::const_iterator iter;
+  for (iter = pending_extension_list_.begin();
+       iter != pending_extension_list_.end();
        ++iter) {
     Extension::Location install_source = iter->second.install_source();
 
@@ -145,7 +152,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->first);
   }
 }
 
@@ -161,7 +168,8 @@ 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)) {
+  PendingExtensionList::iterator iter = GetExtensionListIterById(id);
+  if (iter != pending_extension_list_.end()) {
     // 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
@@ -169,12 +177,12 @@ bool PendingExtensionManager::AddExtensionImpl(
     // 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()
+            << "  old location: " << iter->second.install_source()
             << "  new location: " << install_source;
 
     Extension::Location higher_priority_location =
         Extension::GetHigherPriorityLocation(
-            install_source, pending_extension_map_[id].install_source());
+            install_source, iter->second.install_source());
 
     if (higher_priority_location == install_source) {
       VLOG(1) << "Overwrite existing record.";
@@ -186,12 +194,13 @@ bool PendingExtensionManager::AddExtensionImpl(
   }
 
   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(update_url,
+                                            should_allow_install,
+                                            is_from_sync,
+                                            install_silently,
+                                            install_source);
+
+    pending_extension_list_.push_back(std::make_pair(id, new_pending_record));
     return true;
   }
   return false;
@@ -200,5 +209,34 @@ bool PendingExtensionManager::AddExtensionImpl(
 void PendingExtensionManager::AddForTesting(
     const std::string& id,
     const PendingExtensionInfo& pending_extension_info) {
-  pending_extension_map_[id] = pending_extension_info;
+  pending_extension_list_.push_back(std::make_pair(id, pending_extension_info));
+}
+
+PendingExtensionManager::PendingExtensionList::iterator
+    PendingExtensionManager::GetExtensionListIterById(const std::string& id) {
+  PendingExtensionList::iterator iter;
+  for (iter = pending_extension_list_.begin();

[Dmitry Polukhin, 2012/04/06 08:10:05]

It is basically std::find algorithm and IMHO it would be better to use std::find
instead of these two functions. I don't like function that return only iterator
(not pair(begin,end)) because reading caller code it is hard to make sure that
appropriate end() iterator is used.

I would add extension ID to PendingExtensionInfo struct so PendingExtensionList
will be able to contain PendingExtensionInfo instead of pair and add compare
function to use standard find algorithm.

+       iter != pending_extension_list_.end();
+       ++iter) {
+    if (id == iter->first) {
+      return iter;
+    }
+  }
+
+  return iter;
+}
+
+PendingExtensionManager::PendingExtensionList::const_iterator
+    PendingExtensionManager::GetExtensionListConstIterById(
+        const std::string& id) const {
+  PendingExtensionList::const_iterator iter;
+  for (iter = pending_extension_list_.begin();
+       iter != pending_extension_list_.end();
+       ++iter) {
+    if (id == iter->first) {
+      return iter;
+    }
+  }
+
+  return iter;
 }