re-try r37025 with fix for incognito mode

----------------------------------------

Views: use ExtensionToolbarModel for ordering of browser action buttons.

This doesn't implement drag and drop reording, but the order is stored on shutdown and restored on startup.

BUG=26990


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

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

2 files changed, 111 insertions, 123 deletions

diff --git a/chrome/browser/views/browser_actions_container.cc b/chrome/browser/views/browser_actions_container.cc
index 6aaa387..b0f6147 100644
--- a/chrome/browser/views/browser_actions_container.cc
+++ b/chrome/browser/views/browser_actions_container.cc
@@ -268,27 +268,26 @@ BrowserActionsContainer::BrowserActionsContainer(
       toolbar_(toolbar),
       popup_(NULL),
       popup_button_(NULL),
+      model_(NULL),
       resize_gripper_(NULL),
       chevron_(NULL),
       suppress_chevron_(false),
       resize_amount_(0),
       animation_target_size_(0),
       ALLOW_THIS_IN_INITIALIZER_LIST(task_factory_(this)) {
+  SetID(VIEW_ID_BROWSER_ACTION_TOOLBAR);
+
   ExtensionsService* extension_service = profile->GetExtensionsService();
   if (!extension_service)  // The |extension_service| can be NULL in Incognito.
     return;
 
-  registrar_.Add(this, NotificationType::EXTENSION_LOADED,
-                 Source<Profile>(profile_));
-  registrar_.Add(this, NotificationType::EXTENSION_UNLOADED,
-                 Source<Profile>(profile_));
-  registrar_.Add(this, NotificationType::EXTENSION_UNLOADED_DISABLED,
-                 Source<Profile>(profile_));
   registrar_.Add(this, NotificationType::EXTENSION_HOST_VIEW_SHOULD_CLOSE,
                  Source<Profile>(profile_));
 
-  resize_animation_.reset(new SlideAnimation(this));
+  model_ = extension_service->toolbar_model();
+  model_->AddObserver(this);
 
+  resize_animation_.reset(new SlideAnimation(this));
   resize_gripper_ = new views::ResizeGripper(this);
   resize_gripper_->SetVisible(false);
   AddChildView(resize_gripper_);
@@ -306,8 +305,6 @@ BrowserActionsContainer::BrowserActionsContainer(
   int predefined_width =
       profile_->GetPrefs()->GetInteger(prefs::kBrowserActionContainerWidth);
   container_size_ = gfx::Size(predefined_width, kButtonSize);
-
-  SetID(VIEW_ID_BROWSER_ACTION_TOOLBAR);
 }
 
 BrowserActionsContainer::~BrowserActionsContainer() {
@@ -345,96 +342,6 @@ void BrowserActionsContainer::RefreshBrowserActionViews() {
     browser_action_views_[i]->button()->UpdateState();
 }
 
-void BrowserActionsContainer::AddBrowserAction(Extension* extension) {
-#if defined(DEBUG)
-  for (size_t i = 0; i < browser_action_views_.size(); ++i) {
-    DCHECK(browser_action_views_[i]->button()->extension() != extension) <<
-           "Asked to add a browser action view for an extension that already "
-           "exists.";
-  }
-#endif
-  if (!extension->browser_action())
-    return;
-
-  // Before we change anything, determine the number of visible browser actions.
-  size_t visible_actions = 0;
-  for (size_t i = 0; i < browser_action_views_.size(); ++i) {
-    if (browser_action_views_[i]->IsVisible())
-      ++visible_actions;
-  }
-
-  // Add the new browser action to the vector and the view hierarchy.
-  BrowserActionView* view = new BrowserActionView(extension, this);
-  browser_action_views_.push_back(view);
-  AddChildView(view);
-
-  // For details on why we do the following see the class comments in the
-  // header.
-
-  // Determine if we need to increase (we only do that if the container was
-  // showing all icons before the addition of this icon). We use -1 because
-  // we don't want to count the view that we just added.
-  if (visible_actions < browser_action_views_.size() - 1) {
-    // Some icons were hidden, don't increase the size of the container.
-    OnBrowserActionVisibilityChanged();
-  } else {
-    // Container was at max, increase the size of it by one icon.
-    animation_target_size_ = IconCountToWidth(visible_actions + 1);
-
-    // We don't want the chevron to appear while we animate. See documentation
-    // in the header for why we do this.
-    suppress_chevron_ = !chevron_->IsVisible();
-
-    // Animate!
-    resize_animation_->Reset();
-    resize_animation_->SetTweenType(SlideAnimation::NONE);
-    resize_animation_->Show();
-  }
-}
-
-void BrowserActionsContainer::RemoveBrowserAction(Extension* extension) {
-  if (!extension->browser_action())
-    return;
-
-  if (popup_ && popup_->host()->extension() == extension) {
-    HidePopup();
-  }
-
-  // Before we change anything, determine the number of visible browser actions.
-  int visible_actions = 0;
-  for (size_t i = 0; i < browser_action_views_.size(); ++i) {
-    if (browser_action_views_[i]->IsVisible())
-      ++visible_actions;
-  }
-
-  for (std::vector<BrowserActionView*>::iterator iter =
-       browser_action_views_.begin(); iter != browser_action_views_.end();
-       ++iter) {
-    if ((*iter)->button()->extension() == extension) {
-      RemoveChildView(*iter);
-      delete *iter;
-      browser_action_views_.erase(iter);
-
-      // For details on why we do the following see the class comments in the
-      // header.
-
-      // Calculate the target size we'll animate to (end state). This might be
-      // the same size (if the icon we are removing is in the overflow bucket
-      // and there are other icons there). We don't decrement visible_actions
-      // because we want the container to stay the same size (clamping will take
-      // care of shrinking the container if there aren't enough icons to show).
-      animation_target_size_ =
-          ClampToNearestIconCount(IconCountToWidth(visible_actions));
-
-      // Animate!
-      resize_animation_->Reset();
-      resize_animation_->SetTweenType(SlideAnimation::EASE_OUT);
-      resize_animation_->Show();
-      return;
-    }
-  }
-}
-
 void BrowserActionsContainer::DeleteBrowserActionViews() {
   if (!browser_action_views_.empty()) {
     for (size_t i = 0; i < browser_action_views_.size(); ++i)
@@ -615,15 +522,21 @@ void BrowserActionsContainer::Paint(gfx::Canvas* canvas) {
 void BrowserActionsContainer::ViewHierarchyChanged(bool is_add,
                                                    views::View* parent,
                                                    views::View* child) {
+  // No extensions (e.g., incognito).
+  if (!model_)
+    return;
+
   if (is_add && child == this) {
+    // Initial toolbar button creation and placement in the widget hierarchy.
     // We do this here instead of in the constructor because AddBrowserAction
     // calls Layout on the Toolbar, which needs this object to be constructed
     // before its Layout function is called.
-    ExtensionsService* extension_service = profile_->GetExtensionsService();
-    if (!extension_service)
-      return;  // The |extension_service| can be NULL in Incognito.
-    for (size_t i = 0; i < extension_service->extensions()->size(); ++i)
-      AddBrowserAction(extension_service->extensions()->at(i));
+    for (ExtensionList::iterator iter = model_->begin();
+        iter != model_->end(); ++iter) {
+      BrowserActionView* view = new BrowserActionView(*iter, this);
+      browser_action_views_.push_back(view);
+      AddChildView(view);
+    }
   }
 }
 
@@ -631,17 +544,6 @@ void BrowserActionsContainer::Observe(NotificationType type,
                                       const NotificationSource& source,
                                       const NotificationDetails& details) {
   switch (type.value) {
-    case NotificationType::EXTENSION_LOADED:
-      AddBrowserAction(Details<Extension>(details).ptr());
-      OnBrowserActionVisibilityChanged();
-      break;
-
-    case NotificationType::EXTENSION_UNLOADED:
-    case NotificationType::EXTENSION_UNLOADED_DISABLED:
-      RemoveBrowserAction(Details<Extension>(details).ptr());
-      OnBrowserActionVisibilityChanged();
-      break;
-
     case NotificationType::EXTENSION_HOST_VIEW_SHOULD_CLOSE:
       // If we aren't the host of the popup, then disregard the notification.
       if (!popup_ || Details<ExtensionHost>(popup_->host()) != details)
@@ -737,6 +639,91 @@ int BrowserActionsContainer::ClampToNearestIconCount(int pixelWidth) const {
   return returning;
 }
 
+void BrowserActionsContainer::BrowserActionAdded(Extension* extension,
+                                                 int index) {
+#if defined(DEBUG)
+  for (size_t i = 0; i < browser_action_views_.size(); ++i) {
+    DCHECK(browser_action_views_[i]->button()->extension() != extension) <<
+           "Asked to add a browser action view for an extension that already "
+           "exists.";
+  }
+#endif
+
+  // Before we change anything, determine the number of visible browser actions.
+  size_t visible_actions = 0;
+  for (size_t i = 0; i < browser_action_views_.size(); ++i) {
+    if (browser_action_views_[i]->IsVisible())
+      ++visible_actions;
+  }
+
+  // Add the new browser action to the vector and the view hierarchy.
+  BrowserActionView* view = new BrowserActionView(extension, this);
+  browser_action_views_.push_back(view);
+  AddChildView(index, view);
+
+  // For details on why we do the following see the class comments in the
+  // header.
+
+  // Determine if we need to increase (we only do that if the container was
+  // showing all icons before the addition of this icon). We use -1 because
+  // we don't want to count the view that we just added.
+  if (visible_actions < browser_action_views_.size() - 1) {
+    // Some icons were hidden, don't increase the size of the container.
+    OnBrowserActionVisibilityChanged();
+  } else {
+    // Container was at max, increase the size of it by one icon.
+    animation_target_size_ = IconCountToWidth(visible_actions + 1);
+
+    // We don't want the chevron to appear while we animate. See documentation
+    // in the header for why we do this.
+    suppress_chevron_ = !chevron_->IsVisible();
+
+    // Animate!
+    resize_animation_->Reset();
+    resize_animation_->SetTweenType(SlideAnimation::NONE);
+    resize_animation_->Show();
+  }
+}
+
+void BrowserActionsContainer::BrowserActionRemoved(Extension* extension) {
+  if (popup_ && popup_->host()->extension() == extension)
+    HidePopup();
+
+  // Before we change anything, determine the number of visible browser actions.
+  int visible_actions = 0;
+  for (size_t i = 0; i < browser_action_views_.size(); ++i) {
+    if (browser_action_views_[i]->IsVisible())
+      ++visible_actions;
+  }
+
+  for (std::vector<BrowserActionView*>::iterator iter =
+       browser_action_views_.begin(); iter != browser_action_views_.end();
+       ++iter) {
+    if ((*iter)->button()->extension() == extension) {
+      RemoveChildView(*iter);
+      delete *iter;
+      browser_action_views_.erase(iter);
+
+      // For details on why we do the following see the class comments in the
+      // header.
+
+      // Calculate the target size we'll animate to (end state). This might be
+      // the same size (if the icon we are removing is in the overflow bucket
+      // and there are other icons there). We don't decrement visible_actions
+      // because we want the container to stay the same size (clamping will take
+      // care of shrinking the container if there aren't enough icons to show).
+      animation_target_size_ =
+          ClampToNearestIconCount(IconCountToWidth(visible_actions));
+
+      // Animate!
+      resize_animation_->Reset();
+      resize_animation_->SetTweenType(SlideAnimation::EASE_OUT);
+      resize_animation_->Show();
+      return;
+    }
+  }
+}
+
 int BrowserActionsContainer::WidthOfNonIconArea() const {
   int chevron_size = (chevron_->IsVisible()) ?
                      chevron_->GetPreferredSize().width() : 0;
diff --git a/chrome/browser/views/browser_actions_container.h b/chrome/browser/views/browser_actions_container.h
index d2b87a4..8cf1815 100644
--- a/chrome/browser/views/browser_actions_container.h
+++ b/chrome/browser/views/browser_actions_container.h
@@ -8,6 +8,7 @@
 #include <vector>
 
 #include "base/task.h"
+#include "chrome/browser/extensions/extension_toolbar_model.h"
 #include "chrome/browser/extensions/image_loading_tracker.h"
 #include "chrome/browser/views/browser_bubble.h"
 #include "chrome/browser/views/extensions/extension_action_context_menu.h"
@@ -215,7 +216,8 @@ class BrowserActionsContainer
     public BrowserBubble::Delegate,
     public views::ViewMenuDelegate,
     public views::ResizeGripper::ResizeGripperDelegate,
-    public AnimationDelegate {
+    public AnimationDelegate,
+    public ExtensionToolbarModel::Observer {
  public:
   BrowserActionsContainer(Profile* profile, ToolbarView* toolbar);
   virtual ~BrowserActionsContainer();
@@ -295,13 +297,9 @@ class BrowserActionsContainer
   ExtensionPopup* TestGetPopup() { return popup_; }
 
  private:
-  // Adds a browser action view for the extension if it needs one. DCHECK if
-  // it has already been added.
-  void AddBrowserAction(Extension* extension);
-
-  // Removes the browser action view for an extension if it has one. DCHECK if
-  // no such view.
-  void RemoveBrowserAction(Extension* extension);
+  // ExtensionToolbarModel::Observer implementation.
+  virtual void BrowserActionAdded(Extension* extension, int index);
+  virtual void BrowserActionRemoved(Extension* extension);
 
   // Takes a width in pixels, calculates how many icons fit within that space
   // (up to the maximum number of icons in our vector) and shaves off the
@@ -338,6 +336,9 @@ class BrowserActionsContainer
   // from browser_action_views_).
   BrowserActionButton* popup_button_;
 
+  // The model that tracks the order of the toolbar icons.
+  ExtensionToolbarModel* model_;
+
   // The current size of the container.
   gfx::Size container_size_;