Refactor BrowserActions, and add support for

tab-specific state.

Future changelists will move Page Actions over to
ExtensionAction2, then replace ExtensionAction and
ExtensionActionState with ExtensionAction2.

Also, fix a bug in setIcon({path:...}) where it
would work only the first time.

BUG=24669,24472

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

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

4 files changed, 165 insertions, 101 deletions

diff --git a/chrome/browser/views/browser_actions_container.cc b/chrome/browser/views/browser_actions_container.cc
index d21984b..f3f02f0 100644
--- a/chrome/browser/views/browser_actions_container.cc
+++ b/chrome/browser/views/browser_actions_container.cc
@@ -16,6 +16,7 @@
 #include "chrome/browser/views/extensions/extension_popup.h"
 #include "chrome/browser/views/toolbar_view.h"
 #include "chrome/common/extensions/extension_action.h"
+#include "chrome/common/extensions/extension_action2.h"
 #include "chrome/common/notification_source.h"
 #include "chrome/common/notification_type.h"
 #include "grit/app_resources.h"
@@ -48,34 +49,20 @@ static const int kMinimumNumberOfVisibleBrowserActions = 2;
 ////////////////////////////////////////////////////////////////////////////////
 // BrowserActionButton
 
-BrowserActionButton::BrowserActionButton(
-    ExtensionAction* browser_action, Extension* extension,
-    BrowserActionsContainer* panel)
+BrowserActionButton::BrowserActionButton(Extension* extension,
+                                         BrowserActionsContainer* panel)
     : MenuButton(this, L"", NULL, false),
-      browser_action_(browser_action),
-      browser_action_state_(extension->browser_action_state()),
+      browser_action_(extension->browser_action()),
+      extension_(extension),
       tracker_(NULL),
       panel_(panel) {
   set_alignment(TextButton::ALIGN_CENTER);
 
-  // Load the images this view needs asynchronously on the file thread. We'll
-  // get a call back into OnImageLoaded if the image loads successfully. If not,
-  // the ImageView will have no image and will not appear in the browser chrome.
-  if (!browser_action->icon_paths().empty()) {
-    const std::vector<std::string>& icon_paths = browser_action->icon_paths();
-    browser_action_icons_.resize(icon_paths.size());
-    tracker_ = new ImageLoadingTracker(this, icon_paths.size());
-    for (std::vector<std::string>::const_iterator iter = icon_paths.begin();
-         iter != icon_paths.end(); ++iter) {
-      tracker_->PostLoadImageTask(
-          extension->GetResource(*iter),
-          gfx::Size(Extension::kBrowserActionIconMaxSize,
-                    Extension::kBrowserActionIconMaxSize));
-    }
-  }
+  // No UpdateState() here because View heirarchy not setup yet. Our parent
+  // should call UpdateState() after creation.
 
   registrar_.Add(this, NotificationType::EXTENSION_BROWSER_ACTION_UPDATED,
-                 Source<ExtensionAction>(browser_action_));
+                 Source<ExtensionAction2>(browser_action_));
 }
 
 BrowserActionButton::~BrowserActionButton() {
@@ -95,29 +82,39 @@ void BrowserActionButton::ButtonPressed(
   panel_->OnBrowserActionExecuted(this);
 }
 
+void BrowserActionButton::LoadImage() {
+  // Load the default image from the browser action asynchronously on the file
+  // thread. We'll get a call back into OnImageLoaded if the image loads
+  // successfully.
+  std::string relative_path = browser_action()->GetDefaultIconPath();
+  if (relative_path.empty())
+    return;
+
+  tracker_ = new ImageLoadingTracker(this, 1);
+  tracker_->PostLoadImageTask(
+      extension()->GetResource(relative_path),
+      gfx::Size(Extension::kBrowserActionIconMaxSize,
+                Extension::kBrowserActionIconMaxSize));
+}
+
 void BrowserActionButton::OnImageLoaded(SkBitmap* image, size_t index) {
-  DCHECK(index < browser_action_icons_.size());
-  browser_action_icons_[index] = image ? *image : SkBitmap();
-  if (index == browser_action_icons_.size() - 1) {
-    OnStateUpdated();
-    tracker_ = NULL;  // The tracker object will delete itself when we return.
-  }
+  SetIcon(*image);
+  tracker_ = NULL;  // The tracker object will delete itself when we return.
+  GetParent()->SchedulePaint();
 }
 
-void BrowserActionButton::OnStateUpdated() {
-  SkBitmap* image = browser_action_state_->icon();
-  if (!image) {
-    if (static_cast<size_t>(browser_action_state_->icon_index()) <
-        browser_action_icons_.size()) {
-      image = &browser_action_icons_[browser_action_state_->icon_index()];
-    }
-  }
+void BrowserActionButton::UpdateState() {
+  int tab_id = panel_->GetCurrentTabId();
+  if (tab_id < 0)
+    return;
 
-  if (image)
-    SetIcon(*image);
+  SkBitmap image = browser_action()->GetIcon(tab_id);
+  if (image.isNull())
+    LoadImage();
+  else
+    SetIcon(image);
 
-  SetTooltipText(ASCIIToWide(browser_action_state_->title()));
-  panel_->OnBrowserActionVisibilityChanged();
+  SetTooltipText(ASCIIToWide(browser_action()->GetTitle(tab_id)));
   GetParent()->SchedulePaint();
 }
 
@@ -125,14 +122,14 @@ void BrowserActionButton::Observe(NotificationType type,
                                   const NotificationSource& source,
                                   const NotificationDetails& details) {
   if (type == NotificationType::EXTENSION_BROWSER_ACTION_UPDATED) {
-    OnStateUpdated();
+    UpdateState();
   } else {
     NOTREACHED() << L"Received unexpected notification";
   }
 }
 
 bool BrowserActionButton::IsPopup() {
-  return browser_action_->is_popup();
+  return browser_action_->has_popup();
 }
 
 bool BrowserActionButton::Activate() {
@@ -195,11 +192,12 @@ void BrowserActionButton::PopupDidHide() {
 ////////////////////////////////////////////////////////////////////////////////
 // BrowserActionView
 
-BrowserActionView::BrowserActionView(ExtensionAction* browser_action,
-                                     Extension* extension,
-                                     BrowserActionsContainer* panel) {
-  button_ = new BrowserActionButton(browser_action, extension, panel);
+BrowserActionView::BrowserActionView(Extension* extension,
+                                     BrowserActionsContainer* panel)
+    : panel_(panel) {
+  button_ = new BrowserActionButton(extension, panel);
   AddChildView(button_);
+  button_->UpdateState();
 }
 
 void BrowserActionView::Layout() {
@@ -208,8 +206,16 @@ void BrowserActionView::Layout() {
 
 void BrowserActionView::PaintChildren(gfx::Canvas* canvas) {
   View::PaintChildren(canvas);
-  button_->browser_action_state()->PaintBadge(canvas,
-                                              gfx::Rect(width(), height()));
+  ExtensionAction2* action = button()->browser_action();
+  int tab_id = panel_->GetCurrentTabId();
+  if (tab_id < 0)
+    return;
+
+  ExtensionActionState::PaintBadge(
+    canvas, gfx::Rect(width(), height()),
+    action->GetBadgeText(tab_id),
+    action->GetBadgeTextColor(tab_id),
+    action->GetBadgeBackgroundColor(tab_id));
 }
 
 
@@ -224,6 +230,9 @@ BrowserActionsContainer::BrowserActionsContainer(
       popup_button_(NULL),
       ALLOW_THIS_IN_INITIALIZER_LIST(task_factory_(this)) {
   ExtensionsService* extension_service = profile->GetExtensionsService();
+  if (!extension_service)  // The |extension_service| can be NULL in Incognito.
+    return;
+
   registrar_.Add(this, NotificationType::EXTENSION_LOADED,
                  Source<ExtensionsService>(extension_service));
   registrar_.Add(this, NotificationType::EXTENSION_UNLOADED,
@@ -233,7 +242,9 @@ BrowserActionsContainer::BrowserActionsContainer(
   registrar_.Add(this, NotificationType::EXTENSION_HOST_VIEW_SHOULD_CLOSE,
                  Source<Profile>(profile_));
 
-  RefreshBrowserActionViews();
+  for (size_t i = 0; i < extension_service->extensions()->size(); ++i)
+    AddBrowserAction(extension_service->extensions()->at(i));
+
   SetID(VIEW_ID_BROWSER_ACTION_TOOLBAR);
 }
 
@@ -242,26 +253,50 @@ BrowserActionsContainer::~BrowserActionsContainer() {
   DeleteBrowserActionViews();
 }
 
+int BrowserActionsContainer::GetCurrentTabId() {
+  TabContents* tab_contents = toolbar_->browser()->GetSelectedTabContents();
+  if (!tab_contents)
+    return -1;
+
+  return tab_contents->controller().session_id().id();
+}
+
 void BrowserActionsContainer::RefreshBrowserActionViews() {
-  ExtensionsService* extension_service = profile_->GetExtensionsService();
-  if (!extension_service)  // The |extension_service| can be NULL in Incognito.
+  for (size_t i = 0; i < browser_action_views_.size(); ++i)
+    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;
 
-  // Get all browser actions, including those with popups.
-  std::vector<ExtensionAction*> browser_actions;
-  browser_actions = extension_service->GetBrowserActions(true);
+  BrowserActionView* view = new BrowserActionView(extension, this);
+  browser_action_views_.push_back(view);
+  AddChildView(view);
+}
 
-  DeleteBrowserActionViews();
-  for (size_t i = 0; i < browser_actions.size(); ++i) {
-    Extension* extension = extension_service->GetExtensionById(
-        browser_actions[i]->extension_id());
-    DCHECK(extension);
-
-    BrowserActionView* view =
-        new BrowserActionView(browser_actions[i], extension, this);
-    browser_action_views_.push_back(view);
-    AddChildView(view);
+void BrowserActionsContainer::RemoveBrowserAction(Extension* extension) {
+  if (!extension->browser_action())
+    return;
+
+  for (std::vector<BrowserActionView*>::iterator iter =
+       browser_action_views_.begin(); iter != browser_action_views_.end();
+       ++iter) {
+    if ((*iter)->button()->extension() == extension) {
+      RemoveChildView(*iter);
+      browser_action_views_.erase(iter);
+      return;
+    }
   }
+
+   NOTREACHED() << "Asked to remove a browser action view that doesn't exist.";
 }
 
 void BrowserActionsContainer::DeleteBrowserActionViews() {
@@ -306,7 +341,7 @@ void BrowserActionsContainer::TestExecuteBrowserAction(int index) {
 
 void BrowserActionsContainer::OnBrowserActionExecuted(
     BrowserActionButton* button) {
-  const ExtensionAction& browser_action = button->browser_action();
+  ExtensionAction2* browser_action = button->browser_action();
 
   // Popups just display.  No notification to the extension.
   // TODO(erikkay): should there be?
@@ -326,7 +361,7 @@ void BrowserActionsContainer::OnBrowserActionExecuted(
     gfx::Rect rect = button->bounds();
     rect.set_x(origin.x());
     rect.set_y(origin.y());
-    popup_ = ExtensionPopup::Show(browser_action.popup_url(),
+    popup_ = ExtensionPopup::Show(browser_action->popup_url(),
                                   toolbar_->browser(),
                                   rect);
     popup_->set_delegate(this);
@@ -337,7 +372,7 @@ void BrowserActionsContainer::OnBrowserActionExecuted(
 
   // Otherwise, we send the action to the extension.
   ExtensionBrowserEventRouter::GetInstance()->BrowserActionExecuted(
-      profile_, browser_action.extension_id(), toolbar_->browser());
+      profile_, browser_action->extension_id(), toolbar_->browser());
 }
 
 gfx::Size BrowserActionsContainer::GetPreferredSize() {
@@ -367,20 +402,26 @@ void BrowserActionsContainer::Layout() {
 void BrowserActionsContainer::Observe(NotificationType type,
                                       const NotificationSource& source,
                                       const NotificationDetails& details) {
-  if (type == NotificationType::EXTENSION_LOADED ||
-      type == NotificationType::EXTENSION_UNLOADED ||
-      type == NotificationType::EXTENSION_UNLOADED_DISABLED) {
-    RefreshBrowserActionViews();
-
-    // All these actions may change visibility of BrowserActions.
-    OnBrowserActionVisibilityChanged();
-  } else if (type == NotificationType::EXTENSION_HOST_VIEW_SHOULD_CLOSE) {
-    if (Details<ExtensionHost>(popup_->host()) != details)
-      return;
-
-    HidePopup();
-  } else {
-    NOTREACHED() << L"Received unexpected notification";
+  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 (Details<ExtensionHost>(popup_->host()) != details)
+        return;
+
+      HidePopup();
+
+    default:
+      NOTREACHED() << L"Unexpected notification";
   }
 }
 
diff --git a/chrome/browser/views/browser_actions_container.h b/chrome/browser/views/browser_actions_container.h
index e365bcd..756de0a 100644
--- a/chrome/browser/views/browser_actions_container.h
+++ b/chrome/browser/views/browser_actions_container.h
@@ -17,8 +17,7 @@
 
 class BrowserActionsContainer;
 class Extension;
-class ExtensionAction;
-class ExtensionActionState;
+class ExtensionAction2;
 class ExtensionPopup;
 class Profile;
 class ToolbarView;
@@ -27,20 +26,21 @@ class ToolbarView;
 // BrowserActionButton
 
 // The BrowserActionButton is a specialization of the MenuButton class.
-// It acts on a ExtensionAction, in this case a BrowserAction and handles
+// It acts on a ExtensionAction2, in this case a BrowserAction and handles
 // loading the image for the button asynchronously on the file thread to
 class BrowserActionButton : public views::MenuButton,
                             public views::ButtonListener,
                             public ImageLoadingTracker::Observer,
                             public NotificationObserver {
  public:
-  BrowserActionButton(ExtensionAction* browser_action,
-                      Extension* extension,
-                      BrowserActionsContainer* panel);
+  BrowserActionButton(Extension* extension, BrowserActionsContainer* panel);
   ~BrowserActionButton();
 
-  const ExtensionAction& browser_action() const { return *browser_action_; }
-  ExtensionActionState* browser_action_state() { return browser_action_state_; }
+  ExtensionAction2* browser_action() const { return browser_action_; }
+  Extension* extension() { return extension_; }
+
+  // Called to update the display to match the browser action's state.
+  void UpdateState();
 
   // Overriden from views::View. Return a 0-inset so the icon can draw all the
   // way to the edge of the view if it wants.
@@ -76,15 +76,15 @@ class BrowserActionButton : public views::MenuButton,
   virtual void PopupDidHide();
 
  private:
-  // Called to update the display to match the browser action's state.
-  void OnStateUpdated();
+  // If the image from the browser action needs to be loaded, load it.
+  void LoadImage();
 
-  // The browser action this view represents. The ExtensionAction is not owned
+  // The browser action this view represents. The ExtensionAction2 is not owned
   // by this class.
-  ExtensionAction* browser_action_;
+  ExtensionAction2* browser_action_;
 
-  // The state of our browser action. Not owned by this class.
-  ExtensionActionState* browser_action_state_;
+  // The extension associated with the browser action we're displaying.
+  Extension* extension_;
 
   // The icons representing different states for the browser action.
   std::vector<SkBitmap> browser_action_icons_;
@@ -110,9 +110,7 @@ class BrowserActionButton : public views::MenuButton,
 
 class BrowserActionView : public views::View {
  public:
-  BrowserActionView(ExtensionAction* browser_action, Extension* extension,
-                    BrowserActionsContainer* panel);
-
+  BrowserActionView(Extension* extension, BrowserActionsContainer* panel);
   BrowserActionButton* button() { return button_; }
 
  private:
@@ -121,6 +119,9 @@ class BrowserActionView : public views::View {
   // Override PaintChildren so that we can paint the badge on top of children.
   virtual void PaintChildren(gfx::Canvas* canvas);
 
+  // The container for this view.
+  BrowserActionsContainer* panel_;
+
   // The button this view contains.
   BrowserActionButton* button_;
 };
@@ -143,6 +144,9 @@ class BrowserActionsContainer : public views::View,
   // Get the number of browser actions being displayed.
   int num_browser_actions() { return browser_action_views_.size(); }
 
+  // Returns the current tab's ID, or -1 if there is no current tab.
+  int GetCurrentTabId();
+
   // Get a particular browser action view.
   BrowserActionView* GetBrowserActionViewAt(int index) {
     return browser_action_views_[index];
@@ -192,6 +196,14 @@ class BrowserActionsContainer : public views::View,
   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);
+
   // The vector of browser actions (icons/image buttons for each action).
   std::vector<BrowserActionView*> browser_action_views_;
 
diff --git a/chrome/browser/views/location_bar_view.cc b/chrome/browser/views/location_bar_view.cc
index ff6034b..70ce478 100644
--- a/chrome/browser/views/location_bar_view.cc
+++ b/chrome/browser/views/location_bar_view.cc
@@ -109,8 +109,12 @@ void LocationBarView::PageActionWithBadgeView::PaintChildren(
     gfx::Canvas* canvas) {
   View::PaintChildren(canvas);
   const ExtensionActionState* state = image_view_->GetPageActionState();
-  if (state)
-    state->PaintBadge(canvas, gfx::Rect(width(), height()));
+  if (state) {
+    ExtensionActionState::PaintBadge(canvas, gfx::Rect(width(), height()),
+                                     state->badge_text(),
+                                     state->badge_text_color(),
+                                     state->badge_background_color());
+  }
 }
 
 void LocationBarView::PageActionWithBadgeView::UpdateVisibility(
@@ -1230,8 +1234,12 @@ void LocationBarView::PageActionImageView::Paint(gfx::Canvas* canvas) {
 
   const ExtensionActionState* state =
       contents->GetPageActionState(page_action_);
-  if (state)
-    state->PaintBadge(canvas, gfx::Rect(width(), height()));
+  if (state) {
+    ExtensionActionState::PaintBadge(canvas, gfx::Rect(width(), height()),
+                                     state->badge_text(),
+                                     state->badge_text_color(),
+                                     state->badge_background_color());
+  }
 }
 
 // PageActionImageView----------------------------------------------------------
diff --git a/chrome/browser/views/toolbar_view.cc b/chrome/browser/views/toolbar_view.cc
index f5732c16..8853710 100644
--- a/chrome/browser/views/toolbar_view.cc
+++ b/chrome/browser/views/toolbar_view.cc
@@ -218,6 +218,9 @@ void ToolbarView::SetProfile(Profile* profile) {
 void ToolbarView::Update(TabContents* tab, bool should_restore_state) {
   if (location_bar_)
     location_bar_->Update(should_restore_state ? tab : NULL);
+
+  if (browser_actions_)
+    browser_actions_->RefreshBrowserActionViews();
 }
 
 int ToolbarView::GetNextAccessibleViewIndex(int view_index, bool nav_left) {