[Extensions] Turn tab capture functions to UIThreadExtensionFunction.

tabs api and webview have APIs for capturing tab contents, turn
them to UIThreadExtensionFunction from AsyncExtensionFunction.

AsyncExtensionFunction is deprecated and will be removed soon.

Bug: 829174
Change-Id: I9cc390cc00903eab62b2e0b5ac42394333f243ec
Reviewed-on: https://chromium-review.googlesource.com/997106
Commit-Queue: Istiaque Ahmed <lazyboy@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548633}

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

@@ -1725,25 +1725,27 @@ bool TabsCaptureVisibleTabFunction::ClientAllowsTransparency() {
   return false;
 }
 
-WebContents* TabsCaptureVisibleTabFunction::GetWebContentsForID(int window_id) {
+WebContents* TabsCaptureVisibleTabFunction::GetWebContentsForID(
+    int window_id,
+    std::string* error) {
   Browser* browser = NULL;
-  if (!GetBrowserFromWindowID(chrome_details_, window_id, &browser, &error_))
-    return NULL;
+  if (!GetBrowserFromWindowID(chrome_details_, window_id, &browser, error))
+    return nullptr;
 
   WebContents* contents = browser->tab_strip_model()->GetActiveWebContents();
   if (!contents) {
-    error_ = "No active web contents to capture";
-    return NULL;
+    *error = "No active web contents to capture";
+    return nullptr;
   }
 
   if (!extension()->permissions_data()->CanCaptureVisiblePage(
-          SessionTabHelper::IdForTab(contents).id(), &error_)) {
-    return NULL;
+          SessionTabHelper::IdForTab(contents).id(), error)) {
+    return nullptr;
   }
   return contents;
 }
 
-bool TabsCaptureVisibleTabFunction::RunAsync() {
+ExtensionFunction::ResponseAction TabsCaptureVisibleTabFunction::Run() {
   using api::extension_types::ImageDetails;
 
   EXTENSION_FUNCTION_VALIDATE(args_);
@@ -1758,16 +1760,22 @@ bool TabsCaptureVisibleTabFunction::RunAsync() {
     image_details = ImageDetails::FromValue(*spec);
   }
 
-  WebContents* contents = GetWebContentsForID(context_id);
+  std::string error;
+  WebContents* contents = GetWebContentsForID(context_id, &error);
+  // TODO(wjmaclean): If |error| was populated, shouldn't we send error
+  // response? Currently doing that will fail
+  // ExtensionApiCaptureTest.CaptureNullWindow test.
 
   const CaptureResult capture_result = CaptureAsync(
       contents, image_details.get(),
       base::BindOnce(&TabsCaptureVisibleTabFunction::CopyFromSurfaceComplete,
                      this));
-  if (capture_result == OK)
-    return true;
-  SetErrorMessage(capture_result);
-  return false;
+  if (capture_result == OK) {
+    // CopyFromSurfaceComplete might have already responded.
+    return did_respond() ? AlreadyResponded() : RespondLater();
+  }
+
+  return RespondNow(Error(CaptureResultToErrorMessage(capture_result)));
 }
 
 void TabsCaptureVisibleTabFunction::OnCaptureSuccess(const SkBitmap& bitmap) {
@@ -1777,16 +1785,16 @@ void TabsCaptureVisibleTabFunction::OnCaptureSuccess(const SkBitmap& bitmap) {
     return;
   }
 
-  SetResult(std::make_unique<base::Value>(base64_result));
-  SendResponse(true);
+  Respond(OneArgument(std::make_unique<base::Value>(base64_result)));
 }
 
 void TabsCaptureVisibleTabFunction::OnCaptureFailure(CaptureResult result) {
-  SetErrorMessage(result);
-  SendResponse(false);
+  Respond(Error(CaptureResultToErrorMessage(result)));
 }
 
-void TabsCaptureVisibleTabFunction::SetErrorMessage(CaptureResult result) {
+// static.
+std::string TabsCaptureVisibleTabFunction::CaptureResultToErrorMessage(
+    CaptureResult result) {
   const char* reason_description = "internal error";
   switch (result) {
     case FAILURE_REASON_READBACK_FAILED:
@@ -1799,15 +1807,14 @@ void TabsCaptureVisibleTabFunction::SetErrorMessage(CaptureResult result) {
       reason_description = "view is invisible";
       break;
     case FAILURE_REASON_SCREEN_SHOTS_DISABLED:
-      error_ = keys::kScreenshotsDisabled;
-      return;
+      return keys::kScreenshotsDisabled;
     case OK:
-      NOTREACHED()
-          << "SetErrorMessage should not be called with a successful result";
-      return;
+      NOTREACHED() << "CaptureResultToErrorMessage should not be called"
+                      " with a successful result";
+      return kUnknownErrorDoNotUse;
   }
-  error_ = ErrorUtils::FormatErrorMessage("Failed to capture tab: *",
-                                          reason_description);
+  return ErrorUtils::FormatErrorMessage("Failed to capture tab: *",
+                                        reason_description);
 }
 
 void TabsCaptureVisibleTabFunction::RegisterProfilePrefs(

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

@@ -197,14 +197,14 @@ class TabsDetectLanguageFunction : public ChromeAsyncExtensionFunction,
 
 class TabsCaptureVisibleTabFunction
     : public extensions::WebContentsCaptureClient,
-      public AsyncExtensionFunction {
+      public UIThreadExtensionFunction {
  public:
   TabsCaptureVisibleTabFunction();
   static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
 
   // ExtensionFunction implementation.
   bool HasPermission() override;
-  bool RunAsync() override;
+  ResponseAction Run() override;
 
  protected:
   ~TabsCaptureVisibleTabFunction() override {}
@@ -212,7 +212,7 @@ class TabsCaptureVisibleTabFunction
  private:
   ChromeExtensionFunctionDetails chrome_details_;
 
-  content::WebContents* GetWebContentsForID(int window_id);
+  content::WebContents* GetWebContentsForID(int window_id, std::string* error);
 
   // extensions::WebContentsCaptureClient:
   bool IsScreenshotEnabled() const override;
@@ -221,9 +221,11 @@ class TabsCaptureVisibleTabFunction
   void OnCaptureFailure(CaptureResult result) override;
 
  private:
-  void SetErrorMessage(CaptureResult result);
-
   DECLARE_EXTENSION_FUNCTION("tabs.captureVisibleTab", TABS_CAPTUREVISIBLETAB)
+
+  static std::string CaptureResultToErrorMessage(CaptureResult result);
+
+  DISALLOW_COPY_AND_ASSIGN(TabsCaptureVisibleTabFunction);
 };
 
 // Implement API call tabs.executeScript and tabs.insertCSS.

extensions/browser/api/guest_view/web_view/web_view_internal_api.cc

@@ -299,8 +299,8 @@ WebViewInternalCaptureVisibleRegionFunction::
     WebViewInternalCaptureVisibleRegionFunction()
     : is_guest_transparent_(false) {}
 
-bool WebViewInternalCaptureVisibleRegionFunction::RunAsyncSafe(
-    WebViewGuest* guest) {
+ExtensionFunction::ResponseAction
+WebViewInternalCaptureVisibleRegionFunction::Run() {
   using api::extension_types::ImageDetails;
 
   std::unique_ptr<web_view_internal::CaptureVisibleRegion::Params> params(
@@ -314,16 +314,18 @@ bool WebViewInternalCaptureVisibleRegionFunction::RunAsyncSafe(
     image_details = ImageDetails::FromValue(*spec);
   }
 
-  is_guest_transparent_ = guest->allow_transparency();
+  is_guest_transparent_ = guest_->allow_transparency();
   const CaptureResult capture_result = CaptureAsync(
-      guest->web_contents(), image_details.get(),
+      guest_->web_contents(), image_details.get(),
       base::BindOnce(
           &WebViewInternalCaptureVisibleRegionFunction::CopyFromSurfaceComplete,
           this));
-  if (capture_result == OK)
-    return true;
-  SetErrorMessage(capture_result);
-  return false;
+  if (capture_result == OK) {
+    // CaptureAsync may have responded synchronously.
+    return did_respond() ? AlreadyResponded() : RespondLater();
+  }
+
+  return RespondNow(Error(GetErrorMessage(capture_result)));
 }
 bool WebViewInternalCaptureVisibleRegionFunction::IsScreenshotEnabled() const {
   // TODO(wjmaclean): Is it ok to always return true here?
@@ -342,17 +344,15 @@ void WebViewInternalCaptureVisibleRegionFunction::OnCaptureSuccess(
     return;
   }
 
-  SetResult(std::make_unique<base::Value>(base64_result));
-  SendResponse(true);
+  Respond(OneArgument(std::make_unique<base::Value>(base64_result)));
 }
 
 void WebViewInternalCaptureVisibleRegionFunction::OnCaptureFailure(
     CaptureResult result) {
-  SetErrorMessage(result);
-  SendResponse(false);
+  Respond(Error(GetErrorMessage(result)));
 }
 
-void WebViewInternalCaptureVisibleRegionFunction::SetErrorMessage(
+std::string WebViewInternalCaptureVisibleRegionFunction::GetErrorMessage(
     CaptureResult result) {
   const char* reason_description = "internal error";
   switch (result) {
@@ -371,11 +371,11 @@ void WebViewInternalCaptureVisibleRegionFunction::SetErrorMessage(
       break;
     case OK:
       NOTREACHED()
-          << "SetErrorMessage should not be called with a successful result";
-      return;
+          << "GetErrorMessage should not be called with a successful result";
+      return "";
   }
-  error_ = ErrorUtils::FormatErrorMessage("Failed to capture webview: *",
-                                          reason_description);
+  return ErrorUtils::FormatErrorMessage("Failed to capture webview: *",
+                                        reason_description);
 }
 
 ExtensionFunction::ResponseAction WebViewInternalNavigateFunction::Run() {

extensions/browser/api/guest_view/web_view/web_view_internal_api.h

@@ -49,7 +49,7 @@ class WebViewInternalExtensionFunction : public UIThreadExtensionFunction {
 };
 
 class WebViewInternalCaptureVisibleRegionFunction
-    : public LegacyWebViewInternalExtensionFunction,
+    : public WebViewInternalExtensionFunction,
       public WebContentsCaptureClient {
  public:
   DECLARE_EXTENSION_FUNCTION("webViewInternal.captureVisibleRegion",
@@ -59,17 +59,17 @@ class WebViewInternalCaptureVisibleRegionFunction
  protected:
   ~WebViewInternalCaptureVisibleRegionFunction() override {}
 
- private:
-  // LegacyWebViewInternalExtensionFunction implementation.
-  bool RunAsyncSafe(WebViewGuest* guest) override;
+  // UIThreadExtensionFunction:
+  ResponseAction Run() override;
 
+ private:
   // extensions::WebContentsCaptureClient:
   bool IsScreenshotEnabled() const override;
   bool ClientAllowsTransparency() override;
   void OnCaptureSuccess(const SkBitmap& bitmap) override;
   void OnCaptureFailure(CaptureResult result) override;
 
-  void SetErrorMessage(CaptureResult result);
+  std::string GetErrorMessage(CaptureResult result);
 
   bool is_guest_transparent_;