Only return the visible page actions from PageActionController.

This is a bit of a yak shave to support having the "action box" show active
extensions on the current page rather than page actions. Previously,
PageActionController was creating widgets for everything then
LocationBarViewGtk was hiding them as necessary. This is assumes too much; we
need to push the visibility logic into the Controller for the future.

BUG=127988


Review URL: https://chromiumcodereview.appspot.com/10388160

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

12 files changed, 225 insertions, 161 deletions

diff --git a/chrome/browser/extensions/action_box_controller.cc b/chrome/browser/extensions/action_box_controller.cc
new file mode 100644
index 0000000..13a8f17
--- /dev/null
+++ b/chrome/browser/extensions/action_box_controller.cc
@@ -0,0 +1,23 @@
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/extensions/action_box_controller.h"
+
+#include <algorithm>
+
+namespace extensions {
+
+// static
+void ActionBoxController::AddMissingActions(
+    const std::set<ExtensionAction*>& actions,
+    std::vector<ExtensionAction*>* out) {
+  for (std::set<ExtensionAction*>::const_iterator it = actions.begin();
+       it != actions.end(); ++it) {
+    ExtensionAction* action = (*it);
+    if (!std::count(out->begin(), out->end(), action))
+      out->push_back(action);
+  }
+}
+
+}  // namespace extensions
diff --git a/chrome/browser/extensions/action_box_controller.h b/chrome/browser/extensions/action_box_controller.h
index e3415c7..e85e37e3 100644
--- a/chrome/browser/extensions/action_box_controller.h
+++ b/chrome/browser/extensions/action_box_controller.h
@@ -6,6 +6,7 @@
 #define CHROME_BROWSER_EXTENSIONS_ACTION_BOX_CONTROLLER_H_
 #pragma once
 
+#include <set>
 #include <string>
 #include <vector>
 
@@ -18,22 +19,6 @@ namespace extensions {
 // Controller of the "badges" (aka "page actions") in the UI.
 class ActionBoxController {
  public:
-  // UI decoration on a page box item.
-  enum Decoration {
-    DECORATION_NONE,
-  };
-
-  // Data about a UI badge.
-  struct Data {
-    // The type of decoration that should be applied to the badge.
-    Decoration decoration;
-
-    // The ExtensionAction that corresponds to the badge.
-    ExtensionAction* action;
-  };
-
-  typedef std::vector<Data> DataList;
-
   // The reaction that the UI should take after executing |OnClicked|.
   enum Action {
     ACTION_NONE,
@@ -43,8 +28,13 @@ class ActionBoxController {
 
   virtual ~ActionBoxController() {}
 
-  // Gets the badge data for all extensions.
-  virtual scoped_ptr<DataList> GetAllBadgeData() = 0;
+  // Utility to add any actions to |out| which aren't present in |actions|.
+  static void AddMissingActions(
+      const std::set<ExtensionAction*>& actions,
+      std::vector<ExtensionAction*>* out);
+
+  // Gets the action data for all extensions.
+  virtual scoped_ptr<std::vector<ExtensionAction*> > GetCurrentActions() = 0;
 
   // Notifies this that the badge for an extension has been clicked with some
   // mouse button (1 for left, 2 for middle, and 3 for right click), and
diff --git a/chrome/browser/extensions/execute_code_in_tab_function.cc b/chrome/browser/extensions/execute_code_in_tab_function.cc
index b2c4e7a..d50afc6 100644
--- a/chrome/browser/extensions/execute_code_in_tab_function.cc
+++ b/chrome/browser/extensions/execute_code_in_tab_function.cc
@@ -9,6 +9,7 @@
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_tab_util.h"
+#include "chrome/browser/extensions/extension_tab_helper.h"
 #include "chrome/browser/extensions/extension_tabs_module.h"
 #include "chrome/browser/extensions/extension_tabs_module_constants.h"
 #include "chrome/browser/extensions/file_reader.h"
@@ -244,7 +245,7 @@ bool ExecuteCodeInTabFunction::Execute(const std::string& code_string) {
     NOTREACHED();
   }
 
-  contents->extension_script_executor()->ExecuteScript(
+  contents->extension_tab_helper()->script_executor()->ExecuteScript(
       extension->id(),
       script_type,
       code_string,
diff --git a/chrome/browser/extensions/extension_tab_helper.cc b/chrome/browser/extensions/extension_tab_helper.cc
index ce76ddc..627417d 100644
--- a/chrome/browser/extensions/extension_tab_helper.cc
+++ b/chrome/browser/extensions/extension_tab_helper.cc
@@ -5,6 +5,8 @@
 #include "chrome/browser/extensions/extension_tab_helper.h"
 
 #include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/extensions/page_action_controller.h"
+#include "chrome/browser/extensions/script_executor_impl.h"
 #include "chrome/browser/extensions/webstore_inline_installer.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/sessions/restore_tab_helper.h"
@@ -27,6 +29,8 @@
 #include "ui/gfx/image/image.h"
 
 using content::WebContents;
+using extensions::ScriptExecutorImpl;
+using extensions::PageActionController;
 
 namespace {
 
@@ -41,6 +45,8 @@ ExtensionTabHelper::ExtensionTabHelper(TabContentsWrapper* wrapper)
       ALLOW_THIS_IN_INITIALIZER_LIST(
           extension_function_dispatcher_(wrapper->profile(), this)),
       wrapper_(wrapper) {
+  script_executor_.reset(new ScriptExecutorImpl(wrapper->web_contents()));
+  action_box_controller_.reset(new PageActionController(wrapper, this));
 }
 
 ExtensionTabHelper::~ExtensionTabHelper() {
@@ -52,14 +58,26 @@ void ExtensionTabHelper::CopyStateFrom(const ExtensionTabHelper& source) {
 }
 
 void ExtensionTabHelper::PageActionStateChanged() {
+  // TODO(kalman): replace this with just the Observer interface.
   web_contents()->NotifyNavigationStateChanged(
       content::INVALIDATE_TYPE_PAGE_ACTIONS);
+
+  FOR_EACH_OBSERVER(Observer, observers_, OnPageActionStateChanged());
 }
 
 void ExtensionTabHelper::GetApplicationInfo(int32 page_id) {
   Send(new ExtensionMsg_GetApplicationInfo(routing_id(), page_id));
 }
 
+void ExtensionTabHelper::AddObserver(ExtensionTabHelper::Observer* observer) {
+  observers_.AddObserver(observer);
+}
+
+void ExtensionTabHelper::RemoveObserver(
+    ExtensionTabHelper::Observer* observer) {
+  observers_.RemoveObserver(observer);
+}
+
 void ExtensionTabHelper::SetExtensionApp(const Extension* extension) {
   DCHECK(!extension || extension->GetFullLaunchURL().is_valid());
   extension_app_ = extension;
diff --git a/chrome/browser/extensions/extension_tab_helper.h b/chrome/browser/extensions/extension_tab_helper.h
index 16b4f5f..c095082 100644
--- a/chrome/browser/extensions/extension_tab_helper.h
+++ b/chrome/browser/extensions/extension_tab_helper.h
@@ -7,6 +7,7 @@
 #pragma once
 
 #include "base/memory/weak_ptr.h"
+#include "base/observer_list.h"
 #include "chrome/browser/extensions/app_notify_channel_setup.h"
 #include "chrome/browser/extensions/extension_function_dispatcher.h"
 #include "chrome/browser/extensions/image_loading_tracker.h"
@@ -24,6 +25,11 @@ namespace content {
 struct LoadCommittedDetails;
 }
 
+namespace extensions {
+class ActionBoxController;
+class ScriptExecutor;
+}
+
 // Per-tab extension helper. Also handles non-extension apps.
 class ExtensionTabHelper
     : public content::WebContentsObserver,
@@ -33,6 +39,15 @@ class ExtensionTabHelper
       public AppNotifyChannelSetup::Delegate,
       public base::SupportsWeakPtr<ExtensionTabHelper> {
  public:
+  class Observer {
+   public:
+    // Called when the page action state (such as visibility, title) changes.
+    virtual void OnPageActionStateChanged() = 0;
+
+   protected:
+    virtual ~Observer() {}
+  };
+
   explicit ExtensionTabHelper(TabContentsWrapper* wrapper);
   virtual ~ExtensionTabHelper();
 
@@ -50,6 +65,10 @@ class ExtensionTabHelper
   // the data is available.
   void GetApplicationInfo(int32 page_id);
 
+  // Observer management.
+  void AddObserver(Observer* observer);
+  void RemoveObserver(Observer* observer);
+
   // App extensions ------------------------------------------------------------
 
   // Sets the extension denoting this as an app. If |extension| is non-null this
@@ -90,6 +109,14 @@ class ExtensionTabHelper
     return content::WebContentsObserver::web_contents();
   }
 
+  extensions::ScriptExecutor* script_executor() {
+    return script_executor_.get();
+  }
+
+  extensions::ActionBoxController* action_box_controller() {
+    return action_box_controller_.get();
+  }
+
   // Sets a non-extension app icon associated with WebContents and fires an
   // INVALIDATE_TYPE_TITLE navigation state change to trigger repaint of title.
   void SetAppIcon(const SkBitmap& app_icon);
@@ -172,6 +199,12 @@ class ExtensionTabHelper
 
   TabContentsWrapper* wrapper_;
 
+  scoped_ptr<extensions::ScriptExecutor> script_executor_;
+
+  scoped_ptr<extensions::ActionBoxController> action_box_controller_;
+
+  ObserverList<Observer> observers_;
+
   DISALLOW_COPY_AND_ASSIGN(ExtensionTabHelper);
 };
 
diff --git a/chrome/browser/extensions/extension_tabs_module.cc b/chrome/browser/extensions/extension_tabs_module.cc
index 7f15efc..dc1cc14 100644
--- a/chrome/browser/extensions/extension_tabs_module.cc
+++ b/chrome/browser/extensions/extension_tabs_module.cc
@@ -1320,7 +1320,7 @@ bool UpdateTabFunction::UpdateURLIfPresent(DictionaryValue* update_props,
       return false;
     }
 
-    tab_contents_->extension_script_executor()->ExecuteScript(
+    tab_contents_->extension_tab_helper()->script_executor()->ExecuteScript(
         extension_id(),
         ScriptExecutor::JAVASCRIPT,
         url.path(),
diff --git a/chrome/browser/extensions/page_action_controller.cc b/chrome/browser/extensions/page_action_controller.cc
index c21b768..ed3c8e6 100644
--- a/chrome/browser/extensions/page_action_controller.cc
+++ b/chrome/browser/extensions/page_action_controller.cc
@@ -8,33 +8,39 @@
 #include "chrome/browser/extensions/extension_browser_event_router.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_system.h"
+#include "chrome/browser/extensions/extension_tab_helper.h"
 #include "chrome/browser/extensions/extension_tab_util.h"
 #include "chrome/common/extensions/extension_set.h"
+#include "chrome/common/chrome_notification_types.h"
+#include "content/public/browser/notification_service.h"
 #include "content/public/browser/web_contents.h"
 
 namespace extensions {
 
-PageActionController::PageActionController(TabContentsWrapper* tab_contents)
-    : tab_contents_(tab_contents) {}
+PageActionController::PageActionController(TabContentsWrapper* tab_contents,
+                                           ExtensionTabHelper* tab_helper)
+    : tab_contents_(tab_contents),
+      tab_helper_(tab_helper) {
+  tab_helper->AddObserver(this);
+}
 
-PageActionController::~PageActionController() {}
+PageActionController::~PageActionController() {
+  tab_helper_->RemoveObserver(this);
+}
 
-scoped_ptr<ActionBoxController::DataList>
-PageActionController::GetAllBadgeData() {
+scoped_ptr<std::vector<ExtensionAction*> >
+PageActionController::GetCurrentActions() {
+  int tab_id = ExtensionTabUtil::GetTabId(tab_contents_->web_contents());
   const ExtensionSet* extensions = GetExtensionService()->extensions();
-  scoped_ptr<DataList> all_badge_data(new DataList());
+  scoped_ptr<std::vector<ExtensionAction*> > current_actions(
+      new std::vector<ExtensionAction*>());
   for (ExtensionSet::const_iterator i = extensions->begin();
        i != extensions->end(); ++i) {
     ExtensionAction* action = (*i)->page_action();
-    if (action) {
-      Data data = {
-        DECORATION_NONE,
-        action,
-      };
-      all_badge_data->push_back(data);
-    }
+    if (action && action->GetIsVisible(tab_id))
+      current_actions->push_back(action);
   }
-  return all_badge_data.Pass();
+  return current_actions.Pass();
 }
 
 ActionBoxController::Action PageActionController::OnClicked(
@@ -69,6 +75,25 @@ ActionBoxController::Action PageActionController::OnClicked(
   return ACTION_NONE;
 }
 
+void PageActionController::OnPageActionStateChanged() {
+  content::NotificationService::current()->Notify(
+      chrome::NOTIFICATION_EXTENSION_ACTION_BOX_UPDATED,
+      content::Source<Profile>(tab_contents_->profile()),
+      content::Details<TabContentsWrapper>(tab_contents_));
+
+  // TODO(kalman): remove this, and all occurrences of
+  // NOTIFICATION_EXTENSION_PAGE_ACTION_VISIBILITY_CHANGED, when views and
+  // cocoa have been updated to not use it.
+  //
+  // Only tests care about them, and they only ever use AllSources, so it can
+  // safely be a bogus value.
+  ExtensionAction bogus_action("");
+  content::NotificationService::current()->Notify(
+      chrome::NOTIFICATION_EXTENSION_PAGE_ACTION_VISIBILITY_CHANGED,
+      content::Source<ExtensionAction>(&bogus_action),
+      content::Details<content::WebContents>(tab_contents_->web_contents()));
+}
+
 ExtensionService* PageActionController::GetExtensionService() {
   return ExtensionSystem::Get(tab_contents_->profile())->extension_service();
 }
diff --git a/chrome/browser/extensions/page_action_controller.h b/chrome/browser/extensions/page_action_controller.h
index 22f9fc6..cdb43a3 100644
--- a/chrome/browser/extensions/page_action_controller.h
+++ b/chrome/browser/extensions/page_action_controller.h
@@ -6,7 +6,12 @@
 #define CHROME_BROWSER_EXTENSIONS_PAGE_ACTION_CONTROLLER_H_
 #pragma once
 
+#include <set>
+#include <string>
+
+#include "base/observer_list.h"
 #include "chrome/browser/extensions/action_box_controller.h"
+#include "chrome/browser/extensions/extension_tab_helper.h"
 
 class ExtensionService;
 class TabContentsWrapper;
@@ -14,22 +19,30 @@ class TabContentsWrapper;
 namespace extensions {
 
 // An ActionBoxController which corresponds to the page actions of an extension.
-class PageActionController : public ActionBoxController {
+class PageActionController : public ActionBoxController,
+                             public ExtensionTabHelper::Observer {
  public:
-  explicit PageActionController(TabContentsWrapper* tab_contents);
+  PageActionController(TabContentsWrapper* tab_contents,
+                       ExtensionTabHelper* tab_helper);
   virtual ~PageActionController();
 
-  virtual scoped_ptr<DataList> GetAllBadgeData() OVERRIDE;
-
+  // ActionBoxController implementation.
+  virtual scoped_ptr<std::vector<ExtensionAction*> > GetCurrentActions()
+      OVERRIDE;
   virtual Action OnClicked(const std::string& extension_id,
                            int mouse_button) OVERRIDE;
 
+  // ExtensionTabHelper::Observer implementation.
+  virtual void OnPageActionStateChanged() OVERRIDE;
+
  private:
   // Gets the ExtensionService for |tab_contents_|.
   ExtensionService* GetExtensionService();
 
   TabContentsWrapper* tab_contents_;
 
+  ExtensionTabHelper* tab_helper_;
+
   DISALLOW_COPY_AND_ASSIGN(PageActionController);
 };
 
diff --git a/chrome/browser/ui/gtk/location_bar_view_gtk.cc b/chrome/browser/ui/gtk/location_bar_view_gtk.cc
index 3672740..f3227fe 100644
--- a/chrome/browser/ui/gtk/location_bar_view_gtk.cc
+++ b/chrome/browser/ui/gtk/location_bar_view_gtk.cc
@@ -26,9 +26,9 @@
 #include "chrome/browser/defaults.h"
 #include "chrome/browser/extensions/api/commands/extension_command_service.h"
 #include "chrome/browser/extensions/api/commands/extension_command_service_factory.h"
-#include "chrome/browser/extensions/action_box_controller.h"
 #include "chrome/browser/extensions/extension_browser_event_router.h"
 #include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/extensions/extension_tab_helper.h"
 #include "chrome/browser/extensions/extension_tab_util.h"
 #include "chrome/browser/favicon/favicon_tab_helper.h"
 #include "chrome/browser/instant/instant_controller.h"
@@ -356,6 +356,10 @@ void LocationBarViewGtk::Init(bool popup_window_mode) {
   registrar_.Add(this,
                  chrome::NOTIFICATION_BROWSER_THEME_CHANGED,
                  content::Source<ThemeService>(theme_service_));
+  registrar_.Add(this,
+                 chrome::NOTIFICATION_EXTENSION_ACTION_BOX_UPDATED,
+                 content::Source<Profile>(browser()->profile()));
+
   edit_bookmarks_enabled_.Init(prefs::kEditBookmarksEnabled,
                                profile->GetPrefs(), this);
 
@@ -444,16 +448,10 @@ void LocationBarViewGtk::SetPreviewEnabledPageAction(
     ExtensionAction *page_action,
     bool preview_enabled) {
   DCHECK(page_action);
-  UpdatePageActions();
-  for (ScopedVector<PageActionViewGtk>::iterator iter =
-       page_action_views_.begin(); iter != page_action_views_.end();
-       ++iter) {
-    if ((*iter)->page_action() == page_action) {
-      (*iter)->set_preview_enabled(preview_enabled);
-      UpdatePageActions();
-      return;
-    }
-  }
+  if (preview_enabled && preview_enabled_actions_.insert(page_action).second)
+    UpdatePageActions();
+  if (!preview_enabled && preview_enabled_actions_.erase(page_action) > 0)
+    UpdatePageActions();
 }
 
 GtkWidget* LocationBarViewGtk::GetPageActionWidget(
@@ -685,14 +683,20 @@ void LocationBarViewGtk::UpdateContentSettingsIcons() {
 }
 
 void LocationBarViewGtk::UpdatePageActions() {
-  ActionBoxController::DataList page_actions;
+  std::vector<ExtensionAction*> page_actions;
 
   TabContentsWrapper* tab_contents = GetTabContentsWrapper();
   if (tab_contents) {
-    page_actions.swap(
-        *tab_contents->extension_action_box_controller()->GetAllBadgeData());
+    ActionBoxController* controller =
+        tab_contents->extension_tab_helper()->action_box_controller();
+    page_actions.swap(*controller->GetCurrentActions());
   }
 
+  // Add page actions for any extensions which have "preview enabled" and not
+  // already visible.
+  ActionBoxController::AddMissingActions(
+      preview_enabled_actions_, &page_actions);
+
   // Initialize on the first call, or re-inialize if more extensions have been
   // loaded or added after startup.
   if (page_actions.size() != page_action_views_.size()) {
@@ -700,7 +704,7 @@ void LocationBarViewGtk::UpdatePageActions() {
 
     for (size_t i = 0; i < page_actions.size(); ++i) {
       page_action_views_.push_back(
-          new PageActionViewGtk(this, page_actions[i].action));
+          new PageActionViewGtk(this, page_actions[i]));
       gtk_box_pack_end(GTK_BOX(page_action_hbox_.get()),
                        page_action_views_[i]->widget(), FALSE, FALSE, 0);
     }
@@ -715,7 +719,7 @@ void LocationBarViewGtk::UpdatePageActions() {
     GURL url = browser()->GetSelectedWebContents()->GetURL();
 
     for (size_t i = 0; i < page_action_views_.size(); i++) {
-      page_action_views_[i]->UpdateVisibility(
+      page_action_views_[i]->Update(
           toolbar_model_->input_in_progress() ? NULL : contents, url);
     }
   }
@@ -778,16 +782,7 @@ ExtensionAction* LocationBarViewGtk::GetPageAction(size_t index) {
 }
 
 ExtensionAction* LocationBarViewGtk::GetVisiblePageAction(size_t index) {
-  size_t visible_index = 0;
-  for (size_t i = 0; i < page_action_views_.size(); ++i) {
-    if (page_action_views_[i]->IsVisible()) {
-      if (index == visible_index++)
-        return page_action_views_[i]->page_action();
-    }
-  }
-
-  NOTREACHED();
-  return NULL;
+  return page_action_views_[index]->page_action();
 }
 
 void LocationBarViewGtk::TestPageActionPressed(size_t index) {
@@ -808,6 +803,15 @@ void LocationBarViewGtk::Observe(int type,
     return;
   }
 
+  if (type == chrome::NOTIFICATION_EXTENSION_ACTION_BOX_UPDATED) {
+    // Only update if the updated action box was for the active tab contents.
+    TabContentsWrapper* target_tab =
+        content::Details<TabContentsWrapper>(details).ptr();
+    if (target_tab == GetTabContentsWrapper())
+      UpdatePageActions();
+    return;
+  }
+
   DCHECK_EQ(type, chrome::NOTIFICATION_BROWSER_THEME_CHANGED);
 
   if (theme_service_->UsingNativeTheme()) {
@@ -1503,8 +1507,7 @@ LocationBarViewGtk::PageActionViewGtk::PageActionViewGtk(
       tracker_(this),
       current_tab_id_(-1),
       window_(NULL),
-      accel_group_(NULL),
-      preview_enabled_(false) {
+      accel_group_(NULL) {
   event_box_.Own(gtk_event_box_new());
   gtk_widget_set_size_request(event_box_.get(),
                               Extension::kPageActionIconMaxSize,
@@ -1521,6 +1524,7 @@ LocationBarViewGtk::PageActionViewGtk::PageActionViewGtk(
 
   image_.Own(gtk_image_new());
   gtk_container_add(GTK_CONTAINER(event_box_.get()), image_.get());
+  gtk_widget_show_all(event_box_.get());
 
   const Extension* extension = owner->browser()->profile()->
       GetExtensionService()->GetExtensionById(page_action->extension_id(),
@@ -1559,75 +1563,56 @@ LocationBarViewGtk::PageActionViewGtk::~PageActionViewGtk() {
     g_object_unref(last_icon_pixbuf_);
 }
 
-bool LocationBarViewGtk::PageActionViewGtk::IsVisible() {
-  return gtk_widget_get_visible(widget());
-}
-
-void LocationBarViewGtk::PageActionViewGtk::UpdateVisibility(
+void LocationBarViewGtk::PageActionViewGtk::Update(
     WebContents* contents, const GURL& url) {
   // Save this off so we can pass it back to the extension when the action gets
   // executed. See PageActionImageView::OnMousePressed.
   current_tab_id_ = contents ? ExtensionTabUtil::GetTabId(contents) : -1;
   current_url_ = url;
 
-  bool visible = contents &&
-      (preview_enabled_ || page_action_->GetIsVisible(current_tab_id_));
-  if (visible) {
-    // Set the tooltip.
-    gtk_widget_set_tooltip_text(event_box_.get(),
-        page_action_->GetTitle(current_tab_id_).c_str());
-
-    // Set the image.
-    // It can come from three places. In descending order of priority:
-    // - The developer can set it dynamically by path or bitmap. It will be in
-    //   page_action_->GetIcon().
-    // - The developer can set it dyanmically by index. It will be in
-    //   page_action_->GetIconIndex().
-    // - It can be set in the manifest by path. It will be in page_action_->
-    //   default_icon_path().
-
-    // First look for a dynamically set bitmap.
-    SkBitmap icon = page_action_->GetIcon(current_tab_id_);
-    GdkPixbuf* pixbuf = NULL;
-    if (!icon.isNull()) {
-      if (icon.pixelRef() != last_icon_skbitmap_.pixelRef()) {
-        if (last_icon_pixbuf_)
-          g_object_unref(last_icon_pixbuf_);
-        last_icon_skbitmap_ = icon;
-        last_icon_pixbuf_ = gfx::GdkPixbufFromSkBitmap(&icon);
-      }
-      DCHECK(last_icon_pixbuf_);
-      pixbuf = last_icon_pixbuf_;
-    } else {
-      // Otherwise look for a dynamically set index, or fall back to the
-      // default path.
-      int icon_index = page_action_->GetIconIndex(current_tab_id_);
-      std::string icon_path = (icon_index < 0) ?
-          page_action_->default_icon_path() :
-          page_action_->icon_paths()->at(icon_index);
-      if (!icon_path.empty()) {
-        PixbufMap::iterator iter = pixbufs_.find(icon_path);
-        if (iter != pixbufs_.end())
-          pixbuf = iter->second;
-      }
+  // Set the tooltip.
+  gtk_widget_set_tooltip_text(event_box_.get(),
+      page_action_->GetTitle(current_tab_id_).c_str());
+
+  // Set the image.
+  // It can come from three places. In descending order of priority:
+  // - The developer can set it dynamically by path or bitmap. It will be in
+  //   page_action_->GetIcon().
+  // - The developer can set it dyanmically by index. It will be in
+  //   page_action_->GetIconIndex().
+  // - It can be set in the manifest by path. It will be in page_action_->
+  //   default_icon_path().
+
+  // First look for a dynamically set bitmap.
+  SkBitmap icon = page_action_->GetIcon(current_tab_id_);
+  GdkPixbuf* pixbuf = NULL;
+
+  if (!icon.isNull()) {
+    if (icon.pixelRef() != last_icon_skbitmap_.pixelRef()) {
+      if (last_icon_pixbuf_)
+        g_object_unref(last_icon_pixbuf_);
+      last_icon_skbitmap_ = icon;
+      last_icon_pixbuf_ = gfx::GdkPixbufFromSkBitmap(&icon);
+    }
+    DCHECK(last_icon_pixbuf_);
+    pixbuf = last_icon_pixbuf_;
+  } else {
+    // Otherwise look for a dynamically set index, or fall back to the
+    // default path.
+    int icon_index = page_action_->GetIconIndex(current_tab_id_);
+    std::string icon_path = icon_index < 0 ?
+        page_action_->default_icon_path() :
+        page_action_->icon_paths()->at(icon_index);
+    if (!icon_path.empty()) {
+      PixbufMap::iterator iter = pixbufs_.find(icon_path);
+      if (iter != pixbufs_.end())
+        pixbuf = iter->second;
     }
-    // The pixbuf might not be loaded yet.
-    if (pixbuf)
-      gtk_image_set_from_pixbuf(GTK_IMAGE(image_.get()), pixbuf);
   }
 
-  bool old_visible = IsVisible();
-  if (visible)
-    gtk_widget_show_all(event_box_.get());
-  else
-    gtk_widget_hide_all(event_box_.get());
-
-  if (visible != old_visible) {
-    content::NotificationService::current()->Notify(
-        chrome::NOTIFICATION_EXTENSION_PAGE_ACTION_VISIBILITY_CHANGED,
-        content::Source<ExtensionAction>(page_action_),
-        content::Details<WebContents>(contents));
-  }
+  // The pixbuf might not be loaded yet.
+  if (pixbuf)
+    gtk_image_set_from_pixbuf(GTK_IMAGE(image_.get()), pixbuf);
 }
 
 void LocationBarViewGtk::PageActionViewGtk::OnImageLoaded(
@@ -1748,7 +1733,7 @@ gboolean LocationBarViewGtk::PageActionViewGtk::OnButtonPressed(
     return TRUE;
 
   ActionBoxController* controller =
-      tab_contents->extension_action_box_controller();
+      tab_contents->extension_tab_helper()->action_box_controller();
 
   switch (controller->OnClicked(extension->id(), event->button)) {
     case ActionBoxController::ACTION_NONE:
diff --git a/chrome/browser/ui/gtk/location_bar_view_gtk.h b/chrome/browser/ui/gtk/location_bar_view_gtk.h
index 4a0fbce..3f3d4d8 100644
--- a/chrome/browser/ui/gtk/location_bar_view_gtk.h
+++ b/chrome/browser/ui/gtk/location_bar_view_gtk.h
@@ -9,6 +9,7 @@
 #include <gtk/gtk.h>
 
 #include <map>
+#include <set>
 #include <string>
 
 #include "base/basictypes.h"
@@ -18,6 +19,7 @@
 #include "base/memory/weak_ptr.h"
 #include "chrome/browser/autocomplete/autocomplete_edit.h"
 #include "chrome/browser/command_updater.h"
+#include "chrome/browser/extensions/action_box_controller.h"
 #include "chrome/browser/extensions/extension_context_menu_model.h"
 #include "chrome/browser/extensions/image_loading_tracker.h"
 #include "chrome/browser/prefs/pref_member.h"
@@ -78,10 +80,9 @@ class LocationBarViewGtk : public AutocompleteEditController,
   // Returns the current WebContents.
   content::WebContents* GetWebContents() const;
 
-  // Sets |preview_enabled| for the PageActionViewGtk associated with this
-  // |page_action|. If |preview_enabled| is true, the view will display the
-  // page action's icon even though it has not been activated by the extension.
+  // If |preview_enabled| is true, the view will display the
   // This is used by the ExtensionInstalledBubbleGtk to preview what the icon
+  // page action's icon even though it has not been activated by the extension.
   // will look like for the user upon installation of the extension.
   void SetPreviewEnabledPageAction(ExtensionAction *page_action,
                                    bool preview_enabled);
@@ -228,16 +229,10 @@ class LocationBarViewGtk : public AutocompleteEditController,
 
     ExtensionAction* page_action() { return page_action_; }
 
-    void set_preview_enabled(bool preview_enabled) {
-      preview_enabled_ = preview_enabled;
-    }
-
-    bool IsVisible();
-
-    // Called to notify the PageAction that it should determine whether to be
-    // visible or hidden. |contents| is the WebContents that is active, |url|
+    // Called to notify the PageAction that it should update based on the state
+    // of |page_action_|. |contents| is the WebContents that is active, |url|
     // is the current page URL.
-    void UpdateVisibility(content::WebContents* contents, const GURL& url);
+    void Update(content::WebContents* contents, const GURL& url);
 
     // A callback from ImageLoadingTracker for when the image has loaded.
     virtual void OnImageLoaded(const gfx::Image& image,
@@ -316,10 +311,6 @@ class LocationBarViewGtk : public AutocompleteEditController,
     // The keybinding accelerator registered to show the page action popup.
     scoped_ptr<ui::AcceleratorGtk> keybinding_;
 
-    // This is used for post-install visual feedback. The page_action icon
-    // is briefly shown even if it hasn't been enabled by its extension.
-    bool preview_enabled_;
-
     // The context menu view and model for this extension action.
     scoped_ptr<MenuGtk> context_menu_;
     scoped_refptr<ExtensionContextMenuModel> context_menu_model_;
@@ -493,6 +484,9 @@ class LocationBarViewGtk : public AutocompleteEditController,
   // Used to change the visibility of the star decoration.
   BooleanPrefMember edit_bookmarks_enabled_;
 
+  // The extension actions which have "preview enabled".
+  std::set<ExtensionAction*> preview_enabled_actions_;
+
   DISALLOW_COPY_AND_ASSIGN(LocationBarViewGtk);
 };
 
diff --git a/chrome/browser/ui/tab_contents/tab_contents_wrapper.cc b/chrome/browser/ui/tab_contents/tab_contents_wrapper.cc
index 365ca3a..2010a43 100644
--- a/chrome/browser/ui/tab_contents/tab_contents_wrapper.cc
+++ b/chrome/browser/ui/tab_contents/tab_contents_wrapper.cc
@@ -14,8 +14,6 @@
 #include "chrome/browser/download/download_request_limiter_observer.h"
 #include "chrome/browser/extensions/api/web_navigation/web_navigation_api.h"
 #include "chrome/browser/extensions/extension_tab_helper.h"
-#include "chrome/browser/extensions/page_action_controller.h"
-#include "chrome/browser/extensions/script_executor_impl.h"
 #include "chrome/browser/external_protocol/external_protocol_observer.h"
 #include "chrome/browser/favicon/favicon_tab_helper.h"
 #include "chrome/browser/history/history_tab_helper.h"
@@ -95,10 +93,6 @@ TabContentsWrapper::TabContentsWrapper(WebContents* contents)
   constrained_window_tab_helper_.reset(new ConstrainedWindowTabHelper(this));
   core_tab_helper_.reset(new CoreTabHelper(contents));
   extension_tab_helper_.reset(new ExtensionTabHelper(this));
-  extension_script_executor_.reset(
-      new extensions::ScriptExecutorImpl(web_contents()));
-  extension_action_box_controller_.reset(
-      new extensions::PageActionController(this));
   favicon_tab_helper_.reset(new FaviconTabHelper(contents));
   find_tab_helper_.reset(new FindTabHelper(contents));
   history_tab_helper_.reset(new HistoryTabHelper(contents));
diff --git a/chrome/browser/ui/tab_contents/tab_contents_wrapper.h b/chrome/browser/ui/tab_contents/tab_contents_wrapper.h
index c4fe4cc4..3fc5e14 100644
--- a/chrome/browser/ui/tab_contents/tab_contents_wrapper.h
+++ b/chrome/browser/ui/tab_contents/tab_contents_wrapper.h
@@ -56,8 +56,6 @@ class SyncedTabDelegate;
 }
 
 namespace extensions {
-class ActionBoxController;
-class ScriptExecutor;
 class WebNavigationTabObserver;
 }
 
@@ -134,14 +132,6 @@ class TabContentsWrapper : public content::WebContentsObserver {
 
   CoreTabHelper* core_tab_helper() { return core_tab_helper_.get(); }
 
-  extensions::ScriptExecutor* extension_script_executor() {
-    return extension_script_executor_.get();
-  }
-
-  extensions::ActionBoxController* extension_action_box_controller() {
-    return extension_action_box_controller_.get();
-  }
-
   ExtensionTabHelper* extension_tab_helper() {
     return extension_tab_helper_.get();
   }
@@ -238,9 +228,7 @@ class TabContentsWrapper : public content::WebContentsObserver {
   scoped_ptr<BookmarkTabHelper> bookmark_tab_helper_;
   scoped_ptr<ConstrainedWindowTabHelper> constrained_window_tab_helper_;
   scoped_ptr<CoreTabHelper> core_tab_helper_;
-  scoped_ptr<extensions::ScriptExecutor> extension_script_executor_;
   scoped_ptr<ExtensionTabHelper> extension_tab_helper_;
-  scoped_ptr<extensions::ActionBoxController> extension_action_box_controller_;
   scoped_ptr<FaviconTabHelper> favicon_tab_helper_;
   scoped_ptr<FindTabHelper> find_tab_helper_;
   scoped_ptr<HistoryTabHelper> history_tab_helper_;