PageActions now work across tabs and windows.

The extension system now provides TabId to extensions so it is now possible to activate PageActions in other tabs besides the first in the tab strip. :)

BUG=None
TEST=None (requires a PageAction extension to test against).

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

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

12 files changed, 111 insertions, 97 deletions

diff --git a/chrome/browser/extensions/extension.cc b/chrome/browser/extensions/extension.cc
index 280b32f..e35dfff 100644
--- a/chrome/browser/extensions/extension.cc
+++ b/chrome/browser/extensions/extension.cc
@@ -157,12 +157,12 @@ FilePath Extension::GetThemeResourcePath(const int resource_id) {
   return FilePath();
 }
 
-bool Extension::UpdatePageAction(std::string id, int tab_id, GURL url) {
-  if (page_actions_.find(id) == page_actions_.end())
-    return false;
+const PageAction* Extension::GetPageAction(std::string id) const {
+  PageActionMap::const_iterator it = page_actions_.find(id);
+  if (it == page_actions_.end())
+    return NULL;
 
-  page_actions_[id]->SetActiveTabIdAndUrl(tab_id, url);
-  return true;
+  return it->second;
 }
 
 // static
@@ -360,9 +360,10 @@ bool Extension::LoadUserScriptHelper(const DictionaryValue* content_script,
 
 // Helper method that loads a PageAction object from a dictionary in the
 // page_action list of the manifest.
-bool Extension::LoadPageActionHelper(const DictionaryValue* page_action,
-                                     int definition_index, std::string* error,
-                                     PageAction* result) {
+PageAction* Extension::LoadPageActionHelper(
+    const DictionaryValue* page_action, int definition_index,
+    std::string* error) {
+  scoped_ptr<PageAction> result(new PageAction());
   result->set_extension_id(id());
 
   // Read the page action |icon|.
@@ -370,13 +371,13 @@ bool Extension::LoadPageActionHelper(const DictionaryValue* page_action,
   if (!page_action->GetString(kIconPathKey, &icon)) {
     *error = FormatErrorMessage(kInvalidPageActionIconPathError,
                                 IntToString(definition_index));
-    return false;
+    return NULL;
   }
   FilePath icon_path = path_.AppendASCII(icon);
   if (!file_util::PathExists(icon_path)) {
     *error = FormatErrorMessage(kMissingPageActionIcon,
                                 IntToString(definition_index));
-    return false;
+    return NULL;
   }
   result->set_icon_path(icon_path);
 
@@ -384,7 +385,7 @@ bool Extension::LoadPageActionHelper(const DictionaryValue* page_action,
   std::string id;
   if (!page_action->GetString(kIdKey, &id)) {
     *error = FormatErrorMessage(kInvalidIdError, IntToString(definition_index));
-    return false;
+    return NULL;
   }
   result->set_id(id);
 
@@ -393,7 +394,7 @@ bool Extension::LoadPageActionHelper(const DictionaryValue* page_action,
   if (!page_action->GetString(kNameKey, &name)) {
     *error = FormatErrorMessage(kInvalidNameError,
                                 IntToString(definition_index));
-    return false;
+    return NULL;
   }
   result->set_name(name);
 
@@ -402,7 +403,7 @@ bool Extension::LoadPageActionHelper(const DictionaryValue* page_action,
   if (!page_action->GetString(kTooltipKey, &tooltip)) {
     *error = FormatErrorMessage(kInvalidPageActionTooltipError,
                                 IntToString(definition_index));
-    return false;
+    return NULL;
   }
   result->set_tooltip(tooltip);
 
@@ -415,7 +416,7 @@ bool Extension::LoadPageActionHelper(const DictionaryValue* page_action,
              !LowerCaseEqualsASCII(type, kPageActionTypePermanent)) {
     *error = FormatErrorMessage(kInvalidPageActionTypeValueError,
                                 IntToString(definition_index));
-    return false;
+    return NULL;
   } else {
     if (LowerCaseEqualsASCII(type, kPageActionTypeTab))
       result->set_type(PageAction::TAB);
@@ -423,7 +424,7 @@ bool Extension::LoadPageActionHelper(const DictionaryValue* page_action,
       result->set_type(PageAction::PERMANENT);
   }
 
-  return true;
+  return result.release();
 }
 
 bool Extension::InitFromValue(const DictionaryValue& source, bool require_id,
@@ -593,11 +594,10 @@ bool Extension::InitFromValue(const DictionaryValue& source, bool require_id,
         return false;
       }
 
-      PageAction* page_action = new PageAction();
-      if (!LoadPageActionHelper(page_action_value, i, error, page_action)) {
-        delete page_action;
+      PageAction* page_action =
+          LoadPageActionHelper(page_action_value, i, error);
+      if (!page_action)
         return false;  // Failed to parse page action definition.
-      }
       page_actions_[page_action->id()] = page_action;
     }
   }
diff --git a/chrome/browser/extensions/extension.h b/chrome/browser/extensions/extension.h
index bf62694..c612203 100644
--- a/chrome/browser/extensions/extension.h
+++ b/chrome/browser/extensions/extension.h
@@ -128,9 +128,8 @@ class Extension {
   // as providing a theme.
   FilePath GetThemeResourcePath(const int resource_id);
 
-  // Update the status of the page action with |id| in tab |tab_id| and set the
-  // current page url to |url|.
-  bool UpdatePageAction(std::string id, int tab_id, GURL url);
+  // Retrieves a page action by |id|.
+  const PageAction* GetPageAction(std::string id) const;
 
   const FilePath& path() const { return path_; }
   const GURL& url() const { return extension_url_; }
@@ -158,10 +157,9 @@ class Extension {
 
   // Helper method that loads a PageAction object from a
   // dictionary in the page_action list of the manifest.
-  bool LoadPageActionHelper(const DictionaryValue* page_action,
-                            int definition_index,
-                            std::string* error,
-                            PageAction* result);
+  PageAction* LoadPageActionHelper(const DictionaryValue* page_action,
+                                   int definition_index,
+                                   std::string* error);
 
   // The absolute path to the directory the extension is stored in.
   FilePath path_;
diff --git a/chrome/browser/extensions/extension_page_actions_module.cc b/chrome/browser/extensions/extension_page_actions_module.cc
index edf08dd..bf23f65 100644
--- a/chrome/browser/extensions/extension_page_actions_module.cc
+++ b/chrome/browser/extensions/extension_page_actions_module.cc
@@ -10,6 +10,7 @@
 #include "chrome/browser/extensions/extension.h"
 #include "chrome/browser/extensions/extension_tabs_module.h"
 #include "chrome/browser/extensions/extensions_service.h"
+#include "chrome/browser/tab_contents/navigation_entry.h"
 
 bool EnablePageActionFunction::RunImpl() {
   EXTENSION_FUNCTION_VALIDATE(args_->IsType(Value::TYPE_LIST));
@@ -25,39 +26,33 @@ bool EnablePageActionFunction::RunImpl() {
   std::string url;
   EXTENSION_FUNCTION_VALIDATE(action->GetString(L"url", &url));
 
-  Browser* browser = BrowserList::GetLastActive();
-  if (!browser)
+  // Find the TabContents that contains this tab id.
+  TabContents* contents = NULL;
+  ExtensionTabUtil::GetTabById(tab_id, profile(), NULL, NULL, &contents, NULL);
+  if (!contents)
     return false;
 
-  // HACK: We need to figure out the tab index from the tab_id (pending).
-  // For now we only support page actions in the first tab in the strip (tab 0).
-  int tab_index = 0;
-
-  TabStripModel* tab_strip = browser->tabstrip_model();
-  TabContents* contents = tab_strip->GetTabContentsAt(tab_index);
-
-  // Not needed when we stop hard-coding the tab index.
-  tab_id = ExtensionTabUtil::GetTabId(contents);
+  // Make sure the URL hasn't changed.
+  // TODO(finnur): Add an error message here when there is a way to.
+  if (url != contents->controller().GetActiveEntry()->url().spec())
+    return false;
 
   // Find our extension.
   Extension* extension = NULL;
-  if (profile()->GetExtensionsService()) {
-    const ExtensionList* extensions =
-        profile()->GetExtensionsService()->extensions();
-    for (ExtensionList::const_iterator iter = extensions->begin();
-        iter != extensions->end(); ++iter) {
-      if ((*iter)->id() == extension_id()) {
-        extension = (*iter);
-        break;  // Found our extension.
-      }
-    }
-  }
+  ExtensionsService* service = profile()->GetExtensionsService();
+  if (service)
+    extension = service->GetExtensionByID(extension_id());
+  else
+    NOTREACHED();
+  if (!extension)
+    return false;
 
-  if (!extension ||
-      !extension->UpdatePageAction(page_action_id, tab_id, GURL(url)))
+  const PageAction* page_action = extension->GetPageAction(page_action_id);
+  if (!page_action)
     return false;
 
-  // Broadcast notifications when the UI should be updated.
+  // Set visible and broadcast notifications that the UI should be updated.
+  contents->EnablePageAction(page_action);
   contents->NotifyNavigationStateChanged(TabContents::INVALIDATE_PAGE_ACTIONS);
 
   return true;
diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc
index 338a2ce..0026d18 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 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.
 
@@ -209,6 +209,15 @@ void ExtensionsService::OnExtensionInstalled(Extension* extension,
                            PageTransition::LINK, true, -1, false, NULL);
 }
 
+Extension* ExtensionsService::GetExtensionByID(std::string id) {
+  for (ExtensionList::const_iterator iter = extensions_.begin();
+       iter != extensions_.end(); ++iter) {
+    if ((*iter)->id() == id)
+      return *iter;
+  }
+  return NULL;
+}
+
 ExtensionView* ExtensionsService::CreateView(Extension* extension,
                                              const GURL& url,
                                              Browser* browser) {
@@ -576,7 +585,6 @@ bool ExtensionsServiceBackend::CheckCurrentVersion(
 
 bool ExtensionsServiceBackend::InstallDirSafely(const FilePath& source_dir,
                                                 const FilePath& dest_dir) {
-
   if (file_util::PathExists(dest_dir)) {
     // By the time we get here, it should be safe to assume that this directory
     // is not currently in use (it's not the current active version).
@@ -631,7 +639,6 @@ bool ExtensionsServiceBackend::SetCurrentVersion(const FilePath& dest_dir,
   if (stream.Open(current_version, flags) != 0)
     return false;
   if (stream.Write(version.c_str(), version.size(), NULL) < 0) {
-
     // Restore the old CurrentVersion.
     if (file_util::PathExists(current_version_old)) {
       if (!file_util::Move(current_version_old, current_version)) {
diff --git a/chrome/browser/extensions/extensions_service.h b/chrome/browser/extensions/extensions_service.h
index 4bf2323..67c8c4e 100644
--- a/chrome/browser/extensions/extensions_service.h
+++ b/chrome/browser/extensions/extensions_service.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 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.
 
@@ -32,7 +32,7 @@ typedef std::vector<Extension*> ExtensionList;
 class ExtensionsServiceFrontendInterface
     : public base::RefCountedThreadSafe<ExtensionsServiceFrontendInterface> {
  public:
-  virtual ~ExtensionsServiceFrontendInterface(){}
+  virtual ~ExtensionsServiceFrontendInterface() {}
 
   // The message loop to invoke the frontend's methods on.
   virtual MessageLoop* GetMessageLoop() = 0;
@@ -54,6 +54,9 @@ class ExtensionsServiceFrontendInterface
   // |is_update| is true if the installation was an update to an existing
   // installed extension rather than a new installation.
   virtual void OnExtensionInstalled(Extension* extension, bool is_update) = 0;
+
+  // Lookup an extension by |id|.
+  virtual Extension* GetExtensionByID(std::string id) = 0;
 };
 
 
@@ -77,6 +80,7 @@ class ExtensionsService : public ExtensionsServiceFrontendInterface {
   virtual void LoadExtension(const FilePath& extension_path);
   virtual void OnExtensionsLoaded(ExtensionList* extensions);
   virtual void OnExtensionInstalled(Extension* extension, bool is_update);
+  virtual Extension* GetExtensionByID(std::string id);
 
   // Creates a new ExtensionView, grouping it in the appropriate SiteInstance
   // (and therefore process) based on the URL and profile.
@@ -132,7 +136,7 @@ class ExtensionsServiceBackend
     : public base::RefCountedThreadSafe<ExtensionsServiceBackend> {
  public:
   explicit ExtensionsServiceBackend(const FilePath& install_directory)
-      : install_directory_(install_directory) {};
+      : install_directory_(install_directory) {}
 
   // Loads extensions from the install directory. The extensions are assumed to
   // be unpacked in directories that are direct children of the specified path.
diff --git a/chrome/browser/extensions/extensions_service_unittest.cc b/chrome/browser/extensions/extensions_service_unittest.cc
index d66ea0b..6e1e614 100644
--- a/chrome/browser/extensions/extensions_service_unittest.cc
+++ b/chrome/browser/extensions/extensions_service_unittest.cc
@@ -96,6 +96,10 @@ class ExtensionsServiceTestFrontend
     installed_ = extension;
   }
 
+  virtual Extension* GetExtensionByID(std::string id) {
+    return NULL;
+  }
+
   void TestInstallExtension(const FilePath& path,
                             ExtensionsServiceBackend* backend,
                             bool should_succeed) {
diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc
index 83c49c6..995447d 100644
--- a/chrome/browser/tab_contents/tab_contents.cc
+++ b/chrome/browser/tab_contents/tab_contents.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 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.
 
@@ -32,18 +32,17 @@
 #include "chrome/browser/renderer_host/render_widget_host_view.h"
 #include "chrome/browser/renderer_host/web_cache_manager.h"
 #include "chrome/browser/tab_contents/navigation_entry.h"
-#include "chrome/browser/tab_contents/tab_contents.h"
 #include "chrome/browser/tab_contents/tab_contents_delegate.h"
 #include "chrome/browser/tab_contents/tab_contents_view.h"
 #include "chrome/browser/search_engines/template_url_fetcher.h"
 #include "chrome/browser/search_engines/template_url_model.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/notification_service.h"
+#include "chrome/common/page_action.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/pref_service.h"
 #include "chrome/common/render_messages.h"
 #include "chrome/common/url_constants.h"
-//#include "grit/generated_resources.h"
 #include "grit/locale_settings.h"
 #include "net/base/mime_util.h"
 #include "net/base/net_errors.h"
@@ -253,7 +252,6 @@ TabContents::TabContents(Profile* profile,
 #endif
       last_javascript_message_dismissal_(),
       suppress_javascript_messages_(false) {
-
   pending_install_.page_id = 0;
   pending_install_.callback_functor = NULL;
 
@@ -583,6 +581,21 @@ void TabContents::SetIsCrashed(bool state) {
   NotifyNavigationStateChanged(INVALIDATE_TAB);
 }
 
+void TabContents::EnablePageAction(const PageAction* page_action) {
+  DCHECK(page_action);
+
+  if (IsPageActionEnabled(page_action))
+    return;  // Already enabled.
+
+  enabled_page_actions_.insert(page_action);
+}
+
+bool TabContents::IsPageActionEnabled(const PageAction* page_action) {
+  DCHECK(page_action);
+  return enabled_page_actions_.end() != enabled_page_actions_.find(page_action);
+}
+
+
 void TabContents::NotifyNavigationStateChanged(unsigned changed_flags) {
   if (delegate_)
     delegate_->NavigationStateChanged(this, changed_flags);
@@ -1153,7 +1166,7 @@ void TabContents::SetIsLoading(bool is_loading,
 
   NotificationType type = is_loading ? NotificationType::LOAD_START :
       NotificationType::LOAD_STOP;
-  NotificationDetails det = NotificationService::NoDetails();;
+  NotificationDetails det = NotificationService::NoDetails();
   if (details)
       det = Details<LoadNotificationDetails>(details);
   NotificationService::current()->Notify(type,
@@ -1314,6 +1327,9 @@ void TabContents::DidNavigateMainFramePostCommit(
   // Get the favicon, either from history or request it from the net.
   fav_icon_helper_.FetchFavIcon(details.entry->url());
 
+  // Disable all page actions.
+  enabled_page_actions_.clear();
+
   // Close constrained popups if necessary.
   MaybeCloseChildWindows(details.previous_url, details.entry->url());
 
diff --git a/chrome/browser/tab_contents/tab_contents.h b/chrome/browser/tab_contents/tab_contents.h
index 1a83d8c..59549c4 100644
--- a/chrome/browser/tab_contents/tab_contents.h
+++ b/chrome/browser/tab_contents/tab_contents.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 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.
 
@@ -8,6 +8,7 @@
 #include "build/build_config.h"
 
 #include <map>
+#include <set>
 #include <string>
 #include <vector>
 
@@ -67,6 +68,7 @@ class DOMUIContents;
 class DownloadItem;
 class DownloadShelf;
 class LoadNotificationDetails;
+class PageAction;
 class PasswordManager;
 class PluginInstaller;
 class Profile;
@@ -245,6 +247,11 @@ class TabContents : public PageNavigator,
   bool is_crashed() const { return is_crashed_; }
   void SetIsCrashed(bool state);
 
+  // Adds a page action to the list of page actions that are active in this tab.
+  void EnablePageAction(const PageAction* page_action);
+  // Checks to see if the PageAction should be visible in this tab.
+  bool IsPageActionEnabled(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
   // us not to store focus when a tab is being closed.
@@ -1025,6 +1032,13 @@ class TabContents : public PageNavigator,
   // information to build its presentation.
   FindNotificationDetails find_result_;
 
+  // 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_;
+
   // Data for misc internal state ----------------------------------------------
 
   // See capturing_contents() above.
diff --git a/chrome/browser/views/location_bar_view.cc b/chrome/browser/views/location_bar_view.cc
index 0686de8..d7267ea 100644
--- a/chrome/browser/views/location_bar_view.cc
+++ b/chrome/browser/views/location_bar_view.cc
@@ -583,11 +583,10 @@ void LocationBarView::RefreshPageActionViews() {
 
   TabContents* contents = delegate_->GetTabContents();
   if (!page_action_image_views_.empty() && contents) {
-    int tab_id = ExtensionTabUtil::GetTabId(contents);
     GURL url = GURL(model_->GetText());
 
     for (size_t i = 0; i < page_action_image_views_.size(); i++)
-      page_action_image_views_[i]->UpdateVisibility(tab_id, url);
+      page_action_image_views_[i]->UpdateVisibility(contents, url);
   }
 }
 
@@ -1143,11 +1142,13 @@ void LocationBarView::PageActionImageView::ShowInfoBubble() {
 }
 
 void LocationBarView::PageActionImageView::UpdateVisibility(
-    int tab_id, GURL url) {
-  current_tab_id_ = tab_id;
+    TabContents* contents, 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_ = ExtensionTabUtil::GetTabId(contents);
   current_url_ = url;
 
-  SetVisible(page_action_->IsActive(current_tab_id_, current_url_));
+  SetVisible(contents->IsPageActionEnabled(page_action_));
 }
 
 void LocationBarView::PageActionImageView::OnIconLoaded(
diff --git a/chrome/browser/views/location_bar_view.h b/chrome/browser/views/location_bar_view.h
index 5dac2ef..cfbddbd 100644
--- a/chrome/browser/views/location_bar_view.h
+++ b/chrome/browser/views/location_bar_view.h
@@ -326,8 +326,9 @@ class LocationBarView : public LocationBar,
     virtual void ShowInfoBubble();
 
     // Called to notify the PageAction that it should determine whether to be
-    // visible or hidden.
-    void UpdateVisibility(int tab_id, GURL url);
+    // visible or hidden. |contents| is the TabContents that is active, |url|
+    // is the current page URL.
+    void UpdateVisibility(TabContents* contents, GURL url);
 
     // Called when the IconManager has loaded our icon.
     void OnIconLoaded(IconManager::Handle handle, SkBitmap* icon);
diff --git a/chrome/common/page_action.cc b/chrome/common/page_action.cc
index 1c8096f..233a735 100644
--- a/chrome/common/page_action.cc
+++ b/chrome/common/page_action.cc
@@ -5,23 +5,8 @@
 #include "chrome/common/page_action.h"
 
 PageAction::PageAction()
-  : type_(PERMANENT),
-    active_tab_id_(-1) {
+  : type_(PERMANENT) {
 }
 
 PageAction::~PageAction() {
 }
-
-// TODO(finnur): The tracking of active tab and url probably needs to change
-// but it is hard to test while we are hard coding the tab index, so I'll
-// leave it for later.
-void PageAction::SetActiveTabIdAndUrl(int tab_id, const GURL& url) {
-  active_tab_id_ = tab_id;
-  active_url_ = url;
-}
-
-bool PageAction::IsActive(int tab_id, const GURL& url) const {
-  return !active_url_.is_empty() &&
-         url == active_url_ &&
-         tab_id == active_tab_id_;
-}
diff --git a/chrome/common/page_action.h b/chrome/common/page_action.h
index c22b96f..69bd238 100644
--- a/chrome/common/page_action.h
+++ b/chrome/common/page_action.h
@@ -45,11 +45,6 @@ class PageAction {
     tooltip_ = tooltip;
   }
 
-  // Sets the active tab and url for this PageAction.
-  void SetActiveTabIdAndUrl(int tab_id, const GURL& url);
-  // Returns true if the PageAction is active in the specified |tab_id| & |url|.
-  bool IsActive(int tab_id, const GURL& url) const;
-
  private:
   // The id for the PageAction, for example: "RssPageAction".
   std::string id_;
@@ -69,12 +64,6 @@ class PageAction {
 
   // The tooltip to show when the mouse hovers over the icon of the page action.
   std::string tooltip_;
-
-  // The url of the page which the PageAction is currently active in.
-  GURL active_url_;
-
-  // The tab ID for which the PageAction is currently active in.
-  int active_tab_id_;
 };
 
 typedef std::map<std::string, PageAction*> PageActionMap;