[Extensions] Convert some ChromeSyncExtensionFunctions

chrome/browser/extensions/api/tabs/tabs_api.cc

Index: chrome/browser/extensions/api/tabs/tabs_api.cc
diff --git a/chrome/browser/extensions/api/tabs/tabs_api.cc b/chrome/browser/extensions/api/tabs/tabs_api.cc
index 3ca082f6958ddb27217f7d39980dc2cac49d9d48..0fba3dff671924e5313a17d03ae1d7e0ea35b084 100644
--- a/chrome/browser/extensions/api/tabs/tabs_api.cc
+++ b/chrome/browser/extensions/api/tabs/tabs_api.cc
@@ -154,7 +154,7 @@ bool GetBrowserFromWindowID(const ChromeExtensionFunctionDetails& details,
   return true;
 }
 
-bool GetBrowserFromWindowID(ChromeUIThreadExtensionFunction* function,
+bool GetBrowserFromWindowID(UIThreadExtensionFunction* function,
                             int window_id,
                             Browser** browser,
                             std::string* error) {
@@ -165,15 +165,15 @@ bool GetBrowserFromWindowID(ChromeUIThreadExtensionFunction* function,
 // |error_message| can optionally be passed in and will be set with an
 // appropriate message if the tab cannot be found by id.
 bool GetTabById(int tab_id,
-                Profile* profile,
+                content::BrowserContext* context,
                 bool include_incognito,
                 Browser** browser,
                 TabStripModel** tab_strip,
                 content::WebContents** contents,
                 int* tab_index,
                 std::string* error_message) {
-  if (ExtensionTabUtil::GetTabById(tab_id, profile, include_incognito,
-                                   browser, tab_strip, contents, tab_index)) {
+  if (ExtensionTabUtil::GetTabById(tab_id, context, include_incognito, browser,
+                                   tab_strip, contents, tab_index)) {
     return true;
   }
 
@@ -275,46 +275,49 @@ void ZoomModeToZoomSettings(ZoomController::ZoomMode zoom_mode,
 
 // Windows ---------------------------------------------------------------------
 
-bool WindowsGetFunction::RunSync() {
+ExtensionFunction::ResponseAction WindowsGetFunction::Run() {
   std::unique_ptr<windows::Get::Params> params(
       windows::Get::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
 
   ApiParameterExtractor<windows::Get::Params> extractor(params.get());
-  WindowController* controller;
+  WindowController* controller = nullptr;
+  std::string error;
   if (!windows_util::GetWindowFromWindowID(this, params->window_id,
                                            extractor.type_filters(),
-                                           &controller, &error_)) {
-    return false;
+                                           &controller, &error)) {
+    return RespondNow(Error(error));
   }
 
-  if (extractor.populate_tabs())
-    SetResult(controller->CreateWindowValueWithTabs(extension()));
-  else
-    SetResult(controller->CreateWindowValue());
-  return true;
+  std::unique_ptr<base::DictionaryValue> windows =
+      extractor.populate_tabs()
+          ? controller->CreateWindowValueWithTabs(extension())
+          : controller->CreateWindowValue();
+  return RespondNow(OneArgument(std::move(windows)));
 }
 
-bool WindowsGetCurrentFunction::RunSync() {
+ExtensionFunction::ResponseAction WindowsGetCurrentFunction::Run() {
   std::unique_ptr<windows::GetCurrent::Params> params(
       windows::GetCurrent::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
 
   ApiParameterExtractor<windows::GetCurrent::Params> extractor(params.get());
-  WindowController* controller;
+  WindowController* controller = nullptr;
+  std::string error;
   if (!windows_util::GetWindowFromWindowID(
           this, extension_misc::kCurrentWindowId, extractor.type_filters(),
-          &controller, &error_)) {
-    return false;
+          &controller, &error)) {
+    return RespondNow(Error(error));
   }
-  if (extractor.populate_tabs())
-    SetResult(controller->CreateWindowValueWithTabs(extension()));
-  else
-    SetResult(controller->CreateWindowValue());
-  return true;
+
+  std::unique_ptr<base::DictionaryValue> windows =
+      extractor.populate_tabs()
+          ? controller->CreateWindowValueWithTabs(extension())
+          : controller->CreateWindowValue();
+  return RespondNow(OneArgument(std::move(windows)));
 }
 
-bool WindowsGetLastFocusedFunction::RunSync() {
+ExtensionFunction::ResponseAction WindowsGetLastFocusedFunction::Run() {
   std::unique_ptr<windows::GetLastFocused::Params> params(
       windows::GetLastFocused::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
@@ -332,18 +335,17 @@ bool WindowsGetLastFocusedFunction::RunSync() {
         break;  // Use focused window.
     }
   }
-  if (!controller) {
-    error_ = keys::kNoLastFocusedWindowError;
-    return false;
-  }
-  if (extractor.populate_tabs())
-    SetResult(controller->CreateWindowValueWithTabs(extension()));
-  else
-    SetResult(controller->CreateWindowValue());
-  return true;
+  if (!controller)
+    return RespondNow(Error(keys::kNoLastFocusedWindowError));
+
+  std::unique_ptr<base::DictionaryValue> windows =
+      extractor.populate_tabs()
+          ? controller->CreateWindowValueWithTabs(extension())
+          : controller->CreateWindowValue();
+  return RespondNow(OneArgument(std::move(windows)));
 }
 
-bool WindowsGetAllFunction::RunSync() {
+ExtensionFunction::ResponseAction WindowsGetAllFunction::Run() {
   std::unique_ptr<windows::GetAll::Params> params(
       windows::GetAll::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
@@ -363,27 +365,26 @@ bool WindowsGetAllFunction::RunSync() {
     else
       window_list->Append((*iter)->CreateWindowValue());
   }
-  SetResult(std::move(window_list));
-  return true;
+
+  return RespondNow(OneArgument(std::move(window_list)));
 }
 
 bool WindowsCreateFunction::ShouldOpenIncognitoWindow(
     const windows::Create::Params::CreateData* create_data,
-    std::vector<GURL>* urls, bool* is_error) {
-  *is_error = false;
+    std::vector<GURL>* urls,
+    std::string* error) {
+  Profile* profile = Profile::FromBrowserContext(browser_context());
   const IncognitoModePrefs::Availability incognito_availability =
-      IncognitoModePrefs::GetAvailability(GetProfile()->GetPrefs());
+      IncognitoModePrefs::GetAvailability(profile->GetPrefs());
   bool incognito = false;
   if (create_data && create_data->incognito) {
     incognito = *create_data->incognito;
     if (incognito && incognito_availability == IncognitoModePrefs::DISABLED) {
-      error_ = keys::kIncognitoModeIsDisabled;
-      *is_error = true;
+      *error = keys::kIncognitoModeIsDisabled;
       return false;
     }
     if (!incognito && incognito_availability == IncognitoModePrefs::FORCED) {
-      error_ = keys::kIncognitoModeIsForced;
-      *is_error = true;
+      *error = keys::kIncognitoModeIsForced;
       return false;
     }
   } else if (incognito_availability == IncognitoModePrefs::FORCED) {
@@ -394,10 +395,10 @@ bool WindowsCreateFunction::ShouldOpenIncognitoWindow(
 
   // Remove all URLs that are not allowed in an incognito session. Note that a
   // ChromeOS guest session is not considered incognito in this case.
-  if (incognito && !GetProfile()->IsGuestSession()) {
+  if (incognito && !profile->IsGuestSession()) {
     std::string first_url_erased;
     for (size_t i = 0; i < urls->size();) {
-      if (chrome::IsURLAllowedInIncognito((*urls)[i], GetProfile())) {
+      if (chrome::IsURLAllowedInIncognito((*urls)[i], profile)) {
         i++;
       } else {
         if (first_url_erased.empty())
@@ -406,16 +407,15 @@ bool WindowsCreateFunction::ShouldOpenIncognitoWindow(
       }
     }
     if (urls->empty() && !first_url_erased.empty()) {
-      error_ = ErrorUtils::FormatErrorMessage(
+      *error = ErrorUtils::FormatErrorMessage(
           keys::kURLsNotAllowedInIncognitoError, first_url_erased);
-      *is_error = true;
       return false;
     }
   }
   return incognito;
 }
 
-bool WindowsCreateFunction::RunSync() {
+ExtensionFunction::ResponseAction WindowsCreateFunction::Run() {
   std::unique_ptr<windows::Create::Params> params(
       windows::Create::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params);
@@ -438,61 +438,48 @@ bool WindowsCreateFunction::RunSync() {
     for (std::vector<std::string>::iterator i = url_strings.begin();
          i != url_strings.end(); ++i) {
       GURL url = ExtensionTabUtil::ResolvePossiblyRelativeURL(*i, extension());
-      if (!url.is_valid()) {
-        error_ = ErrorUtils::FormatErrorMessage(keys::kInvalidUrlError, *i);
-        return false;
-      }
+      if (!url.is_valid())
+        return RespondNow(Error(keys::kInvalidUrlError, *i));
       // Don't let the extension crash the browser or renderers.
-      if (ExtensionTabUtil::IsKillURL(url)) {
-        error_ = keys::kNoCrashBrowserError;
-        return false;
-      }
+      if (ExtensionTabUtil::IsKillURL(url))
+        return RespondNow(Error(keys::kNoCrashBrowserError));
       urls.push_back(url);
     }
   }
 
   // Decide whether we are opening a normal window or an incognito window.
-  bool is_error = true;
+  std::string error;
   bool open_incognito_window =
-      ShouldOpenIncognitoWindow(create_data, &urls, &is_error);
-  if (is_error)
-    return false;  // error_ member is set inside of ShouldOpenIncognitoWindow.
+      ShouldOpenIncognitoWindow(create_data, &urls, &error);
+  if (!error.empty())
+    return RespondNow(Error(error));
 
-  Profile* window_profile = GetProfile();
-  if (open_incognito_window)
-    window_profile = window_profile->GetOffTheRecordProfile();
+  Profile* calling_profile = Profile::FromBrowserContext(browser_context());
+  Profile* window_profile = open_incognito_window
+                                ? calling_profile->GetOffTheRecordProfile()
+                                : calling_profile;
 
   // Look for optional tab id.
   if (create_data && create_data->tab_id) {
     // Find the tab. |source_tab_strip| and |tab_index| will later be used to
     // move the tab into the created window.
     Browser* source_browser = nullptr;
-    if (!GetTabById(*create_data->tab_id,
-                    GetProfile(),
-                    include_incognito(),
-                    &source_browser,
-                    &source_tab_strip,
-                    nullptr,
-                    &tab_index,
-                    &error_))
-      return false;
-
-    if (!source_browser->window()->IsTabStripEditable()) {
-      error_ = keys::kTabStripNotEditableError;
-      return false;
+    if (!GetTabById(*create_data->tab_id, calling_profile, include_incognito(),
+                    &source_browser, &source_tab_strip, nullptr, &tab_index,
+                    &error)) {
+      return RespondNow(Error(error));
     }
 
-    if (source_browser->profile() != window_profile) {
-      error_ = keys::kCanOnlyMoveTabsWithinSameProfileError;
-      return false;
-    }
-  }
+    if (!source_browser->window()->IsTabStripEditable())
+      return RespondNow(Error(keys::kTabStripNotEditableError));
 
-  if (!IsValidStateForWindowsCreateFunction(create_data)) {
-    error_ = keys::kInvalidWindowStateError;
-    return false;
+    if (source_browser->profile() != window_profile)
+      return RespondNow(Error(keys::kCanOnlyMoveTabsWithinSameProfileError));
   }
 
+  if (!IsValidStateForWindowsCreateFunction(create_data))
+    return RespondNow(Error(keys::kInvalidWindowStateError));
+
   Browser::Type window_type = Browser::TYPE_TABBED;
 
 #if defined(USE_ASH)
@@ -539,8 +526,7 @@ bool WindowsCreateFunction::RunSync() {
       case windows::CREATE_TYPE_NORMAL:
         break;
       default:
-        error_ = keys::kInvalidWindowTypeError;
-        return false;
+        return RespondNow(Error(keys::kInvalidWindowTypeError));
     }
 
     // Initialize default window bounds according to window type.
@@ -554,11 +540,10 @@ bool WindowsCreateFunction::RunSync() {
       // GetBrowserWindowBounds will default to saved "default" values for
       // the app.
       ui::WindowShowState show_state = ui::SHOW_STATE_DEFAULT;
-      WindowSizer::GetBrowserWindowBoundsAndShowState(std::string(),
-                                                      gfx::Rect(),
-                                                      GetCurrentBrowser(),
-                                                      &window_bounds,
-                                                      &show_state);
+      WindowSizer::GetBrowserWindowBoundsAndShowState(
+          std::string(), gfx::Rect(),
+          ChromeExtensionFunctionDetails(this).GetCurrentBrowser(),
+          &window_bounds, &show_state);
     }
 
     // Any part of the bounds can optionally be set by the caller.
@@ -601,9 +586,9 @@ bool WindowsCreateFunction::RunSync() {
         WindowControllerList::GetInstance()->FindWindowById(
             app_window->session_id().id());
     if (!window_controller)
-      return false;
-    SetResult(window_controller->CreateWindowValueWithTabs(extension()));
-    return true;
+      return RespondNow(Error(kUnknownErrorDoNotUse));
+    return RespondNow(
+        OneArgument(window_controller->CreateWindowValueWithTabs(extension())));
   }
 #endif  // defined(USE_ASH)
 
@@ -660,28 +645,30 @@ bool WindowsCreateFunction::RunSync() {
 
   WindowController* controller = new_window->extension_window_controller();
 
+  std::unique_ptr<base::Value> result;
   if (new_window->profile()->IsOffTheRecord() &&
-      !GetProfile()->IsOffTheRecord() && !include_incognito()) {
+      !browser_context()->IsOffTheRecord() && !include_incognito()) {
     // Don't expose incognito windows if extension itself works in non-incognito
     // profile and CanCrossIncognito isn't allowed.
-    SetResult(base::Value::CreateNullValue());
+    result = base::Value::CreateNullValue();
   } else {
-    SetResult(controller->CreateWindowValueWithTabs(extension()));
+    result = controller->CreateWindowValueWithTabs(extension());
   }
 
-  return true;
+  return RespondNow(OneArgument(std::move(result)));
 }
 
-bool WindowsUpdateFunction::RunSync() {
+ExtensionFunction::ResponseAction WindowsUpdateFunction::Run() {
   std::unique_ptr<windows::Update::Params> params(
       windows::Update::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params);
 
   WindowController* controller;
+  std::string error;
   if (!windows_util::GetWindowFromWindowID(
           this, params->window_id, WindowController::GetAllWindowFilter(),
-          &controller, &error_)) {
-    return false;
+          &controller, &error)) {
+    return RespondNow(Error(error));
   }
 
   ui::WindowShowState show_state =
@@ -743,8 +730,7 @@ bool WindowsUpdateFunction::RunSync() {
     if (show_state == ui::SHOW_STATE_MINIMIZED ||
         show_state == ui::SHOW_STATE_MAXIMIZED ||
         show_state == ui::SHOW_STATE_FULLSCREEN) {
-      error_ = keys::kInvalidWindowStateError;
-      return false;
+      return RespondNow(Error(keys::kInvalidWindowStateError));
     }
     // TODO(varkha): Updating bounds during a drag can cause problems and a more
     // general solution is needed. See http://crbug.com/251813 .
@@ -753,16 +739,13 @@ bool WindowsUpdateFunction::RunSync() {
 
   if (params->update_info.focused) {
     if (*params->update_info.focused) {
-      if (show_state == ui::SHOW_STATE_MINIMIZED) {
-        error_ = keys::kInvalidWindowStateError;
-        return false;
-      }
+      if (show_state == ui::SHOW_STATE_MINIMIZED)
+        return RespondNow(Error(keys::kInvalidWindowStateError));
       controller->window()->Activate();
     } else {
       if (show_state == ui::SHOW_STATE_MAXIMIZED ||
           show_state == ui::SHOW_STATE_FULLSCREEN) {
-        error_ = keys::kInvalidWindowStateError;
-        return false;
+        return RespondNow(Error(keys::kInvalidWindowStateError));
       }
       controller->window()->Deactivate();
     }
@@ -771,36 +754,35 @@ bool WindowsUpdateFunction::RunSync() {
   if (params->update_info.draw_attention)
     controller->window()->FlashFrame(*params->update_info.draw_attention);
 
-  SetResult(controller->CreateWindowValue());
-
-  return true;
+  return RespondNow(OneArgument(controller->CreateWindowValue()));
 }
 
-bool WindowsRemoveFunction::RunSync() {
+ExtensionFunction::ResponseAction WindowsRemoveFunction::Run() {
   std::unique_ptr<windows::Remove::Params> params(
       windows::Remove::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params);
 
-  WindowController* controller;
+  WindowController* controller = nullptr;
+  std::string error;
   if (!windows_util::GetWindowFromWindowID(this, params->window_id,
                                            WindowController::kNoWindowFilter,
-                                           &controller, &error_)) {
-    return false;
+                                           &controller, &error)) {
+    return RespondNow(Error(error));
   }
 
   WindowController::Reason reason;
   if (!controller->CanClose(&reason)) {
-    if (reason == WindowController::REASON_NOT_EDITABLE)
-      error_ = keys::kTabStripNotEditableError;
-    return false;
+    return RespondNow(Error(reason == WindowController::REASON_NOT_EDITABLE
+                                ? keys::kTabStripNotEditableError
+                                : kUnknownErrorDoNotUse));
   }
   controller->window()->Close();
-  return true;
+  return RespondNow(NoArguments());
 }
 
 // Tabs ------------------------------------------------------------------------
 
-bool TabsGetSelectedFunction::RunSync() {
+ExtensionFunction::ResponseAction TabsGetSelectedFunction::Run() {
   // windowId defaults to "current" window.
   int window_id = extension_misc::kCurrentWindowId;
 
@@ -811,21 +793,20 @@ bool TabsGetSelectedFunction::RunSync() {
     window_id = *params->window_id;
 
   Browser* browser = NULL;
-  if (!GetBrowserFromWindowID(this, window_id, &browser, &error_))
-    return false;
+  std::string error;
+  if (!GetBrowserFromWindowID(this, window_id, &browser, &error))
+    return RespondNow(Error(error));
 
   TabStripModel* tab_strip = browser->tab_strip_model();
   WebContents* contents = tab_strip->GetActiveWebContents();
-  if (!contents) {
-    error_ = keys::kNoSelectedTabError;
-    return false;
-  }
-  results_ = tabs::Get::Results::Create(*ExtensionTabUtil::CreateTabObject(
-      contents, tab_strip, tab_strip->active_index(), extension()));
-  return true;
+  if (!contents)
+    return RespondNow(Error(keys::kNoSelectedTabError));
+  return RespondNow(ArgumentList(
+      tabs::Get::Results::Create(*ExtensionTabUtil::CreateTabObject(
+          contents, tab_strip, tab_strip->active_index(), extension()))));
 }
 
-bool TabsGetAllInWindowFunction::RunSync() {
+ExtensionFunction::ResponseAction TabsGetAllInWindowFunction::Run() {
   std::unique_ptr<tabs::GetAllInWindow::Params> params(
       tabs::GetAllInWindow::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
@@ -835,15 +816,15 @@ bool TabsGetAllInWindowFunction::RunSync() {
     window_id = *params->window_id;
 
   Browser* browser = NULL;
-  if (!GetBrowserFromWindowID(this, window_id, &browser, &error_))
-    return false;
-
-  SetResult(ExtensionTabUtil::CreateTabList(browser, extension()));
+  std::string error;
+  if (!GetBrowserFromWindowID(this, window_id, &browser, &error))
+    return RespondNow(Error(error));
 
-  return true;
+  return RespondNow(
+      OneArgument(ExtensionTabUtil::CreateTabList(browser, extension())));
 }
 
-bool TabsQueryFunction::RunSync() {
+ExtensionFunction::ResponseAction TabsQueryFunction::Run() {
   std::unique_ptr<tabs::Query::Params> params(
       tabs::Query::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
@@ -861,9 +842,10 @@ bool TabsQueryFunction::RunSync() {
     // It is o.k. to use URLPattern::SCHEME_ALL here because this function does
     // not grant access to the content of the tabs, only to seeing their URLs
     // and meta data.
+    std::string error;
     if (!url_patterns.Populate(url_pattern_strings, URLPattern::SCHEME_ALL,
-                               true, &error_)) {
-      return false;
+                               true, &error)) {
+      return RespondNow(Error(error));
     }
   }
 
@@ -884,17 +866,19 @@ bool TabsQueryFunction::RunSync() {
     window_type = tabs::ToString(params->query_info.window_type);
 
   std::unique_ptr<base::ListValue> result(new base::ListValue());
+  Profile* profile = Profile::FromBrowserContext(browser_context());
   Browser* last_active_browser =
-      chrome::FindAnyBrowser(GetProfile(), include_incognito());
-  Browser* current_browser = GetCurrentBrowser();
+      chrome::FindAnyBrowser(profile, include_incognito());
+  Browser* current_browser =
+      ChromeExtensionFunctionDetails(this).GetCurrentBrowser();
   for (auto* browser : *BrowserList::GetInstance()) {
-    if (!GetProfile()->IsSameProfile(browser->profile()))
+    if (!profile->IsSameProfile(browser->profile()))
       continue;
 
     if (!browser->window())
       continue;
 
-    if (!include_incognito() && GetProfile() != browser->profile())
+    if (!include_incognito() && profile != browser->profile())
       continue;
 
     if (!browser->extension_window_controller()->IsVisibleToExtension(
@@ -1003,11 +987,10 @@ bool TabsQueryFunction::RunSync() {
     }
   }
 
-  SetResult(std::move(result));
-  return true;
+  return RespondNow(OneArgument(std::move(result)));
 }
 
-bool TabsCreateFunction::RunSync() {
+ExtensionFunction::ResponseAction TabsCreateFunction::Run() {
   std::unique_ptr<tabs::Create::Params> params(
       tabs::Create::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
@@ -1026,19 +1009,15 @@ bool TabsCreateFunction::RunSync() {
   std::string error;
   std::unique_ptr<base::DictionaryValue> result(
       ExtensionTabUtil::OpenTab(this, options, &error));
-  if (!result) {
-    SetError(error);
-    return false;
-  }
+  if (!result)
+    return RespondNow(Error(error));
 
   // Return data about the newly created tab.
-  if (has_callback()) {
-    SetResult(std::move(result));
-  }
-  return true;
+  return RespondNow(has_callback() ? OneArgument(std::move(result))
+                                   : NoArguments());
 }
 
-bool TabsDuplicateFunction::RunSync() {
+ExtensionFunction::ResponseAction TabsDuplicateFunction::Run() {
   std::unique_ptr<tabs::Duplicate::Params> params(
       tabs::Duplicate::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
@@ -1047,20 +1026,15 @@ bool TabsDuplicateFunction::RunSync() {
   Browser* browser = NULL;
   TabStripModel* tab_strip = NULL;
   int tab_index = -1;
-  if (!GetTabById(tab_id,
-                  GetProfile(),
-                  include_incognito(),
-                  &browser,
-                  &tab_strip,
-                  NULL,
-                  &tab_index,
-                  &error_)) {
-    return false;
+  std::string error;
+  if (!GetTabById(tab_id, browser_context(), include_incognito(), &browser,
+                  &tab_strip, NULL, &tab_index, &error)) {
+    return RespondNow(Error(error));
   }
 
   WebContents* new_contents = chrome::DuplicateTabAt(browser, tab_index);
   if (!has_callback())
-    return true;
+    return RespondNow(NoArguments());
 
   // Duplicated tab may not be in the same window as the original, so find
   // the window and the tab.
@@ -1070,17 +1044,15 @@ bool TabsDuplicateFunction::RunSync() {
                                      &new_tab_strip,
                                      &new_tab_index);
   if (!new_tab_strip || new_tab_index == -1) {
-    return false;
+    return RespondNow(Error(kUnknownErrorDoNotUse));

[asargent_no_longer_on_chrome, 2016/10/06 19:50:04]

are you sure you wanted to introduce a new usage of this apparently-forbidden
error code? 

[Devlin, 2016/10/06 23:21:17]

Thanks for asking (that means it worked)! :)

  // A string used in the case of an unknown error being detected.
  // DON'T USE THIS. It's only here during conversion to flag cases where errors
  // aren't already set.
  // TODO(devlin): Remove this if/when all functions are updated to return real
  // errors.
  static const char* kUnknownErrorDoNotUse;

But that's exactly what this one is for.

   }
 
-  // Return data about the newly created tab.
-  results_ = tabs::Get::Results::Create(*ExtensionTabUtil::CreateTabObject(
-      new_contents, new_tab_strip, new_tab_index, extension()));
-
-  return true;
+  return RespondNow(ArgumentList(
+      tabs::Get::Results::Create(*ExtensionTabUtil::CreateTabObject(
+          new_contents, new_tab_strip, new_tab_index, extension()))));
 }
 
-bool TabsGetFunction::RunSync() {
+ExtensionFunction::ResponseAction TabsGetFunction::Run() {
   std::unique_ptr<tabs::Get::Params> params(tabs::Get::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
   int tab_id = params->tab_id;
@@ -1088,35 +1060,32 @@ bool TabsGetFunction::RunSync() {
   TabStripModel* tab_strip = NULL;
   WebContents* contents = NULL;
   int tab_index = -1;
-  if (!GetTabById(tab_id,
-                  GetProfile(),
-                  include_incognito(),
-                  NULL,
-                  &tab_strip,
-                  &contents,
-                  &tab_index,
-                  &error_))
-    return false;
+  std::string error;
+  if (!GetTabById(tab_id, browser_context(), include_incognito(), NULL,
+                  &tab_strip, &contents, &tab_index, &error)) {
+    return RespondNow(Error(error));
+  }
 
-  results_ = tabs::Get::Results::Create(*ExtensionTabUtil::CreateTabObject(
-      contents, tab_strip, tab_index, extension()));
-  return true;
+  return RespondNow(ArgumentList(
+      tabs::Get::Results::Create(*ExtensionTabUtil::CreateTabObject(
+          contents, tab_strip, tab_index, extension()))));
 }
 
-bool TabsGetCurrentFunction::RunSync() {
+ExtensionFunction::ResponseAction TabsGetCurrentFunction::Run() {
   DCHECK(dispatcher());
 
   // Return the caller, if it's a tab. If not the result isn't an error but an
   // empty tab (hence returning true).
   WebContents* caller_contents = GetSenderWebContents();
-  if (caller_contents && ExtensionTabUtil::GetTabId(caller_contents) >= 0)
-    results_ = tabs::Get::Results::Create(
+  std::unique_ptr<base::ListValue> results;
+  if (caller_contents && ExtensionTabUtil::GetTabId(caller_contents) >= 0) {
+    results = tabs::Get::Results::Create(
         *ExtensionTabUtil::CreateTabObject(caller_contents, extension()));
-
-  return true;
+  }
+  return RespondNow(results ? ArgumentList(std::move(results)) : NoArguments());
 }
 
-bool TabsHighlightFunction::RunSync() {
+ExtensionFunction::ResponseAction TabsHighlightFunction::Run() {
   std::unique_ptr<tabs::Highlight::Params> params(
       tabs::Highlight::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
@@ -1127,8 +1096,9 @@ bool TabsHighlightFunction::RunSync() {
     window_id = *params->highlight_info.window_id;
 
   Browser* browser = NULL;
-  if (!GetBrowserFromWindowID(this, window_id, &browser, &error_))
-    return false;
+  std::string error;
+  if (!GetBrowserFromWindowID(this, window_id, &browser, &error))
+    return RespondNow(Error(error));
 
   TabStripModel* tabstrip = browser->tab_strip_model();
   ui::ListSelectionModel selection;
@@ -1138,40 +1108,39 @@ bool TabsHighlightFunction::RunSync() {
     std::vector<int>& tab_indices = *params->highlight_info.tabs.as_integers;
     // Create a new selection model as we read the list of tab indices.
     for (size_t i = 0; i < tab_indices.size(); ++i) {
-      if (!HighlightTab(tabstrip, &selection, &active_index, tab_indices[i]))
-        return false;
+      if (!HighlightTab(tabstrip, &selection, &active_index, tab_indices[i],
+                        &error)) {
+        return RespondNow(Error(error));
+      }
     }
   } else {
     EXTENSION_FUNCTION_VALIDATE(params->highlight_info.tabs.as_integer);
-    if (!HighlightTab(tabstrip,
-                      &selection,
-                      &active_index,
-                      *params->highlight_info.tabs.as_integer)) {
-      return false;
+    if (!HighlightTab(tabstrip, &selection, &active_index,
+                      *params->highlight_info.tabs.as_integer, &error)) {
+      return RespondNow(Error(error));
     }
   }
 
   // Make sure they actually specified tabs to select.
-  if (selection.empty()) {
-    error_ = keys::kNoHighlightedTabError;
-    return false;
-  }
+  if (selection.empty())
+    return RespondNow(Error(keys::kNoHighlightedTabError));
 
   selection.set_active(active_index);
   browser->tab_strip_model()->SetSelectionFromModel(selection);
-  SetResult(browser->extension_window_controller()->CreateWindowValueWithTabs(
-      extension()));
-  return true;
+  return RespondNow(OneArgument(
+      browser->extension_window_controller()->CreateWindowValueWithTabs(
+          extension())));
 }
 
 bool TabsHighlightFunction::HighlightTab(TabStripModel* tabstrip,
                                          ui::ListSelectionModel* selection,
                                          int* active_index,
-                                         int index) {
+                                         int index,
+                                         std::string* error) {
   // Make sure the index is in range.
   if (!tabstrip->ContainsIndex(index)) {
-    error_ = ErrorUtils::FormatErrorMessage(
-        keys::kTabIndexNotFoundError, base::IntToString(index));
+    *error = ErrorUtils::FormatErrorMessage(keys::kTabIndexNotFoundError,
+                                            base::IntToString(index));
     return false;
   }
 
@@ -1211,14 +1180,8 @@ bool TabsUpdateFunction::RunAsync() {
 
   int tab_index = -1;
   TabStripModel* tab_strip = NULL;
-  if (!GetTabById(tab_id,
-                  GetProfile(),
-                  include_incognito(),
-                  NULL,
-                  &tab_strip,
-                  &contents,
-                  &tab_index,
-                  &error_)) {
+  if (!GetTabById(tab_id, browser_context(), include_incognito(), NULL,
+                  &tab_strip, &contents, &tab_index, &error_)) {
     return false;
   }
 
@@ -1290,13 +1253,9 @@ bool TabsUpdateFunction::RunAsync() {
     int opener_id = *params->update_properties.opener_tab_id;
 
     WebContents* opener_contents = NULL;
-    if (!ExtensionTabUtil::GetTabById(opener_id,
-                                      GetProfile(),
-                                      include_incognito(),
-                                      NULL,
-                                      NULL,
-                                      &opener_contents,
-                                      NULL))
+    if (!ExtensionTabUtil::GetTabById(opener_id, browser_context(),
+                                      include_incognito(), NULL, NULL,
+                                      &opener_contents, NULL))

[asargent_no_longer_on_chrome, 2016/10/06 19:50:04]

might as well s/NULL/nullptr/ while you're touching these lines?

[Devlin, 2016/10/06 23:21:18]

Done.

       return false;
 
     tab_strip->SetOpenerOfWebContentsAt(tab_index, opener_contents);
@@ -1394,7 +1353,7 @@ void TabsUpdateFunction::OnExecuteCodeFinished(
   SendResponse(error.empty());
 }
 
-bool TabsMoveFunction::RunSync() {
+ExtensionFunction::ResponseAction TabsMoveFunction::Run() {
   std::unique_ptr<tabs::Move::Params> params(
       tabs::Move::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
@@ -1404,66 +1363,61 @@ bool TabsMoveFunction::RunSync() {
   std::unique_ptr<base::ListValue> tab_values(new base::ListValue());
 
   size_t num_tabs = 0;
+  std::string error;
   if (params->tab_ids.as_integers) {
     std::vector<int>& tab_ids = *params->tab_ids.as_integers;
     num_tabs = tab_ids.size();
     for (size_t i = 0; i < tab_ids.size(); ++i) {
-      if (!MoveTab(tab_ids[i], &new_index, i, tab_values.get(), window_id))
-        return false;
+      if (!MoveTab(tab_ids[i], &new_index, i, tab_values.get(), window_id,
+                   &error)) {
+        return RespondNow(Error(error));
+      }
     }
   } else {
     EXTENSION_FUNCTION_VALIDATE(params->tab_ids.as_integer);
     num_tabs = 1;
-    if (!MoveTab(*params->tab_ids.as_integer,
-                 &new_index,
-                 0,
-                 tab_values.get(),
-                 window_id)) {
-      return false;
+    if (!MoveTab(*params->tab_ids.as_integer, &new_index, 0, tab_values.get(),
+                 window_id, &error)) {
+      return RespondNow(Error(error));
     }
   }
 
+  // TODO(devlin): It's weird that whether or not the method provides a callback
+  // can determine its success (as we return errors below).

[asargent_no_longer_on_chrome, 2016/10/06 19:50:04]

If a tree falls in a forest and no one is around to hear it, does it make a
sound?

[Devlin, 2016/10/06 23:21:17]

Don't quite follow.  You mean if we set the last error and there's no callback,
does it matter?  It actually does, because we throw unchecked lastErrors (even
if there's no callback).

[asargent_no_longer_on_chrome, 2016/10/06 23:28:11]

This was an attempt at humor - your comment just reminded me of the classic
philosophical question. 

(I.e. if an extension function invocation fails and there's no callback to
notice the failure... sounds pretty similar to "if a tree falls and there's no
one to hear it")

   if (!has_callback())
-    return true;
+    return RespondNow(NoArguments());
 
-  if (num_tabs == 0) {
-    error_ = "No tabs given.";
-    return false;
-  } else if (num_tabs == 1) {
+  if (num_tabs == 0)
+    return RespondNow(Error("No tabs given."));
+  if (num_tabs == 1) {
     std::unique_ptr<base::Value> value;
     CHECK(tab_values->Remove(0, &value));
-    SetResult(std::move(value));
-  } else {
-    // Only return the results as an array if there are multiple tabs.
-    SetResult(std::move(tab_values));
+    return RespondNow(OneArgument(std::move(value)));
   }
 
-  return true;
+  // Return the results as an array if there are multiple tabs.
+  return RespondNow(OneArgument(std::move(tab_values)));
 }
 
 bool TabsMoveFunction::MoveTab(int tab_id,
                                int* new_index,
                                int iteration,
                                base::ListValue* tab_values,
-                               int* window_id) {
+                               int* window_id,
+                               std::string* error) {
   Browser* source_browser = NULL;
   TabStripModel* source_tab_strip = NULL;
   WebContents* contents = NULL;
   int tab_index = -1;
-  if (!GetTabById(tab_id,
-                  GetProfile(),
-                  include_incognito(),
-                  &source_browser,
-                  &source_tab_strip,
-                  &contents,
-                  &tab_index,
-                  &error_)) {
+  if (!GetTabById(tab_id, browser_context(), include_incognito(),
+                  &source_browser, &source_tab_strip, &contents, &tab_index,
+                  error)) {
     return false;
   }
 
   // Don't let the extension move the tab if the user is dragging tabs.
   if (!source_browser->window()->IsTabStripEditable()) {
-    error_ = keys::kTabStripNotEditableError;
+    *error = keys::kTabStripNotEditableError;
     return false;
   }
 
@@ -1473,21 +1427,21 @@ bool TabsMoveFunction::MoveTab(int tab_id,
   if (window_id) {
     Browser* target_browser = NULL;
 
-    if (!GetBrowserFromWindowID(this, *window_id, &target_browser, &error_))
+    if (!GetBrowserFromWindowID(this, *window_id, &target_browser, error))
       return false;
 
     if (!target_browser->window()->IsTabStripEditable()) {
-      error_ = keys::kTabStripNotEditableError;
+      *error = keys::kTabStripNotEditableError;
       return false;
     }
 
     if (!target_browser->is_type_tabbed()) {
-      error_ = keys::kCanOnlyMoveTabsWithinNormalWindowsError;
+      *error = keys::kCanOnlyMoveTabsWithinNormalWindowsError;
       return false;
     }
 
     if (target_browser->profile() != source_browser->profile()) {
-      error_ = keys::kCanOnlyMoveTabsWithinSameProfileError;
+      *error = keys::kCanOnlyMoveTabsWithinSameProfileError;
       return false;
     }
 
@@ -1498,8 +1452,8 @@ bool TabsMoveFunction::MoveTab(int tab_id,
       WebContents* web_contents =
           source_tab_strip->DetachWebContentsAt(tab_index);
       if (!web_contents) {
-        error_ = ErrorUtils::FormatErrorMessage(
-            keys::kTabNotFoundError, base::IntToString(tab_id));
+        *error = ErrorUtils::FormatErrorMessage(keys::kTabNotFoundError,
+                                                base::IntToString(tab_id));
         return false;
       }
 
@@ -1542,7 +1496,7 @@ bool TabsMoveFunction::MoveTab(int tab_id,
   return true;
 }
 
-bool TabsReloadFunction::RunSync() {
+ExtensionFunction::ResponseAction TabsReloadFunction::Run() {
   std::unique_ptr<tabs::Reload::Params> params(
       tabs::Reload::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
@@ -1557,28 +1511,22 @@ bool TabsReloadFunction::RunSync() {
 
   // If |tab_id| is specified, look for it. Otherwise default to selected tab
   // in the current window.
+  Browser* current_browser =
+      ChromeExtensionFunctionDetails(this).GetCurrentBrowser();
   if (!params->tab_id.get()) {
-    Browser* browser = GetCurrentBrowser();
-    if (!browser) {
-      error_ = keys::kNoCurrentWindowError;
-      return false;
-    }
+    if (!current_browser)
+      return RespondNow(Error(keys::kNoCurrentWindowError));
 
-    if (!ExtensionTabUtil::GetDefaultTab(browser, &web_contents, NULL))
-      return false;
+    if (!ExtensionTabUtil::GetDefaultTab(current_browser, &web_contents, NULL))
+      return RespondNow(Error(kUnknownErrorDoNotUse));
   } else {
     int tab_id = *params->tab_id;
 
     Browser* browser = NULL;
-    if (!GetTabById(tab_id,
-                    GetProfile(),
-                    include_incognito(),
-                    &browser,
-                    NULL,
-                    &web_contents,
-                    NULL,
-                    &error_)) {
-      return false;
+    std::string error;
+    if (!GetTabById(tab_id, browser_context(), include_incognito(), &browser,
+                    NULL, &web_contents, NULL, &error)) {
+      return RespondNow(Error(error));
     }
   }
 
@@ -1589,52 +1537,47 @@ bool TabsReloadFunction::RunSync() {
     OpenURLParams params(reload_url, Referrer(),
                          WindowOpenDisposition::CURRENT_TAB,
                          ui::PAGE_TRANSITION_RELOAD, false);
-    GetCurrentBrowser()->OpenURL(params);
+    current_browser->OpenURL(params);
   } else if (bypass_cache) {
     web_contents->GetController().ReloadBypassingCache(true);
   } else {
     web_contents->GetController().Reload(true);
   }
 
-  return true;
+  return RespondNow(NoArguments());
 }
 
-bool TabsRemoveFunction::RunSync() {
+ExtensionFunction::ResponseAction TabsRemoveFunction::Run() {
   std::unique_ptr<tabs::Remove::Params> params(
       tabs::Remove::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
 
+  std::string error;
   if (params->tab_ids.as_integers) {
     std::vector<int>& tab_ids = *params->tab_ids.as_integers;
     for (size_t i = 0; i < tab_ids.size(); ++i) {
-      if (!RemoveTab(tab_ids[i]))
-        return false;
+      if (!RemoveTab(tab_ids[i], &error))
+        return RespondNow(Error(error));
     }
   } else {
     EXTENSION_FUNCTION_VALIDATE(params->tab_ids.as_integer);
-    if (!RemoveTab(*params->tab_ids.as_integer))
-      return false;
+    if (!RemoveTab(*params->tab_ids.as_integer, &error))
+      return RespondNow(Error(error));
   }
-  return true;
+  return RespondNow(NoArguments());
 }
 
-bool TabsRemoveFunction::RemoveTab(int tab_id) {
+bool TabsRemoveFunction::RemoveTab(int tab_id, std::string* error) {
   Browser* browser = NULL;
   WebContents* contents = NULL;
-  if (!GetTabById(tab_id,
-                  GetProfile(),
-                  include_incognito(),
-                  &browser,
-                  NULL,
-                  &contents,
-                  NULL,
-                  &error_)) {
+  if (!GetTabById(tab_id, browser_context(), include_incognito(), &browser,
+                  NULL, &contents, NULL, error)) {

[asargent_no_longer_on_chrome, 2016/10/06 19:50:04]

NULL->nullptr

[Devlin, 2016/10/06 23:21:18]

Done.

     return false;
   }
 
   // Don't let the extension remove a tab if the user is dragging tabs around.
   if (!browser->window()->IsTabStripEditable()) {
-    error_ = keys::kTabStripNotEditableError;
+    *error = keys::kTabStripNotEditableError;
     return false;
   }
   // There's a chance that the tab is being dragged, or we're in some other
@@ -1754,14 +1697,8 @@ bool TabsDetectLanguageFunction::RunAsync() {
   // in the current window.
   if (params->tab_id.get()) {
     tab_id = *params->tab_id;
-    if (!GetTabById(tab_id,
-                    GetProfile(),
-                    include_incognito(),
-                    &browser,
-                    NULL,
-                    &contents,
-                    NULL,
-                    &error_)) {
+    if (!GetTabById(tab_id, browser_context(), include_incognito(), &browser,
+                    NULL, &contents, NULL, &error_)) {

[asargent_no_longer_on_chrome, 2016/10/06 19:50:04]

NULL->nullptr

[Devlin, 2016/10/06 23:21:17]

Done.

       return false;
     }
     if (!browser || !contents)
@@ -1891,14 +1828,8 @@ bool ExecuteCodeInTabFunction::CanExecuteScriptOnPage() {
   // If |tab_id| is specified, look for the tab. Otherwise default to selected
   // tab in the current window.
   CHECK_GE(execute_tab_id_, 0);
-  if (!GetTabById(execute_tab_id_,
-                  chrome_details_.GetProfile(),
-                  include_incognito(),
-                  NULL,
-                  NULL,
-                  &contents,
-                  NULL,
-                  &error_)) {
+  if (!GetTabById(execute_tab_id_, browser_context(), include_incognito(), NULL,
+                  NULL, &contents, NULL, &error_)) {

[asargent_no_longer_on_chrome, 2016/10/06 19:50:04]

NULL->nullptr

[Devlin, 2016/10/06 23:21:18]

Done.

     return false;
   }
 
@@ -1953,15 +1884,10 @@ ScriptExecutor* ExecuteCodeInTabFunction::GetScriptExecutor() {
   Browser* browser = NULL;
   content::WebContents* contents = NULL;
 
-  bool success = GetTabById(execute_tab_id_,
-                            chrome_details_.GetProfile(),
-                            include_incognito(),
-                            &browser,
-                            NULL,
-                            &contents,
-                            NULL,
-                            &error_) &&
-                 contents && browser;
+  bool success =
+      GetTabById(execute_tab_id_, browser_context(), include_incognito(),
+                 &browser, NULL, &contents, NULL, &error_) &&

[asargent_no_longer_on_chrome, 2016/10/06 19:50:04]

NULL->nullptr

[Devlin, 2016/10/06 23:21:17]

Done.

+      contents && browser;
 
   if (!success)
     return NULL;
@@ -1998,14 +1924,10 @@ content::WebContents* ZoomAPIFunction::GetWebContents(int tab_id) {
   content::WebContents* web_contents = NULL;
   if (tab_id != -1) {
     // We assume this call leaves web_contents unchanged if it is unsuccessful.
-    GetTabById(tab_id,
-               GetProfile(),
-               include_incognito(),
+    GetTabById(tab_id, browser_context(), include_incognito(),
                NULL /* ignore Browser* output */,
-               NULL /* ignore TabStripModel* output */,
-               &web_contents,
-               NULL /* ignore int tab_index output */,
-               &error_);
+               NULL /* ignore TabStripModel* output */, &web_contents,
+               NULL /* ignore int tab_index output */, &error_);

[asargent_no_longer_on_chrome, 2016/10/06 19:50:04]

NULL->nullptr

[Devlin, 2016/10/06 23:21:18]

Done.

   } else {
     Browser* browser = GetCurrentBrowser();
     if (!browser)
@@ -2147,15 +2069,14 @@ ExtensionFunction::ResponseAction TabsDiscardFunction::Run() {
   EXTENSION_FUNCTION_VALIDATE(params);
 
   WebContents* contents = nullptr;
-  Profile* profile = Profile::FromBrowserContext(browser_context());
   // If |tab_id| is given, find the web_contents respective to it.
   // Otherwise invoke discard function in TabManager with null web_contents
   // that will discard the least important tab.
   if (params->tab_id) {
     int tab_id = *params->tab_id;
     std::string error;
-    if (!GetTabById(tab_id, profile, include_incognito(), nullptr, nullptr,
-                    &contents, nullptr, &error)) {
+    if (!GetTabById(tab_id, browser_context(), include_incognito(), nullptr,
+                    nullptr, &contents, nullptr, &error)) {
       return RespondNow(Error(error));
     }
   }