Only return the visible page actions from PageActionController.

chrome/browser/ui/gtk/location_bar_view_gtk.cc

 // 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/ui/gtk/location_bar_view_gtk.h"
 
 #include <algorithm>
 #include <string>
 #include <vector>
 
 #include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/debug/trace_event.h"
 #include "base/i18n/rtl.h"
 #include "base/logging.h"
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/accessibility/accessibility_events.h"
 #include "chrome/browser/alternate_nav_url_fetcher.h"
 #include "chrome/browser/autocomplete/autocomplete_popup_model.h"
 #include "chrome/browser/chrome_to_mobile_service.h"
 #include "chrome/browser/chrome_to_mobile_service_factory.h"
 #include "chrome/browser/command_updater.h"
 #include "chrome/browser/content_settings/tab_specific_content_settings.h"
 #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_util.h"
 #include "chrome/browser/favicon/favicon_tab_helper.h"
 #include "chrome/browser/instant/instant_controller.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search_engines/template_url.h"
 #include "chrome/browser/search_engines/template_url_service.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
 #include "chrome/browser/ui/browser.h"
@@ skipping 309 matching lines @@
 
   // Now that we've created the widget hierarchy, connect to the main |hbox_|'s
   // size-allocate so we can do proper resizing and eliding on
   // |security_info_label_|.
   g_signal_connect(hbox_.get(), "size-allocate",
                    G_CALLBACK(&OnHboxSizeAllocateThunk), this);
 
   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);
 
   theme_service_->InitThemesFor(this);
 }
 
 void LocationBarViewGtk::BuildSiteTypeArea() {
   location_icon_image_ = gtk_image_new();
   gtk_widget_set_name(location_icon_image_, "chrome-location-icon");
 
@@ skipping 68 matching lines @@
 }
 
 WebContents* LocationBarViewGtk::GetWebContents() const {
   return browser_->GetSelectedWebContents();
 }
 
 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_.count(page_action)) {
+    preview_enabled_actions_.insert(page_action);

[Matt Perry, 2012/05/17 00:17:24]

std::set allows you to test and set simultaneously. You could restructure this
as

if (preview_enabled && insert(page_action).second)
  update();
else if (!preview_enabled && erase(page_action) > 0)
  update();

[not at google - send to devlin, 2012/05/17 01:30:35]

Done.

+    UpdatePageActions();
+  } else if (!preview_enabled && preview_enabled_actions_.count(page_action)) {
+    preview_enabled_actions_.erase(page_action);
+    UpdatePageActions();
   }
 }
 
 GtkWidget* LocationBarViewGtk::GetPageActionWidget(
     ExtensionAction *page_action) {
   DCHECK(page_action);
   for (ScopedVector<PageActionViewGtk>::iterator iter =
            page_action_views_.begin();
        iter != page_action_views_.end();
        ++iter) {
@@ skipping 212 matching lines @@
         toolbar_model_->input_in_progress() ? NULL : web_contents);
     any_visible = (*i)->IsVisible() || any_visible;
   }
 
   // If there are no visible content things, hide the top level box so it
   // doesn't mess with padding.
   gtk_widget_set_visible(content_setting_hbox_.get(), any_visible);
 }
 
 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());
+        *tab_contents->extension_action_box_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()) {
     page_action_views_.reset();  // Delete the old views (if any).
 
     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);
     }
     content::NotificationService::current()->Notify(
         chrome::NOTIFICATION_EXTENSION_PAGE_ACTION_COUNT_CHANGED,
         content::Source<LocationBar>(this),
         content::NotificationService::NoDetails());
   }
 
   WebContents* contents = GetWebContents();
   if (!page_action_views_.empty() && contents) {
     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);
     }
   }
 
   // If there are no visible page actions, hide the hbox too, so that it does
   // not affect the padding in the location bar.
   gtk_widget_set_visible(page_action_hbox_.get(),
                          PageActionVisibleCount() && !ShouldOnlyShowLocation());
 }
 
@@ skipping 42 matching lines @@
 ExtensionAction* LocationBarViewGtk::GetPageAction(size_t index) {
   if (index >= page_action_views_.size()) {
     NOTREACHED();
     return NULL;
   }
 
   return page_action_views_[index]->page_action();
 }
 
 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) {
   if (index >= page_action_views_.size()) {
     NOTREACHED();
     return;
   }
 
   page_action_views_[index]->TestActivatePageAction();
 }
 
 void LocationBarViewGtk::Observe(int type,
                                  const content::NotificationSource& source,
                                  const content::NotificationDetails& details) {
   if (type == chrome::NOTIFICATION_PREF_CHANGED) {
     UpdateStarIcon();
     UpdateChromeToMobileIcon();
     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();

[Evan Stade, 2012/05/16 21:58:17]

too much indent

[not at google - send to devlin, 2012/05/17 01:30:35]

Done.

+    return;
+  }
+
   DCHECK_EQ(type, chrome::NOTIFICATION_BROWSER_THEME_CHANGED);
 
   if (theme_service_->UsingNativeTheme()) {
     gtk_widget_modify_bg(tab_to_search_box_, GTK_STATE_NORMAL, NULL);
 
     GdkColor border_color = theme_service_->GetGdkColor(
         ThemeService::COLOR_FRAME);
     gtk_util::SetRoundedWindowBorderColor(tab_to_search_box_, border_color);
 
     gtk_util::UndoForceFontSize(security_info_label_);
@@ skipping 675 matching lines @@
 
 LocationBarViewGtk::PageActionViewGtk::PageActionViewGtk(
     LocationBarViewGtk* owner,
     ExtensionAction* page_action)
     : owner_(NULL),
       page_action_(page_action),
       last_icon_pixbuf_(NULL),
       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,
                               Extension::kPageActionIconMaxSize);
 
   // Make the event box not visible so it does not paint a background.
   gtk_event_box_set_visible_window(GTK_EVENT_BOX(event_box_.get()), FALSE);
   g_signal_connect(event_box_.get(), "button-press-event",
                    G_CALLBACK(&OnButtonPressedThunk), this);
   g_signal_connect_after(event_box_.get(), "expose-event",
                          G_CALLBACK(OnExposeEventThunk), this);
   g_signal_connect(event_box_.get(), "realize",
                    G_CALLBACK(OnRealizeThunk), this);
 
   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(),
                                               false);
   DCHECK(extension);
 
   // Load all the icons declared in the manifest. This is the contents of the
   // icons array, plus the default_icon property, if any.
   std::vector<std::string> icon_paths(*page_action->icon_paths());
   if (!page_action_->default_icon_path().empty())
@@ skipping 18 matching lines @@
   image_.Destroy();
   event_box_.Destroy();
   for (PixbufMap::iterator iter = pixbufs_.begin(); iter != pixbufs_.end();
        ++iter) {
     g_object_unref(iter->second);
   }
   if (last_icon_pixbuf_)
     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 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().
+  // 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;
-      }
+  // 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);
     }
-    // The pixbuf might not be loaded yet.
-    if (pixbuf)
-      gtk_image_set_from_pixbuf(GTK_IMAGE(image_.get()), pixbuf);
+    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) ?

[Evan Stade, 2012/05/16 21:58:17]

no parens

[not at google - send to devlin, 2012/05/17 01:30:35]

Done.

+        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 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);

[Evan Stade, 2012/05/16 21:58:17]

please add some more vertical space somewhere as this is a huge block of code
with no empty lines

[not at google - send to devlin, 2012/05/17 01:30:35]

Done.

 }
 
 void LocationBarViewGtk::PageActionViewGtk::OnImageLoaded(
     const gfx::Image& image,
     const std::string& extension_id,
     int index) {
   // We loaded icons()->size() icons, plus one extra if the page action had
   // a default icon.
   int total_icons = static_cast<int>(page_action_->icon_paths()->size());
   if (!page_action_->default_icon_path().empty())
@@ skipping 167 matching lines @@
     GdkModifierType modifier,
     void* user_data) {
   PageActionViewGtk* view = static_cast<PageActionViewGtk*>(user_data);
   if (!gtk_widget_get_visible(view->widget()))
     return FALSE;
 
   GdkEventButton event = {};
   event.button = 1;
   return view->OnButtonPressed(view->widget(), &event);
 }