Add error handling to PageAction extension API.

BUG=None
TEST=None

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

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

2 files changed, 29 insertions, 13 deletions

diff --git a/chrome/browser/extensions/extension_page_actions_module.cc b/chrome/browser/extensions/extension_page_actions_module.cc
index bf23f65..d1fa723 100644
--- a/chrome/browser/extensions/extension_page_actions_module.cc
+++ b/chrome/browser/extensions/extension_page_actions_module.cc
@@ -8,10 +8,19 @@
 #include "chrome/browser/browser_list.h"
 #include "chrome/browser/profile.h"
 #include "chrome/browser/extensions/extension.h"
+#include "chrome/browser/extensions/extension_error_utils.h"
 #include "chrome/browser/extensions/extension_tabs_module.h"
 #include "chrome/browser/extensions/extensions_service.h"
 #include "chrome/browser/tab_contents/navigation_entry.h"
 
+namespace {
+  // Error messages.
+  const char* kNoExtensionError = "No extension with id: *.";
+  const char* kNoTabError = "No tab with id: *.";
+  const char* kNoPageActionError = "No PageAction with id: *.";
+  const char* kUrlNotActiveError = "This url is no longer active: *.";
+}
+
 bool EnablePageActionFunction::RunImpl() {
   EXTENSION_FUNCTION_VALIDATE(args_->IsType(Value::TYPE_LIST));
   const ListValue* args = static_cast<const ListValue*>(args_);
@@ -29,27 +38,34 @@ bool EnablePageActionFunction::RunImpl() {
   // Find the TabContents that contains this tab id.
   TabContents* contents = NULL;
   ExtensionTabUtil::GetTabById(tab_id, profile(), NULL, NULL, &contents, NULL);
-  if (!contents)
+  if (!contents) {
+    error_ = ExtensionErrorUtils::FormatErrorMessage(kNoTabError,
+                                                     IntToString(tab_id));
     return false;
+  }
 
   // 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())
+  if (url != contents->controller().GetActiveEntry()->url().spec()) {
+    error_ = ExtensionErrorUtils::FormatErrorMessage(kUrlNotActiveError, url);
     return false;
+  }
 
   // Find our extension.
   Extension* extension = NULL;
   ExtensionsService* service = profile()->GetExtensionsService();
-  if (service)
-    extension = service->GetExtensionByID(extension_id());
-  else
-    NOTREACHED();
-  if (!extension)
+  extension = service->GetExtensionByID(extension_id());
+  if (!extension) {
+    error_ = ExtensionErrorUtils::FormatErrorMessage(kNoExtensionError,
+                                                     extension_id());
     return false;
+  }
 
   const PageAction* page_action = extension->GetPageAction(page_action_id);
-  if (!page_action)
+  if (!page_action) {
+    error_ = ExtensionErrorUtils::FormatErrorMessage(kNoPageActionError,
+                                                     page_action_id);
     return false;
+  }
 
   // Set visible and broadcast notifications that the UI should be updated.
   contents->EnablePageAction(page_action);
diff --git a/chrome/browser/extensions/extension_tabs_module.cc b/chrome/browser/extensions/extension_tabs_module.cc
index 82a2770..6752449 100644
--- a/chrome/browser/extensions/extension_tabs_module.cc
+++ b/chrome/browser/extensions/extension_tabs_module.cc
@@ -22,7 +22,7 @@
 #endif
 
 namespace {
-// keys
+// Keys.
 const wchar_t* kIdKey = L"id";
 const wchar_t* kIndexKey = L"index";
 const wchar_t* kWindowIdKey = L"windowId";
@@ -37,7 +37,7 @@ const wchar_t* kWidthKey = L"width";
 const wchar_t* kHeightKey = L"height";
 const wchar_t* kTabsKey = L"tabs";
 
-// errors
+// Error messages.
 const char* kWindowNotFoundError = "No window with id: *.";
 const char* kTabNotFoundError = "No tab with id: *.";
 const char* kInvalidUrlError = "Invalid url: \"*\".";
@@ -321,7 +321,7 @@ bool RemoveWindowFunction::RunImpl() {
   return true;
 }
 
-// Tabs ---------------------------------------------------------------------
+// Tabs ------------------------------------------------------------------------
 
 bool GetSelectedTabFunction::RunImpl() {
   Browser* browser;
@@ -414,7 +414,7 @@ bool CreateTabFunction::RunImpl() {
     EXTENSION_FUNCTION_VALIDATE(args->GetInteger(kIndexKey, &index));
 
   if (index < 0) {
-    // Default insert behavior
+    // Default insert behavior.
     index = -1;
   }
   if (index > tab_strip->count()) {