FormatErrorMessage() functions are now publicly available from ExtensionErrorUtils.

ExtensionTabsModule implements a bunch of error_messages.

Extension Calls now always deliver a response to the calling context and route error messages if any to the window.console.error log.

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

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

1 files changed, 101 insertions, 73 deletions

diff --git a/chrome/browser/extensions/extension_tabs_module.cc b/chrome/browser/extensions/extension_tabs_module.cc
index 39066c3..6e61976 100644
--- a/chrome/browser/extensions/extension_tabs_module.cc
+++ b/chrome/browser/extensions/extension_tabs_module.cc
@@ -7,6 +7,7 @@
 #include "base/string_util.h"
 #include "chrome/browser/browser.h"
 #include "chrome/browser/browser_list.h"
+#include "chrome/browser/extensions/extension_error_utils.h"
 #include "chrome/browser/extensions/extension_function_dispatcher.h"
 #include "chrome/browser/renderer_host/render_view_host_delegate.h"
 #include "chrome/browser/tab_contents/navigation_entry.h"
@@ -33,13 +34,29 @@ const wchar_t* kTopKey = L"top";
 const wchar_t* kWidthKey = L"width";
 const wchar_t* kHeightKey = L"height";
 const wchar_t* kTabsKey = L"tabs";
+
+// errors
+const char* kWindowNotFoundError = "No window with id: *.";
+const char* kTabNotFoundError = "No tab with id: *.";
+const char* kInvalidUrlError = "Invalid url: \"*\".";
 }
 
 // Forward declare static helper functions defined below.
 static DictionaryValue* CreateWindowValue(Browser* browser, bool populate_tabs);
 static ListValue* CreateTabList(Browser* browser);
+
+// |error_message| can optionally be passed in a will be set with an appropriate
+// message if the window cannot be found by id.
 static Browser* GetBrowserInProfileWithId(Profile* profile,
-                                          const int window_id);
+                                          const int window_id,
+                                          std::string* error_message);
+
+// |error_message| can optionally be passed in a will be set with an appropriate
+// message if the tab cannot be found by id.
+static bool GetTabById(int tab_id, Profile* profile, Browser** browser,
+                       TabStripModel** tab_strip,
+                       TabContents** contents,
+                       int* tab_index, std::string* error_message);
 
 // ExtensionTabUtil
 int ExtensionTabUtil::GetWindowId(const Browser* browser) {
@@ -123,35 +140,30 @@ bool ExtensionTabUtil::GetTabById(int tab_id, Profile* profile,
   return false;
 }
 
-static bool GetWindowFunctionHelper(Browser* browser, Profile* profile,
-                            scoped_ptr<Value>* result) {
-  // TODO(rafaelw): need "not found" error message.
-  if (browser == NULL || browser->profile() != profile)
-    return false;
-
-  result->reset(CreateWindowValue(browser, false));
-
-  return true;
-}
-
 // Windows ---------------------------------------------------------------------
 
 bool GetWindowFunction::RunImpl() {
   int window_id;
   EXTENSION_FUNCTION_VALIDATE(args_->GetAsInteger(&window_id));
 
-  Browser* target = GetBrowserInProfileWithId(profile(), window_id);
-  return GetWindowFunctionHelper(target, profile(), &result_);
+  Browser* browser = GetBrowserInProfileWithId(profile(), window_id, &error_);
+  if (!browser)
+    return false;
+
+  result_.reset(CreateWindowValue(browser, false));
+  return true;
 }
 
 bool GetCurrentWindowFunction::RunImpl() {
-  return GetWindowFunctionHelper(dispatcher_->GetBrowser(), profile(),
-                                 &result_);
+  Browser* browser = dispatcher_->GetBrowser();
+  result_.reset(CreateWindowValue(browser, false));
+  return true;
 }
 
-bool GetFocusedWindowFunction::RunImpl() {
-  return GetWindowFunctionHelper(BrowserList::GetLastActive(), profile(),
-      &result_);
+bool GetLastFocusedWindowFunction::RunImpl() {
+  Browser* browser = BrowserList::GetLastActiveWithProfile(profile());
+  result_.reset(CreateWindowValue(browser, false));
+  return true;
 }
 
 bool GetAllWindowsFunction::RunImpl() {
@@ -185,7 +197,8 @@ bool CreateWindowFunction::RunImpl() {
       EXTENSION_FUNCTION_VALIDATE(args->GetString(kUrlKey, &url_input));
       url.reset(new GURL(url_input));
       if (!url->is_valid()) {
-        // TODO(rafaelw): need error message/callback here
+        error_ = ExtensionErrorUtils::FormatErrorMessage(kInvalidUrlError,
+            url_input);
         return false;
       }
     }
@@ -244,24 +257,11 @@ bool RemoveWindowFunction::RunImpl() {
   int window_id;
   EXTENSION_FUNCTION_VALIDATE(args_->GetAsInteger(&window_id));
 
-  Browser* target = NULL;
-  for (BrowserList::const_iterator browser = BrowserList::begin();
-      browser != BrowserList::end(); ++browser) {
-    // Only examine browsers in the current profile.
-    if ((*browser)->profile() == profile()) {
-      if (ExtensionTabUtil::GetWindowId(*browser) == window_id) {
-        target = *browser;
-        break;
-      }
-    }
-  }
-
-  if (target == NULL) {
-    // TODO(rafaelw): need error message.
+  Browser* browser = GetBrowserInProfileWithId(profile(), window_id, &error_);
+  if (!browser)
     return false;
-  }
 
-  target->CloseWindow();
+  browser->CloseWindow();
 
   return true;
 }
@@ -269,12 +269,15 @@ bool RemoveWindowFunction::RunImpl() {
 // Tabs ---------------------------------------------------------------------
 
 bool GetSelectedTabFunction::RunImpl() {
-  int window_id;
   Browser* browser;
   // windowId defaults to "current" window.
+  int window_id = -1;
+
   if (!args_->IsType(Value::TYPE_NULL)) {
     EXTENSION_FUNCTION_VALIDATE(args_->GetAsInteger(&window_id));
-    browser = GetBrowserInProfileWithId(profile(), window_id);
+    browser = GetBrowserInProfileWithId(profile(), window_id, &error_);
+    if (!browser)
+      return false;
   } else {
     browser = dispatcher_->GetBrowser();
   }
@@ -288,12 +291,14 @@ bool GetSelectedTabFunction::RunImpl() {
 }
 
 bool GetAllTabsInWindowFunction::RunImpl() {
-  int window_id;
   Browser* browser;
   // windowId defaults to "current" window.
+  int window_id = -1;
   if (!args_->IsType(Value::TYPE_NULL)) {
     EXTENSION_FUNCTION_VALIDATE(args_->GetAsInteger(&window_id));
-    browser = GetBrowserInProfileWithId(profile(), window_id);
+    browser = GetBrowserInProfileWithId(profile(), window_id, &error_);
+    if (!browser)
+      return false;
   } else {
     browser = dispatcher_->GetBrowser();
   }
@@ -307,14 +312,16 @@ bool CreateTabFunction::RunImpl() {
   EXTENSION_FUNCTION_VALIDATE(args_->IsType(Value::TYPE_DICTIONARY));
   const DictionaryValue* args = static_cast<const DictionaryValue*>(args_);
 
-  int window_id;
-  Browser* browser;
+  Browser *browser;
   // windowId defaults to "current" window.
+  int window_id = -1;
   if (args->HasKey(kWindowIdKey)) {
     EXTENSION_FUNCTION_VALIDATE(args->GetInteger(kWindowIdKey, &window_id));
-    browser = GetBrowserInProfileWithId(profile(), window_id);
+    browser = GetBrowserInProfileWithId(profile(), window_id, &error_);
+    if (!browser)
+      return false;
   } else {
-    browser = dispatcher_->GetBrowser();
+      browser = dispatcher_->GetBrowser();
   }
 
   TabStripModel* tab_strip = browser->tabstrip_model();
@@ -328,9 +335,11 @@ bool CreateTabFunction::RunImpl() {
   if (args->HasKey(kUrlKey)) {
     EXTENSION_FUNCTION_VALIDATE(args->GetString(kUrlKey, &url_string));
     url.reset(new GURL(url_string));
-    // TODO(rafaelw): return an "invalid url" error.
-    if (!url->is_valid())
+    if (!url->is_valid()) {
+      error_ = ExtensionErrorUtils::FormatErrorMessage(kInvalidUrlError,
+          url_string);
       return false;
+    }
   }
 
   // Default to foreground for the new tab. The presence of 'selected' property
@@ -371,9 +380,8 @@ bool GetTabFunction::RunImpl() {
   TabStripModel* tab_strip = NULL;
   TabContents* contents = NULL;
   int tab_index = -1;
-  // TODO(rafaelw): return "tab_id not found" error.
-  if (!ExtensionTabUtil::GetTabById(tab_id, profile(), NULL, &tab_strip,
-                                    &contents, &tab_index))
+  if (!GetTabById(tab_id, profile(), NULL, &tab_strip, &contents, &tab_index,
+      &error_))
     return false;
 
   result_.reset(ExtensionTabUtil::CreateTabValue(contents, tab_strip,
@@ -392,9 +400,8 @@ bool UpdateTabFunction::RunImpl() {
   TabStripModel* tab_strip = NULL;
   TabContents* contents = NULL;
   int tab_index = -1;
-  // TODO(rafaelw): return "tab_id not found" error.
-  if (!ExtensionTabUtil::GetTabById(tab_id, profile(), NULL, &tab_strip,
-                                    &contents, &tab_index))
+  if (!GetTabById(tab_id, profile(), NULL, &tab_strip, &contents, &tab_index,
+      &error_))
     return false;
 
   NavigationController& controller = contents->controller();
@@ -409,9 +416,10 @@ bool UpdateTabFunction::RunImpl() {
     EXTENSION_FUNCTION_VALIDATE(update_props->GetString(kUrlKey, &url));
     GURL new_gurl(url);
 
-    // TODO(rafaelw): return "invalid url" here.
-    if (!new_gurl.is_valid())
+    if (!new_gurl.is_valid()) {
+      error_ = ExtensionErrorUtils::FormatErrorMessage(kInvalidUrlError, url);
       return false;
+    }
 
     controller.LoadURL(new_gurl, GURL(), PageTransition::LINK);
   }
@@ -445,19 +453,18 @@ bool MoveTabFunction::RunImpl() {
   Browser* source_browser = NULL;
   TabStripModel* source_tab_strip = NULL;
   int tab_index = -1;
-  // TODO(rafaelw): return "tab_id not found" error.
-  if (!ExtensionTabUtil::GetTabById(tab_id, profile(), &source_browser,
-                                    &source_tab_strip, NULL,
-                                    &tab_index))
+  if (!GetTabById(tab_id, profile(), &source_browser, &source_tab_strip, NULL,
+      &tab_index, &error_))
     return false;
 
   if (update_props->HasKey(kWindowIdKey)) {
+    Browser* target_browser;
     int window_id;
     EXTENSION_FUNCTION_VALIDATE(update_props->GetInteger(kWindowIdKey,
         &window_id));
-    Browser* target_browser = GetBrowserInProfileWithId(profile(), window_id);
-    // TODO(rafaelw): return "window_id not found" error.
-    if (!target_browser) 
+    target_browser = GetBrowserInProfileWithId(profile(), window_id,
+        &error_);
+    if (!target_browser)
       return false;
 
     // If windowId is different from the current window, move between windows.
@@ -465,10 +472,12 @@ bool MoveTabFunction::RunImpl() {
         ExtensionTabUtil::GetWindowId(source_browser)) {
       TabStripModel* target_tab_strip = target_browser->tabstrip_model();
       TabContents *contents = source_tab_strip->DetachTabContentsAt(tab_index);
-      // TODO(rafaelw): return a "tab not found" error.
-      if (!contents)
+      if (!contents) {
+        error_ = ExtensionErrorUtils::FormatErrorMessage(
+            kTabNotFoundError, IntToString(tab_id));
         return false;
-      
+      }
+
       // Clamp move location to the last position.
       // This is ">" because it can append to a new index position.
       if (new_index > target_tab_strip->count())
@@ -478,15 +487,15 @@ bool MoveTabFunction::RunImpl() {
           false, true);
 
       return true;
-    } 
+    }
   }
- 
+
   // Perform a simple within-window move.
   // Clamp move location to the last position.
   // This is ">=" because the move must be to an existing location.
   if (new_index >= source_tab_strip->count())
     new_index = source_tab_strip->count() - 1;
-    
+
   if (new_index != tab_index)
     source_tab_strip->MoveTabContentsAt(tab_index, new_index, false);
 
@@ -495,16 +504,12 @@ bool MoveTabFunction::RunImpl() {
 
 
 bool RemoveTabFunction::RunImpl() {
-  // TODO(rafaelw): This should have a callback, but it can't because it could
-  // close it's own tab.
   int tab_id;
   EXTENSION_FUNCTION_VALIDATE(args_->GetAsInteger(&tab_id));
 
   Browser* browser = NULL;
   TabContents* contents = NULL;
-  // TODO(rafaelw): return "tab_id not found" error.
-  if (!ExtensionTabUtil::GetTabById(tab_id, profile(), &browser, NULL,
-                                    &contents, NULL))
+  if (!GetTabById(tab_id, profile(), &browser, NULL, &contents, NULL, &error_))
     return false;
 
   browser->CloseTabContents(contents);
@@ -512,6 +517,7 @@ bool RemoveTabFunction::RunImpl() {
 }
 
 // static helpers
+
 // if |populate| is true, each window gets a list property |tabs| which contains
 // fully populated tab objects.
 static DictionaryValue* CreateWindowValue(Browser* browser,
@@ -546,12 +552,34 @@ static ListValue* CreateTabList(Browser* browser) {
 }
 
 static Browser* GetBrowserInProfileWithId(Profile* profile,
-                                          const int window_id) {
+                                          const int window_id,
+                                          std::string* error_message) {
   for (BrowserList::const_iterator browser = BrowserList::begin();
       browser != BrowserList::end(); ++browser) {
     if ((*browser)->profile() == profile &&
         ExtensionTabUtil::GetWindowId(*browser) == window_id)
       return *browser;
   }
+
+  if (error_message)
+    *error_message= ExtensionErrorUtils::FormatErrorMessage(
+        kWindowNotFoundError, IntToString(window_id));
+
   return NULL;
 }
+
+static bool GetTabById(int tab_id, Profile* profile, Browser** browser,
+                       TabStripModel** tab_strip,
+                       TabContents** contents,
+                       int* tab_index,
+                       std::string* error_message) {
+  if (ExtensionTabUtil::GetTabById(tab_id, profile, browser, tab_strip,
+                                   contents, tab_index))
+    return true;
+  
+  if (error_message)
+    *error_message = ExtensionErrorUtils::FormatErrorMessage(
+        kTabNotFoundError, IntToString(tab_id));
+    
+  return false;
+}