part 3 of toolstrip dragging - persist the drag order

Also notify other browser windows of changes to the order.

BUG=http://code.google.com/p/chromium/issues/detail?id=12123
TEST=browser_tests.exe --gtest_filter=ExtensionShelfModelTest.*


Review URL: http://codereview.chromium.org/131114

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@18845 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 182 insertions, 38 deletions

diff --git a/chrome/browser/extensions/extension_prefs.cc b/chrome/browser/extensions/extension_prefs.cc
index 56754fe..50618b5 100644
--- a/chrome/browser/extensions/extension_prefs.cc
+++ b/chrome/browser/extensions/extension_prefs.cc
@@ -128,6 +128,16 @@ ExtensionPrefs::URLList ExtensionPrefs::GetShelfToolstripOrder() {
   return urls;
 }
 
+void ExtensionPrefs::SetShelfToolstripOrder(const URLList& urls) {
+  ListValue* toolstrip_urls = prefs_->GetMutableList(kExtensionShelf);
+  toolstrip_urls->Clear();
+  for (size_t i = 0; i < urls.size(); ++i) {
+    GURL url = urls[i];
+    toolstrip_urls->Append(new StringValue(url.spec()));
+  }
+  prefs_->ScheduleSavePersistentPrefs();
+}
+
 void ExtensionPrefs::OnExtensionInstalled(Extension* extension) {
   std::string id = extension->id();
   UpdateExtensionPref(id, kPrefState,
diff --git a/chrome/browser/extensions/extension_prefs.h b/chrome/browser/extensions/extension_prefs.h
index 8244234..a6e7ec4 100644
--- a/chrome/browser/extensions/extension_prefs.h
+++ b/chrome/browser/extensions/extension_prefs.h
@@ -33,6 +33,9 @@ class ExtensionPrefs {
   typedef std::vector<GURL> URLList;
   URLList GetShelfToolstripOrder();
 
+  // Sets the order that toolstrip URLs appear in the shelf.
+  void SetShelfToolstripOrder(const URLList& urls);
+
   // Called when an extension is installed, so that prefs get created.
   void OnExtensionInstalled(Extension* extension);
 
diff --git a/chrome/browser/extensions/extension_shelf.cc b/chrome/browser/extensions/extension_shelf.cc
index 2c1924b..1d4eac3 100644
--- a/chrome/browser/extensions/extension_shelf.cc
+++ b/chrome/browser/extensions/extension_shelf.cc
@@ -365,6 +365,12 @@ void ExtensionShelf::ExtensionShelfEmpty() {
   PreferredSizeChanged();
 }
 
+void ExtensionShelf::ShelfModelReloaded() {
+  // None of the child views are parent owned, so nothing is being leaked here.
+  RemoveAllChildViews(false);
+  LoadFromModel();
+}
+
 void ExtensionShelf::OnExtensionMouseEvent(ExtensionView* view) {
   // Ignore these events when dragging.
   if (drag_placeholder_view_)
@@ -578,7 +584,6 @@ void ExtensionShelf::LayoutShelfHandle() {
 
 void ExtensionShelf::LoadFromModel() {
   int count = model_->count();
-  for (int i = 0; i < count; ++i) {
+  for (int i = 0; i < count; ++i)
     ToolstripInsertedAt(model_->ToolstripAt(i), i);
-  }
 }
diff --git a/chrome/browser/extensions/extension_shelf.h b/chrome/browser/extensions/extension_shelf.h
index 7cc1a638..ddef2ed 100644
--- a/chrome/browser/extensions/extension_shelf.h
+++ b/chrome/browser/extensions/extension_shelf.h
@@ -56,6 +56,7 @@ class ExtensionShelf : public views::View,
                               int to_index);
   virtual void ToolstripChangedAt(ExtensionHost* toolstrip, int index);
   virtual void ExtensionShelfEmpty();
+  virtual void ShelfModelReloaded();
 
   // Dragging toolstrips
   void DragExtension();
diff --git a/chrome/browser/extensions/extension_shelf_model.cc b/chrome/browser/extensions/extension_shelf_model.cc
index 6cda271..70013e5 100644
--- a/chrome/browser/extensions/extension_shelf_model.cc
+++ b/chrome/browser/extensions/extension_shelf_model.cc
@@ -14,18 +14,31 @@
 
 /////////////////////////////
 
-ExtensionShelfModel::ExtensionShelfModel(Browser* browser) : browser_(browser) {
+ExtensionShelfModel::ExtensionShelfModel(Browser* browser)
+    : browser_(browser), ready_(false) {
   // Watch extensions loaded and unloaded notifications.
-  registrar_.Add(this, NotificationType::EXTENSIONS_LOADED,
+  registrar_.Add(this, NotificationType::EXTENSION_INSTALLED,
                  NotificationService::AllSources());
   registrar_.Add(this, NotificationType::EXTENSION_UNLOADED,
                  NotificationService::AllSources());
+  registrar_.Add(this, NotificationType::EXTENSIONS_LOADED,
+                 NotificationService::AllSources());
+  registrar_.Add(this, NotificationType::EXTENSIONS_READY,
+                 NotificationService::AllSources());
 
   // Add any already-loaded extensions now, since we missed the notification for
   // those.
   ExtensionsService* service = browser_->profile()->GetExtensionsService();
-  if (service)  // This can be null in unit tests.
-    AddExtensions(service->extensions());
+  if (service) {  // This can be null in unit tests.
+    prefs_ = browser_->profile()->GetExtensionsService()->extension_prefs();
+    registrar_.Add(this, NotificationType::EXTENSION_SHELF_MODEL_CHANGED,
+                   Source<ExtensionPrefs>(prefs_));
+    ready_ = service->is_ready();
+    if (ready_) {
+      AddExtensions(service->extensions());
+      SortToolstrips();
+    }
+  }
 }
 
 ExtensionShelfModel::~ExtensionShelfModel() {
@@ -53,8 +66,10 @@ void ExtensionShelfModel::AppendToolstrip(ExtensionHost* toolstrip) {
 void ExtensionShelfModel::InsertToolstripAt(int index,
                                             ExtensionHost* toolstrip) {
   toolstrips_.insert(toolstrips_.begin() + index, toolstrip);
-  FOR_EACH_OBSERVER(ExtensionShelfModelObserver, observers_,
-                    ToolstripInsertedAt(toolstrip, index));
+  if (ready_) {
+    FOR_EACH_OBSERVER(ExtensionShelfModelObserver, observers_,
+                      ToolstripInsertedAt(toolstrip, index));
+  }
 }
 
 void ExtensionShelfModel::RemoveToolstripAt(int index) {
@@ -75,10 +90,12 @@ void ExtensionShelfModel::MoveToolstripAt(int index, int to_index) {
 
   FOR_EACH_OBSERVER(ExtensionShelfModelObserver, observers_,
                     ToolstripMoved(toolstrip, index, to_index));
+
+  UpdatePrefs();
 }
 
 int ExtensionShelfModel::IndexOfToolstrip(ExtensionHost* toolstrip) {
-  ExtensionToolstripList::iterator i =
+  ExtensionToolstrips::iterator i =
     std::find(toolstrips_.begin(), toolstrips_.end(), toolstrip);
   if (i == toolstrips_.end())
     return -1;
@@ -93,49 +110,123 @@ void ExtensionShelfModel::Observe(NotificationType type,
                                   const NotificationSource& source,
                                   const NotificationDetails& details) {
   switch (type.value) {
-    case NotificationType::EXTENSIONS_LOADED: {
-      const ExtensionList* extensions = Details<ExtensionList>(details).ptr();
-      AddExtensions(extensions);
+    case NotificationType::EXTENSION_INSTALLED:
+      if (ready_) {
+        AddExtension(Details<Extension>(details).ptr());
+        UpdatePrefs();
+      }
       break;
-    }
-    case NotificationType::EXTENSION_UNLOADED: {
-      Extension* extension = Details<Extension>(details).ptr();
-      RemoveExtension(extension);
+    case NotificationType::EXTENSIONS_LOADED:
+      if (ready_)
+        AddExtensions(Details<ExtensionList>(details).ptr());
+      break;
+    case NotificationType::EXTENSION_UNLOADED:
+      RemoveExtension(Details<Extension>(details).ptr());
+      break;
+    case NotificationType::EXTENSIONS_READY:
+      AddExtensions(browser_->profile()->GetExtensionsService()->extensions());
+      SortToolstrips();
+      ready_ = true;
+      break;
+    case NotificationType::EXTENSION_SHELF_MODEL_CHANGED:
+      // Ignore changes that this model originated.
+      if (Details<ExtensionShelfModel>(details).ptr() != this)
+        SortToolstrips();
       break;
-    }
     default:
       DCHECK(false) << "Unhandled notification of type: " << type.value;
       break;
   }
 }
 
-void ExtensionShelfModel::AddExtensions(const ExtensionList* extensions) {
+void ExtensionShelfModel::AddExtension(Extension* extension) {
   ExtensionProcessManager* manager =
       browser_->profile()->GetExtensionProcessManager();
+  DCHECK(manager);
   if (!manager)
     return;
 
-  for (ExtensionList::const_iterator extension = extensions->begin();
-       extension != extensions->end(); ++extension) {
-    for (std::vector<std::string>::const_iterator toolstrip_path =
-         (*extension)->toolstrips().begin();
-         toolstrip_path != (*extension)->toolstrips().end(); ++toolstrip_path) {
-      ExtensionHost* host =
-          manager->CreateView(*extension,
-                              (*extension)->GetResourceURL(*toolstrip_path),
-                              browser_);
-      AppendToolstrip(host);
-    }
+  for (std::vector<std::string>::const_iterator toolstrip_path =
+       extension->toolstrips().begin();
+       toolstrip_path != extension->toolstrips().end(); ++toolstrip_path) {
+    GURL url = extension->GetResourceURL(*toolstrip_path);
+    ExtensionHost* host = manager->CreateView(extension, url, browser_);
+    AppendToolstrip(host);
+  }
+}
+
+void ExtensionShelfModel::AddExtensions(const ExtensionList* extensions) {
+  if (extensions->size()) {
+    ExtensionList::const_iterator extension = extensions->begin();
+    for (; extension != extensions->end(); ++extension)
+      AddExtension(*extension);
   }
 }
 
-void ExtensionShelfModel::RemoveExtension(const Extension* extension) {
+void ExtensionShelfModel::RemoveExtension(Extension* extension) {
+  bool changed = false;
   for (int i = count() - 1; i >= 0; --i) {
     ExtensionHost* t = ToolstripAt(i);
     if (t->extension()->id() == extension->id()) {
+      changed = true;
       RemoveToolstripAt(i);
+
       // There can be more than one toolstrip per extension, so we have to keep
       // looping even after finding a match.
     }
   }
+  if (changed)
+    UpdatePrefs();
+}
+
+void ExtensionShelfModel::UpdatePrefs() {
+  if (!prefs_)
+    return;
+
+  // It's easiest to just rebuild the list each time.
+  ExtensionPrefs::URLList urls;
+  for (int i = 0; i < count(); ++i)
+    urls.push_back(ToolstripAt(i)->GetURL());
+  prefs_->SetShelfToolstripOrder(urls);
+
+  NotificationService::current()->Notify(
+      NotificationType::EXTENSION_SHELF_MODEL_CHANGED,
+      Source<ExtensionPrefs>(prefs_),
+      Details<ExtensionShelfModel>(this));
+}
+
+void ExtensionShelfModel::SortToolstrips() {
+  ExtensionPrefs::URLList urls = prefs_->GetShelfToolstripOrder();
+  ExtensionToolstrips copy =
+      ExtensionToolstrips(toolstrips_.begin(), toolstrips_.end());
+  toolstrips_.clear();
+
+  // Go through the urls and find the matching toolstrip, re-adding it to the
+  // new list in the proper order.
+  for (size_t i = 0; i < urls.size(); ++i) {
+    GURL& url = urls[i];
+    for (ExtensionToolstrips::iterator toolstrip = copy.begin();
+        toolstrip != copy.end(); ++toolstrip) {
+      if (url == (*toolstrip)->GetURL()) {
+        // Note that it's technically possible for the same URL to appear in
+        // multiple toolstrips, so we don't do any testing for uniqueness.
+        toolstrips_.push_back(*toolstrip);
+
+        // Remove the toolstrip from the list so we don't have to iterate over
+        // it next time.
+        copy.erase(toolstrip);
+        break;
+      }
+    }
+  }
+
+  // Any toolstrips remaining in |copy| were somehow missing from the prefs,
+  // so just append them to the end.
+  for (ExtensionToolstrips::iterator toolstrip = copy.begin();
+      toolstrip != copy.end(); ++toolstrip) {
+    toolstrips_.push_back(*toolstrip);
+  }
+
+  FOR_EACH_OBSERVER(ExtensionShelfModelObserver, observers_,
+                    ShelfModelReloaded());
 }
diff --git a/chrome/browser/extensions/extension_shelf_model.h b/chrome/browser/extensions/extension_shelf_model.h
index 13fc520..7af61f8 100644
--- a/chrome/browser/extensions/extension_shelf_model.h
+++ b/chrome/browser/extensions/extension_shelf_model.h
@@ -11,11 +11,11 @@
 #include "base/observer_list.h"
 #include "chrome/common/notification_observer.h"
 #include "chrome/common/notification_registrar.h"
+#include "chrome/browser/extensions/extension_host.h"
 #include "chrome/browser/extensions/extensions_service.h"
 
 class Browser;
-class ExtensionHost;
-class ExtensionView;
+class ExtensionPrefs;
 
 // Objects implement this interface when they wish to be notified of changes to
 // the ExtensionShelfModel.
@@ -41,10 +41,16 @@ class ExtensionShelfModelObserver {
   // There are no more toolstrips in the model.
   virtual void ExtensionShelfEmpty() {}
 
+  // The entire model may have changed.
+  virtual void ShelfModelReloaded() {}
+
   // TODO(erikkay) - any more?
 };
 
-// The model representing the toolstrips on an ExtensionShelf.
+// The model representing the toolstrips on an ExtensionShelf.  The order of
+// the toolstrips is common across all of the models for a given Profile,
+// but there are multiple models.  Each model contains the hosts/views which
+// are specific to a Browser.
 class ExtensionShelfModel : public NotificationObserver {
  public:
   ExtensionShelfModel(Browser* browser);
@@ -82,27 +88,42 @@ class ExtensionShelfModel : public NotificationObserver {
                        const NotificationDetails& details);
 
  private:
+  // Add all of the toolstrips from |extension|.
+  void AddExtension(Extension* extension);
+
   // Add all of the toolstrips from each extension in |extensions|.
   void AddExtensions(const ExtensionList* extensions);
 
   // Remove all of the toolstrips in |extension| from the shelf.
-  void RemoveExtension(const Extension* extension);
+  void RemoveExtension(Extension* extension);
+
+  // Update prefs with the most recent changes.
+  void UpdatePrefs();
+
+  // Reloads order from prefs.
+  void SortToolstrips();
 
   // The browser that this model is attached to.
   Browser* browser_;
 
+  // The preferences that this model uses.
+  ExtensionPrefs* prefs_;
+
   // Manages our notification registrations.
   NotificationRegistrar registrar_;
 
-  // The Toolstrips in this model.  The model owns these objects.
-  typedef std::vector<ExtensionHost*> ExtensionToolstripList;
-  ExtensionToolstripList toolstrips_;
+  // The Toolstrips loaded in this model. The model owns these objects.
+  typedef std::vector<ExtensionHost*> ExtensionToolstrips;
+  ExtensionToolstrips toolstrips_;
 
   // Our observers.
   typedef ObserverList<ExtensionShelfModelObserver>
       ExtensionShelfModelObservers;
   ExtensionShelfModelObservers observers_;
 
+  // Whether the model has received an EXTENSIONS_READY notification.
+  bool ready_;
+
   DISALLOW_COPY_AND_ASSIGN(ExtensionShelfModel);
 };
 
diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc
index 07c6fba..a9f81ca 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -219,7 +219,8 @@ ExtensionsService::ExtensionsService(Profile* profile,
       extensions_enabled_(
           CommandLine::ForCurrentProcess()->
           HasSwitch(switches::kEnableExtensions)),
-      show_extensions_prompts_(true) {
+      show_extensions_prompts_(true),
+      ready_(false) {
   // We pass ownership of this object to the Backend.
   DictionaryValue* extensions = extension_prefs_->CopyCurrentExtensions();
   backend_ = new ExtensionsServiceBackend(
@@ -343,6 +344,7 @@ void ExtensionsService::GarbageCollectExtensions() {
 }
 
 void ExtensionsService::OnLoadedInstalledExtensions() {
+  ready_ = true;
   NotificationService::current()->Notify(
       NotificationType::EXTENSIONS_READY,
       Source<ExtensionsService>(this),
diff --git a/chrome/browser/extensions/extensions_service.h b/chrome/browser/extensions/extensions_service.h
index d8fa0d0..f2f0b56 100644
--- a/chrome/browser/extensions/extensions_service.h
+++ b/chrome/browser/extensions/extensions_service.h
@@ -142,6 +142,11 @@ class ExtensionsService
     return show_extensions_prompts_;
   }
 
+  ExtensionPrefs* extension_prefs() { return extension_prefs_.get(); }
+
+  // Whether the extension service is ready.
+  bool is_ready() { return ready_; }
+
  private:
   // For OnExtensionLoaded, OnExtensionInstalled, and
   // OnExtensionVersionReinstalled.
@@ -186,6 +191,9 @@ class ExtensionsService
   // The backend that will do IO on behalf of this instance.
   scoped_refptr<ExtensionsServiceBackend> backend_;
 
+  // Is the service ready to go?
+  bool ready_;
+
   DISALLOW_COPY_AND_ASSIGN(ExtensionsService);
 };
 
diff --git a/chrome/common/notification_type.h b/chrome/common/notification_type.h
index d4ced73..0ee474a 100644
--- a/chrome/common/notification_type.h
+++ b/chrome/common/notification_type.h
@@ -600,6 +600,9 @@ class NotificationType {
     // and is fully functional. The details are an ExtensionHost*.
     EXTENSION_PROCESS_RESTORED,
 
+    // Sent when the contents or order of toolstrips in the shelf model change.
+    EXTENSION_SHELF_MODEL_CHANGED,
+
     // Debugging ---------------------------------------------------------------
 
     // Sent from ~RenderViewHost. The source is the RenderViewHost.