Revert 137630 - Broke ASAN

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

TBR=kalman@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10392142

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

15 files changed, 162 insertions, 231 deletions

diff --git a/chrome/browser/extensions/action_box_controller.cc b/chrome/browser/extensions/action_box_controller.cc
deleted file mode 100644
index 13a8f17..0000000
--- a/chrome/browser/extensions/action_box_controller.cc
+++ /dev/null
@@ -1,23 +0,0 @@
-// 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 e85e37e3..e3415c7 100644
--- a/chrome/browser/extensions/action_box_controller.h
+++ b/chrome/browser/extensions/action_box_controller.h
@@ -6,7 +6,6 @@
 #define CHROME_BROWSER_EXTENSIONS_ACTION_BOX_CONTROLLER_H_
 #pragma once
 
-#include <set>
 #include <string>
 #include <vector>
 
@@ -19,6 +18,22 @@ 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,
@@ -28,13 +43,8 @@ class ActionBoxController {
 
   virtual ~ActionBoxController() {}
 
-  // 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;
+  // Gets the badge data for all extensions.
+  virtual scoped_ptr<DataList> GetAllBadgeData() = 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 d50afc6..b2c4e7a 100644
--- a/chrome/browser/extensions/execute_code_in_tab_function.cc
+++ b/chrome/browser/extensions/execute_code_in_tab_function.cc
@@ -9,7 +9,6 @@
 #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"
@@ -245,7 +244,7 @@ bool ExecuteCodeInTabFunction::Execute(const std::string& code_string) {
     NOTREACHED();
   }
 
-  contents->extension_tab_helper()->script_executor()->ExecuteScript(
+  contents->extension_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 627417d..ce76ddc 100644
--- a/chrome/browser/extensions/extension_tab_helper.cc
+++ b/chrome/browser/extensions/extension_tab_helper.cc
@@ -5,8 +5,6 @@
 #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"
@@ -29,8 +27,6 @@
 #include "ui/gfx/image/image.h"
 
 using content::WebContents;
-using extensions::ScriptExecutorImpl;
-using extensions::PageActionController;
 
 namespace {
 
@@ -45,8 +41,6 @@ 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() {
@@ -58,26 +52,14 @@ 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 c095082..16b4f5f 100644
--- a/chrome/browser/extensions/extension_tab_helper.h
+++ b/chrome/browser/extensions/extension_tab_helper.h
@@ -7,7 +7,6 @@
 #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"
@@ -25,11 +24,6 @@ namespace content {
 struct LoadCommittedDetails;
 }
 
-namespace extensions {
-class ActionBoxController;
-class ScriptExecutor;
-}
-
 // Per-tab extension helper. Also handles non-extension apps.
 class ExtensionTabHelper
     : public content::WebContentsObserver,
@@ -39,15 +33,6 @@ 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();
 
@@ -65,10 +50,6 @@ 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
@@ -109,14 +90,6 @@ 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);
@@ -199,12 +172,6 @@ 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 dc1cc14..7f15efc 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_tab_helper()->script_executor()->ExecuteScript(
+    tab_contents_->extension_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 ed3c8e6..c21b768 100644
--- a/chrome/browser/extensions/page_action_controller.cc
+++ b/chrome/browser/extensions/page_action_controller.cc
@@ -8,39 +8,33 @@
 #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,
-                                           ExtensionTabHelper* tab_helper)
-    : tab_contents_(tab_contents),
-      tab_helper_(tab_helper) {
-  tab_helper->AddObserver(this);
-}
+PageActionController::PageActionController(TabContentsWrapper* tab_contents)
+    : tab_contents_(tab_contents) {}
 
-PageActionController::~PageActionController() {
-  tab_helper_->RemoveObserver(this);
-}
+PageActionController::~PageActionController() {}
 
-scoped_ptr<std::vector<ExtensionAction*> >
-PageActionController::GetCurrentActions() {
-  int tab_id = ExtensionTabUtil::GetTabId(tab_contents_->web_contents());
+scoped_ptr<ActionBoxController::DataList>
+PageActionController::GetAllBadgeData() {
   const ExtensionSet* extensions = GetExtensionService()->extensions();
-  scoped_ptr<std::vector<ExtensionAction*> > current_actions(
-      new std::vector<ExtensionAction*>());
+  scoped_ptr<DataList> all_badge_data(new DataList());
   for (ExtensionSet::const_iterator i = extensions->begin();
        i != extensions->end(); ++i) {
     ExtensionAction* action = (*i)->page_action();
-    if (action && action->GetIsVisible(tab_id))
-      current_actions->push_back(action);
+    if (action) {
+      Data data = {
+        DECORATION_NONE,
+        action,
+      };
+      all_badge_data->push_back(data);
+    }
   }
-  return current_actions.Pass();
+  return all_badge_data.Pass();
 }
 
 ActionBoxController::Action PageActionController::OnClicked(
@@ -75,25 +69,6 @@ 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 cdb43a3..22f9fc6 100644
--- a/chrome/browser/extensions/page_action_controller.h
+++ b/chrome/browser/extensions/page_action_controller.h
@@ -6,12 +6,7 @@
 #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;
@@ -19,30 +14,22 @@ class TabContentsWrapper;
 namespace extensions {
 
 // An ActionBoxController which corresponds to the page actions of an extension.
-class PageActionController : public ActionBoxController,
-                             public ExtensionTabHelper::Observer {
+class PageActionController : public ActionBoxController {
  public:
-  PageActionController(TabContentsWrapper* tab_contents,
-                       ExtensionTabHelper* tab_helper);
+  explicit PageActionController(TabContentsWrapper* tab_contents);
   virtual ~PageActionController();
 
-  // ActionBoxController implementation.
-  virtual scoped_ptr<std::vector<ExtensionAction*> > GetCurrentActions()
-      OVERRIDE;
+  virtual scoped_ptr<DataList> GetAllBadgeData() 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 f3227fe..3672740 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,10 +356,6 @@ 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);
 
@@ -448,10 +444,16 @@ void LocationBarViewGtk::SetPreviewEnabledPageAction(
     ExtensionAction *page_action,
     bool preview_enabled) {
   DCHECK(page_action);
-  if (preview_enabled && preview_enabled_actions_.insert(page_action).second)
-    UpdatePageActions();
-  if (!preview_enabled && preview_enabled_actions_.erase(page_action) > 0)
-    UpdatePageActions();
+  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;
+    }
+  }
 }
 
 GtkWidget* LocationBarViewGtk::GetPageActionWidget(
@@ -683,20 +685,14 @@ void LocationBarViewGtk::UpdateContentSettingsIcons() {
 }
 
 void LocationBarViewGtk::UpdatePageActions() {
-  std::vector<ExtensionAction*> page_actions;
+  ActionBoxController::DataList page_actions;
 
   TabContentsWrapper* tab_contents = GetTabContentsWrapper();
   if (tab_contents) {
-    ActionBoxController* controller =
-        tab_contents->extension_tab_helper()->action_box_controller();
-    page_actions.swap(*controller->GetCurrentActions());
+    page_actions.swap(
+        *tab_contents->extension_action_box_controller()->GetAllBadgeData());
   }
 
-  // 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()) {
@@ -704,7 +700,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]));
+          new PageActionViewGtk(this, page_actions[i].action));
       gtk_box_pack_end(GTK_BOX(page_action_hbox_.get()),
                        page_action_views_[i]->widget(), FALSE, FALSE, 0);
     }
@@ -719,7 +715,7 @@ void LocationBarViewGtk::UpdatePageActions() {
     GURL url = browser()->GetSelectedWebContents()->GetURL();
 
     for (size_t i = 0; i < page_action_views_.size(); i++) {
-      page_action_views_[i]->Update(
+      page_action_views_[i]->UpdateVisibility(
           toolbar_model_->input_in_progress() ? NULL : contents, url);
     }
   }
@@ -782,7 +778,16 @@ ExtensionAction* LocationBarViewGtk::GetPageAction(size_t index) {
 }
 
 ExtensionAction* LocationBarViewGtk::GetVisiblePageAction(size_t index) {
-  return page_action_views_[index]->page_action();
+  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;
 }
 
 void LocationBarViewGtk::TestPageActionPressed(size_t index) {
@@ -803,15 +808,6 @@ 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()) {
@@ -1507,7 +1503,8 @@ LocationBarViewGtk::PageActionViewGtk::PageActionViewGtk(
       tracker_(this),
       current_tab_id_(-1),
       window_(NULL),
-      accel_group_(NULL) {
+      accel_group_(NULL),
+      preview_enabled_(false) {
   event_box_.Own(gtk_event_box_new());
   gtk_widget_set_size_request(event_box_.get(),
                               Extension::kPageActionIconMaxSize,
@@ -1524,7 +1521,6 @@ 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(),
@@ -1563,56 +1559,75 @@ LocationBarViewGtk::PageActionViewGtk::~PageActionViewGtk() {
     g_object_unref(last_icon_pixbuf_);
 }
 
-void LocationBarViewGtk::PageActionViewGtk::Update(
+bool LocationBarViewGtk::PageActionViewGtk::IsVisible() {
+  return gtk_widget_get_visible(widget());
+}
+
+void LocationBarViewGtk::PageActionViewGtk::UpdateVisibility(
     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;
 
-  // 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;
+  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;
+      }
     }
+    // The pixbuf might not be loaded yet.
+    if (pixbuf)
+      gtk_image_set_from_pixbuf(GTK_IMAGE(image_.get()), pixbuf);
   }
 
-  // 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));
+  }
 }
 
 void LocationBarViewGtk::PageActionViewGtk::OnImageLoaded(
@@ -1733,7 +1748,7 @@ gboolean LocationBarViewGtk::PageActionViewGtk::OnButtonPressed(
     return TRUE;
 
   ActionBoxController* controller =
-      tab_contents->extension_tab_helper()->action_box_controller();
+      tab_contents->extension_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 3f3d4d8..4a0fbce 100644
--- a/chrome/browser/ui/gtk/location_bar_view_gtk.h
+++ b/chrome/browser/ui/gtk/location_bar_view_gtk.h
@@ -9,7 +9,6 @@
 #include <gtk/gtk.h>
 
 #include <map>
-#include <set>
 #include <string>
 
 #include "base/basictypes.h"
@@ -19,7 +18,6 @@
 #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"
@@ -80,9 +78,10 @@ class LocationBarViewGtk : public AutocompleteEditController,
   // Returns the current WebContents.
   content::WebContents* GetWebContents() const;
 
-  // If |preview_enabled| is true, the view will display the
-  // This is used by the ExtensionInstalledBubbleGtk to preview what the icon
+  // 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.
+  // This is used by the ExtensionInstalledBubbleGtk to preview what the icon
   // will look like for the user upon installation of the extension.
   void SetPreviewEnabledPageAction(ExtensionAction *page_action,
                                    bool preview_enabled);
@@ -229,10 +228,16 @@ class LocationBarViewGtk : public AutocompleteEditController,
 
     ExtensionAction* page_action() { return page_action_; }
 
-    // Called to notify the PageAction that it should update based on the state
-    // of |page_action_|. |contents| is the WebContents that is active, |url|
+    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|
     // is the current page URL.
-    void Update(content::WebContents* contents, const GURL& url);
+    void UpdateVisibility(content::WebContents* contents, const GURL& url);
 
     // A callback from ImageLoadingTracker for when the image has loaded.
     virtual void OnImageLoaded(const gfx::Image& image,
@@ -311,6 +316,10 @@ 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_;
@@ -484,9 +493,6 @@ 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 2010a43..365ca3a 100644
--- a/chrome/browser/ui/tab_contents/tab_contents_wrapper.cc
+++ b/chrome/browser/ui/tab_contents/tab_contents_wrapper.cc
@@ -14,6 +14,8 @@
 #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"
@@ -93,6 +95,10 @@ 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 3fc5e14..c4fe4cc4 100644
--- a/chrome/browser/ui/tab_contents/tab_contents_wrapper.h
+++ b/chrome/browser/ui/tab_contents/tab_contents_wrapper.h
@@ -56,6 +56,8 @@ class SyncedTabDelegate;
 }
 
 namespace extensions {
+class ActionBoxController;
+class ScriptExecutor;
 class WebNavigationTabObserver;
 }
 
@@ -132,6 +134,14 @@ 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();
   }
@@ -228,7 +238,9 @@ 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_;
diff --git a/chrome/chrome_browser_extensions.gypi b/chrome/chrome_browser_extensions.gypi
index e4bb59f..a9c8b52 100644
--- a/chrome/chrome_browser_extensions.gypi
+++ b/chrome/chrome_browser_extensions.gypi
@@ -53,7 +53,6 @@
       'sources': [
         # All .cc, .h, .m, and .mm files under browser/extensions except for
         # tests and mocks.
-        'browser/extensions/action_box_controller.cc',
         'browser/extensions/action_box_controller.h',
         'browser/extensions/api/api_function.cc',
         'browser/extensions/api/api_function.h',
diff --git a/chrome/common/chrome_notification_types.h b/chrome/common/chrome_notification_types.h
index 2f58db9..1314c8e 100644
--- a/chrome/common/chrome_notification_types.h
+++ b/chrome/common/chrome_notification_types.h
@@ -430,10 +430,6 @@ enum NotificationType {
   // unloaded and reloaded. The source is a Profile.
   NOTIFICATION_EXTENSIONS_READY,
 
-  // Sent when the extension action box is updated for a tab. The source is the
-  // Profile and the details are the TabContentsWrapper for the tab.
-  NOTIFICATION_EXTENSION_ACTION_BOX_UPDATED,
-
   // Sent when a new extension is loaded. The details are an Extension, and
   // the source is a Profile.
   NOTIFICATION_EXTENSION_LOADED,
diff --git a/chrome/common/extensions/extension_action.h b/chrome/common/extensions/extension_action.h
index a081459..6f40f9a 100644
--- a/chrome/common/extensions/extension_action.h
+++ b/chrome/common/extensions/extension_action.h
@@ -41,7 +41,7 @@ class ExtensionAction {
   ~ExtensionAction();
 
   // extension id
-  const std::string& extension_id() const { return extension_id_; }
+  std::string extension_id() const { return extension_id_; }
 
   // action id -- only used with legacy page actions API
   std::string id() const { return id_; }