PageActions can now specify multiple icons and switch between them

using optional parameters to enableForTab.

BUG=http://crbug.com/11906
TEST=None

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

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

9 files changed, 132 insertions, 44 deletions

diff --git a/chrome/browser/extensions/extension_page_actions_module.cc b/chrome/browser/extensions/extension_page_actions_module.cc
index 66bd9e3..d3ba09a 100644
--- a/chrome/browser/extensions/extension_page_actions_module.cc
+++ b/chrome/browser/extensions/extension_page_actions_module.cc
@@ -31,6 +31,18 @@ bool PageActionFunction::SetPageActionEnabled(bool enable) {
   std::string url;
   EXTENSION_FUNCTION_VALIDATE(action->GetString(keys::kUrlKey, &url));
 
+  std::string title;
+  int icon_id = 0;
+  if (enable) {
+    // Both of those are optional.
+    if (action->HasKey(keys::kTitleKey))
+      EXTENSION_FUNCTION_VALIDATE(action->GetString(keys::kTitleKey, &title));
+    if (action->HasKey(keys::kIconIdKey)) {
+      EXTENSION_FUNCTION_VALIDATE(action->GetInteger(keys::kIconIdKey,
+                                                     &icon_id));
+    }
+  }
+
   // Find the TabContents that contains this tab id.
   TabContents* contents = NULL;
   ExtensionTabUtil::GetTabById(tab_id, profile(), NULL, NULL, &contents, NULL);
@@ -66,7 +78,7 @@ bool PageActionFunction::SetPageActionEnabled(bool enable) {
   }
 
   // Set visibility and broadcast notifications that the UI should be updated.
-  contents->SetPageActionEnabled(page_action, enable);
+  contents->SetPageActionEnabled(page_action, enable, title, icon_id);
   contents->NotifyNavigationStateChanged(TabContents::INVALIDATE_PAGE_ACTIONS);
 
   return true;
diff --git a/chrome/browser/extensions/extension_page_actions_module_constants.cc b/chrome/browser/extensions/extension_page_actions_module_constants.cc
index e58264b..2b54f1b 100755
--- a/chrome/browser/extensions/extension_page_actions_module_constants.cc
+++ b/chrome/browser/extensions/extension_page_actions_module_constants.cc
@@ -8,6 +8,8 @@ namespace extension_page_actions_module_constants {
 
 const wchar_t kTabIdKey[] = L"tabId";
 const wchar_t kUrlKey[] = L"url";
+const wchar_t kTitleKey[] = L"title";
+const wchar_t kIconIdKey[] = L"iconId";
 
 const char kNoExtensionError[] = "No extension with id: *.";
 const char kNoTabError[] = "No tab with id: *.";
diff --git a/chrome/browser/extensions/extension_page_actions_module_constants.h b/chrome/browser/extensions/extension_page_actions_module_constants.h
index bf28b9f..91db0d8 100755
--- a/chrome/browser/extensions/extension_page_actions_module_constants.h
+++ b/chrome/browser/extensions/extension_page_actions_module_constants.h
@@ -12,6 +12,8 @@ namespace extension_page_actions_module_constants {
 // Keys.
 extern const wchar_t kTabIdKey[];
 extern const wchar_t kUrlKey[];
+extern const wchar_t kTitleKey[];
+extern const wchar_t kIconIdKey[];
 
 // Error messages.
 extern const char kNoExtensionError[];
diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc
index df59e04..b44c630 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -753,12 +753,16 @@ Extension* ExtensionsServiceBackend::LoadExtension(
   for (PageActionMap::const_iterator i(page_actions.begin());
        i != page_actions.end(); ++i) {
     PageAction* page_action = i->second;
-    FilePath path = page_action->icon_path();
-    if (!file_util::PathExists(path)) {
-      ReportExtensionLoadError(extension_path,
-          StringPrintf("Could not load icon '%s' for page action.",
-          WideToUTF8(path.ToWStringHack()).c_str()));
-      return NULL;
+    const std::vector<FilePath>& icon_paths = page_action->icon_paths();
+    for (std::vector<FilePath>::const_iterator iter = icon_paths.begin();
+         iter != icon_paths.end(); ++iter) {
+      FilePath path = *iter;
+      if (!file_util::PathExists(path)) {
+        ReportExtensionLoadError(extension_path,
+            StringPrintf("Could not load icon '%s' for page action.",
+            WideToUTF8(path.ToWStringHack()).c_str()));
+        return NULL;
+      }
     }
   }
 
diff --git a/chrome/browser/extensions/extensions_service_unittest.cc b/chrome/browser/extensions/extensions_service_unittest.cc
index b195151..3b75dcc 100644
--- a/chrome/browser/extensions/extensions_service_unittest.cc
+++ b/chrome/browser/extensions/extensions_service_unittest.cc
@@ -40,7 +40,7 @@ const char* const good0 = "behllobkkfkfnphdnhnkndlbkcpglgmj";
 const char* const good1 = "hpiknbiabeeppbpihjehijgoemciehgk";
 const char* const good2 = "bjafgdebaacbbbecmhlhpofkepfkgcpa";
 const char* const good_crx = "ldnnhddmnhbkjipkidpdiheffobcpfmf";
-const char* const page_action = "kemkhnabegjkabakmlcaafgikalipenj";
+const char* const page_action = "obcimlgaoabeegjmmpldobjndiealpln";
 const char* const theme_crx = "iamefpfkojoapidjnbafmgkgncegbkad";
 const char* const theme2_crx = "pjpgmfcmabopnnfonnhmdjglfpjjfkbf";
 
@@ -654,7 +654,9 @@ TEST_F(ExtensionsServiceTest, CleanupOnStartup) {
   ASSERT_FALSE(file_util::PathExists(vers));
 }
 
-// Test installing extensions.
+// Test installing extensions. This test tries to install few extensions using
+// crx files. If you need to change those crx files, feel free to repackage
+// them, throw away the key used and change the id's above.
 TEST_F(ExtensionsServiceTest, InstallExtension) {
   InitializeEmptyExtensionsService();
 
@@ -690,6 +692,7 @@ TEST_F(ExtensionsServiceTest, InstallExtension) {
   // Bad signature.
   path = extensions_path.AppendASCII("bad_signature.crx");
   InstallExtension(path, false);
+  ValidatePrefKeyCount(pref_count);
 
   // 0-length extension file.
   path = extensions_path.AppendASCII("not_an_extension.crx");
diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc
index c4906ce..0884317 100644
--- a/chrome/browser/tab_contents/tab_contents.cc
+++ b/chrome/browser/tab_contents/tab_contents.cc
@@ -565,23 +565,31 @@ void TabContents::SetIsCrashed(bool state) {
 }
 
 void TabContents::SetPageActionEnabled(const PageAction* page_action,
-                                       bool enable) {
+                                       bool enable,
+                                       const std::string& title,
+                                       int icon_id) {
   DCHECK(page_action);
 
-  if (enable == IsPageActionEnabled(page_action))
-    return;  // Don't need to do anything more.
+  if (!enable &&
+      enabled_page_actions_.end() == enabled_page_actions_.find(page_action)) {
+    return;  // Don't need to disable twice.
+  }
 
-  if (enable)
-    enabled_page_actions_.insert(page_action);
-  else
+  if (enable) {
+    enabled_page_actions_[page_action].reset(
+        new PageActionState(title, icon_id));
+  } else {
     enabled_page_actions_.erase(page_action);
+  }
 }
 
-bool TabContents::IsPageActionEnabled(const PageAction* page_action) {
-  DCHECK(page_action);
-  return enabled_page_actions_.end() != enabled_page_actions_.find(page_action);
-}
+const PageActionState* TabContents::GetPageActionState(
+    const PageAction* page_action) {
+  if (enabled_page_actions_.end() == enabled_page_actions_.find(page_action))
+    return NULL;
 
+  return enabled_page_actions_[page_action].get();
+}
 
 void TabContents::NotifyNavigationStateChanged(unsigned changed_flags) {
   if (delegate_)
diff --git a/chrome/browser/tab_contents/tab_contents.h b/chrome/browser/tab_contents/tab_contents.h
index 5b4fa7b..f81c725 100644
--- a/chrome/browser/tab_contents/tab_contents.h
+++ b/chrome/browser/tab_contents/tab_contents.h
@@ -32,6 +32,7 @@
 #include "chrome/common/gears_api.h"
 #include "chrome/common/navigation_types.h"
 #include "chrome/common/notification_registrar.h"
+#include "chrome/common/page_action.h"
 #include "chrome/common/property_bag.h"
 #include "chrome/common/renderer_preferences.h"
 #include "net/base/load_states.h"
@@ -254,10 +255,17 @@ class TabContents : public PageNavigator,
   void SetIsCrashed(bool state);
 
   // Adds/removes a page action to the list of page actions that are active in
+  // this tab. The parameter |title| (if not empty) can be used to override the
+  // page action title for this tab and |icon_id| specifies an icon index
+  // (defined in the manifest) to use instead of the first icon (for this tab).
+  void SetPageActionEnabled(const PageAction* page_action, bool enable,
+                            const std::string& title, int icon_id);
+
+  // Returns the page action state for this tab. The pair returns contains
+  // the title (string) for the page action and the icon index to use (int).
+  // If this function returns NULL it means the page action is not enabled for
   // this tab.
-  void SetPageActionEnabled(const PageAction* page_action, bool enable);
-  // Checks to see if the PageAction should be visible in this tab.
-  bool IsPageActionEnabled(const PageAction* page_action);
+  const PageActionState* GetPageActionState(const PageAction* page_action);
 
   // Whether the tab is in the process of being destroyed.
   // Added as a tentative work-around for focus related bug #4633.  This allows
@@ -1036,10 +1044,12 @@ class TabContents : public PageNavigator,
 
   // Data for Page Actions -----------------------------------------------------
 
-  // A set of page actions that are enabled in this tab. This list is cleared
-  // every time the mainframe navigates and populated by the PageAction
-  // extension API.
-  std::set<const PageAction*> enabled_page_actions_;
+  // A map of page actions that are enabled in this tab (and a state object
+  // that can be used to override the title and icon used for the page action).
+  // This map is cleared every time the mainframe navigates and populated by the
+  // PageAction extension API.
+  std::map< const PageAction*, linked_ptr<PageActionState> >
+      enabled_page_actions_;
 
   // Data for misc internal state ----------------------------------------------
 
diff --git a/chrome/browser/views/location_bar_view.cc b/chrome/browser/views/location_bar_view.cc
index 7530dee..af8c57f 100644
--- a/chrome/browser/views/location_bar_view.cc
+++ b/chrome/browser/views/location_bar_view.cc
@@ -1140,7 +1140,8 @@ void LocationBarView::SecurityImageView::ShowInfoBubble() {
 class LocationBarView::PageActionImageView::ImageLoadingTracker
   : public base::RefCountedThreadSafe<ImageLoadingTracker> {
  public:
-  explicit ImageLoadingTracker(PageActionImageView* view) : view_(view) {
+  explicit ImageLoadingTracker(PageActionImageView* view, int image_count)
+    : view_(view), image_count_(image_count) {
     AddRef();  // We hold on to a reference to ourself to make sure we don't
                // get deleted until we get a response from image loading (see
                // ImageLoadingDone).
@@ -1151,16 +1152,20 @@ class LocationBarView::PageActionImageView::ImageLoadingTracker
     view_ = NULL;
   }
 
-  void OnImageLoaded(SkBitmap* image) {
-    view_->OnImageLoaded(image);
+  void OnImageLoaded(SkBitmap* image, int index) {
+    view_->OnImageLoaded(image, index);
     delete image;
-    Release();  // We are no longer needed.
+    if (--image_count_ == 0)
+      Release();  // We are no longer needed.
   }
 
  private:
 
   // The view that is waiting for the image to load.
   PageActionImageView* view_;
+
+  // The number of images this ImageTracker should keep track of.
+  int image_count_;
 };
 
 // The LoadImageTask is for asynchronously loading the image on the file thread.
@@ -1168,16 +1173,24 @@ class LocationBarView::PageActionImageView::ImageLoadingTracker
 // |callback_loop| to let the caller know the image is done loading.
 class LocationBarView::PageActionImageView::LoadImageTask : public Task {
  public:
+  // Constructor for the LoadImageTask class. |tracker| is the object that
+  // we use to communicate back to the entity that wants the image after we
+  // decode it. |path| is the path to load the image from. |index| is an
+  // identifier for the image that we pass back to the caller.
   LoadImageTask(ImageLoadingTracker* tracker,
-                const FilePath& path)
+                const FilePath& path,
+                int index)
     : callback_loop_(MessageLoop::current()),
       tracker_(tracker),
-      path_(path) {}
+      path_(path),
+      index_(index) {}
 
   void ReportBack(SkBitmap* image) {
     DCHECK(image);
     callback_loop_->PostTask(FROM_HERE, NewRunnableMethod(tracker_,
-        &PageActionImageView::ImageLoadingTracker::OnImageLoaded, image));
+        &PageActionImageView::ImageLoadingTracker::OnImageLoaded,
+        image,
+        index_));
   }
 
   virtual void Run() {
@@ -1223,6 +1236,9 @@ class LocationBarView::PageActionImageView::LoadImageTask : public Task {
 
   // The path to the image to load asynchronously.
   FilePath path_;
+
+  // The index of the icon being loaded.
+  int index_;
 };
 
 LocationBarView::PageActionImageView::PageActionImageView(
@@ -1233,15 +1249,22 @@ LocationBarView::PageActionImageView::PageActionImageView(
       owner_(owner),
       profile_(profile),
       page_action_(page_action),
-      tracker_(new ImageLoadingTracker(this)) {
+      current_tab_id_(-1),
+      tooltip_(page_action_->name()) {
   // 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 Omnibox.
-  DCHECK(!page_action->icon_path().empty());
-  FilePath path = FilePath(page_action->icon_path());
+  DCHECK(!page_action->icon_paths().empty());
+  const std::vector<FilePath>& icon_paths = page_action->icon_paths();
+  page_action_icons_.resize(icon_paths.size());
+  int index = 0;
   MessageLoop* file_loop = g_browser_process->file_thread()->message_loop();
-  file_loop->PostTask(FROM_HERE,
-      new LoadImageTask(tracker_, page_action->icon_path()));
+  tracker_ = new ImageLoadingTracker(this, icon_paths.size());
+  for (std::vector<FilePath>::const_iterator iter = icon_paths.begin();
+       iter != icon_paths.end(); ++iter) {
+    FilePath path = *iter;
+    file_loop->PostTask(FROM_HERE, new LoadImageTask(tracker_, path, index++));
+  }
 }
 
 LocationBarView::PageActionImageView::~PageActionImageView() {
@@ -1259,7 +1282,7 @@ bool LocationBarView::PageActionImageView::OnMousePressed(
 
 void LocationBarView::PageActionImageView::ShowInfoBubble() {
   SkColor text_color = SK_ColorBLACK;
-  ShowInfoBubbleImpl(ASCIIToWide(page_action_->name()), text_color);
+  ShowInfoBubbleImpl(ASCIIToWide(tooltip_), text_color);
 }
 
 void LocationBarView::PageActionImageView::UpdateVisibility(
@@ -1269,12 +1292,30 @@ void LocationBarView::PageActionImageView::UpdateVisibility(
   current_tab_id_ = ExtensionTabUtil::GetTabId(contents);
   current_url_ = url;
 
-  SetVisible(contents->IsPageActionEnabled(page_action_));
+  const PageActionState* state = contents->GetPageActionState(page_action_);
+  bool visible = state != NULL;
+  if (visible) {
+    // Set the tooltip.
+    if (state->title().empty())
+      tooltip_ = page_action_->name();
+    else
+      tooltip_ = state->title();
+    // Set the image.
+    int index = state->icon_index();
+    // The image index (if not within bounds) will be set to the first image.
+    if (index < 0 || index >= static_cast<int>(page_action_icons_.size()))
+      index = 0;
+    ImageView::SetImage(page_action_icons_[index]);
+  }
+  SetVisible(visible);
 }
 
-void LocationBarView::PageActionImageView::OnImageLoaded(SkBitmap* image) {
-  tracker_ = NULL;  // The tracker object will delete itself when we return.
-  ImageView::SetImage(image);
+void LocationBarView::PageActionImageView::OnImageLoaded(SkBitmap* image,
+                                                         size_t index) {
+  DCHECK(index < page_action_icons_.size());
+  if (index == page_action_icons_.size() - 1)
+    tracker_ = NULL;  // The tracker object will delete itself when we return.
+  page_action_icons_[index] = *image;
   owner_->UpdatePageActions();
 }
 
diff --git a/chrome/browser/views/location_bar_view.h b/chrome/browser/views/location_bar_view.h
index b90693f..97e7eb6 100644
--- a/chrome/browser/views/location_bar_view.h
+++ b/chrome/browser/views/location_bar_view.h
@@ -332,7 +332,7 @@ class LocationBarView : public LocationBar,
     void UpdateVisibility(TabContents* contents, GURL url);
 
     // A callback for when the image has loaded.
-    void OnImageLoaded(SkBitmap* image);
+    void OnImageLoaded(SkBitmap* image, size_t index);
 
   private:
     // We load the images for the PageActions on the file thread. These tasks
@@ -350,6 +350,9 @@ class LocationBarView : public LocationBar,
     // us, it resides in the extension of this particular profile.
     const PageAction* page_action_;
 
+    // The icons representing different states for the page action.
+    std::vector<SkBitmap> page_action_icons_;
+
     // The object that is waiting for the image loading to complete
     // asynchronously.
     ImageLoadingTracker* tracker_;
@@ -360,6 +363,9 @@ class LocationBarView : public LocationBar,
     // The URL we are currently showing the icon for.
     GURL current_url_;
 
+    // The string to show for a tooltip;
+    std::string tooltip_;
+
     DISALLOW_COPY_AND_ASSIGN(PageActionImageView);
   };