Implement webview.captureVisibleRegion()

This CL implements  webview.captureVisibleRegion(), an extension/apps
API to allow WebView users to capture screenshots of the contents
displayed in a WebView. The surfaces contents capture has been plumbed
via RenderWidgetHostViewChildFrame so this implementation should not
require changes when WebView switches to using OOPIF.

As part of the implementation, there are two notable refactors:

1) CaptureWebContentsFunction has been refactored into
WebContentsCaptureClient to remove the extensions::AsyncExtensionFunction
dependence so that this code can be used by both tabs.captureVisibleTab
and webview.captureVisibleRegion, and

2) common code from DelegatedFrameHost has ben moved to
content/browser/compositor/surface_utils.* in order to avoid duplication
as both DelegatedFrameHost and RenderWidgetHostViewChildFrame now
use the code.

Finally, this CL adds a surface-drawn callback to
RenderWidgetHostViewChildFrame, to allow deferring a
screen capture request until a frame has actually been
drawn. This callback can be used to simplify some
existing tests.

BUG=326755
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Review URL: https://codereview.chromium.org/1635513003

Cr-Commit-Position: refs/heads/master@{#372799}

5 files changed, 133 insertions, 70 deletions

diff --git a/extensions/browser/api/guest_view/extension_view/extension_view_internal_api.h b/extensions/browser/api/guest_view/extension_view/extension_view_internal_api.h
index 9e77cbd..1899d42 100644
--- a/extensions/browser/api/guest_view/extension_view/extension_view_internal_api.h
+++ b/extensions/browser/api/guest_view/extension_view/extension_view_internal_api.h
@@ -6,7 +6,6 @@
 #define EXTENSIONS_BROWSER_API_EXTENSION_VIEW_EXTENSION_VIEW_INTERNAL_API_H_
 
 #include "base/macros.h"
-#include "extensions/browser/api/capture_web_contents_function.h"
 #include "extensions/browser/api/execute_code_function.h"
 #include "extensions/browser/extension_function.h"
 #include "extensions/browser/guest_view/extension_view/extension_view_guest.h"
diff --git a/extensions/browser/api/guest_view/web_view/web_view_internal_api.cc b/extensions/browser/api/guest_view/web_view/web_view_internal_api.cc
index bba531d..3e3a8ed 100644
--- a/extensions/browser/api/guest_view/web_view/web_view_internal_api.cc
+++ b/extensions/browser/api/guest_view/web_view/web_view_internal_api.cc
@@ -259,6 +259,62 @@ bool WebViewInternalExtensionFunction::RunAsync() {
   return RunAsyncSafe(guest);
 }
 
+bool WebViewInternalCaptureVisibleRegionFunction::RunAsyncSafe(
+    WebViewGuest* guest) {
+  using api::extension_types::ImageDetails;
+
+  scoped_ptr<web_view_internal::CaptureVisibleRegion::Params> params(
+      web_view_internal::CaptureVisibleRegion::Params::Create(*args_));
+  EXTENSION_FUNCTION_VALIDATE(params.get());
+
+  scoped_ptr<ImageDetails> image_details;
+  if (args_->GetSize() > 1) {
+    base::Value* spec = NULL;
+    EXTENSION_FUNCTION_VALIDATE(args_->Get(1, &spec) && spec);
+    image_details = ImageDetails::FromValue(*spec);
+  }
+
+  return CaptureAsync(guest->web_contents(), image_details.get(),
+                      base::Bind(&WebViewInternalCaptureVisibleRegionFunction::
+                                     CopyFromBackingStoreComplete,
+                                 this));
+}
+bool WebViewInternalCaptureVisibleRegionFunction::IsScreenshotEnabled() {
+  // TODO(wjmaclean): Is it ok to always return true here?
+  return true;
+}
+
+void WebViewInternalCaptureVisibleRegionFunction::OnCaptureSuccess(
+    const SkBitmap& bitmap) {
+  std::string base64_result;
+  if (!EncodeBitmap(bitmap, &base64_result)) {
+    OnCaptureFailure(FAILURE_REASON_ENCODING_FAILED);
+    return;
+  }
+
+  SetResult(new base::StringValue(base64_result));
+  SendResponse(true);
+}
+
+void WebViewInternalCaptureVisibleRegionFunction::OnCaptureFailure(
+    FailureReason reason) {
+  const char* reason_description = "internal error";
+  switch (reason) {
+    case FAILURE_REASON_UNKNOWN:
+      reason_description = "unknown error";
+      break;
+    case FAILURE_REASON_ENCODING_FAILED:
+      reason_description = "encoding failed";
+      break;
+    case FAILURE_REASON_VIEW_INVISIBLE:
+      reason_description = "view is invisible";
+      break;
+  }
+  error_ = ErrorUtils::FormatErrorMessage("Failed to capture webview: *",
+                                          reason_description);
+  SendResponse(false);
+}
+
 bool WebViewInternalNavigateFunction::RunAsyncSafe(WebViewGuest* guest) {
   scoped_ptr<web_view_internal::Navigate::Params> params(
       web_view_internal::Navigate::Params::Create(*args_));
diff --git a/extensions/browser/api/guest_view/web_view/web_view_internal_api.h b/extensions/browser/api/guest_view/web_view/web_view_internal_api.h
index dcdb828..388491e 100644
--- a/extensions/browser/api/guest_view/web_view/web_view_internal_api.h
+++ b/extensions/browser/api/guest_view/web_view/web_view_internal_api.h
@@ -8,8 +8,8 @@
 #include <stdint.h>
 
 #include "base/macros.h"
-#include "extensions/browser/api/capture_web_contents_function.h"
 #include "extensions/browser/api/execute_code_function.h"
+#include "extensions/browser/api/web_contents_capture_client.h"
 #include "extensions/browser/extension_function.h"
 #include "extensions/browser/guest_view/web_view/web_ui/web_ui_url_fetcher.h"
 #include "extensions/browser/guest_view/web_view/web_view_guest.h"
@@ -37,6 +37,29 @@ class WebViewInternalExtensionFunction : public AsyncExtensionFunction {
   virtual bool RunAsyncSafe(WebViewGuest* guest) = 0;
 };
 
+class WebViewInternalCaptureVisibleRegionFunction
+    : public WebViewInternalExtensionFunction,
+      public WebContentsCaptureClient {
+ public:
+  DECLARE_EXTENSION_FUNCTION("webViewInternal.captureVisibleRegion",
+                             WEBVIEWINTERNAL_CAPTUREVISIBLEREGION);
+  WebViewInternalCaptureVisibleRegionFunction() {}
+
+ protected:
+  ~WebViewInternalCaptureVisibleRegionFunction() override {}
+
+ private:
+  // WebViewInternalExtensionFunction implementation.
+  bool RunAsyncSafe(WebViewGuest* guest) override;
+
+  // extensions::WebContentsCaptureClient:
+  bool IsScreenshotEnabled() override;
+  void OnCaptureSuccess(const SkBitmap& bitmap) override;
+  void OnCaptureFailure(FailureReason reason) override;
+
+  DISALLOW_COPY_AND_ASSIGN(WebViewInternalCaptureVisibleRegionFunction);
+};
+
 class WebViewInternalNavigateFunction
     : public WebViewInternalExtensionFunction {
  public:
diff --git a/extensions/browser/api/capture_web_contents_function.cc b/extensions/browser/api/web_contents_capture_client.cc
index 77c8a3e..78c3760 100644
--- a/extensions/browser/api/capture_web_contents_function.cc
+++ b/extensions/browser/api/web_contents_capture_client.cc
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "extensions/browser/api/capture_web_contents_function.h"
+#include "extensions/browser/api/web_contents_capture_client.h"
 
 #include "base/base64.h"
 #include "base/strings/stringprintf.h"
@@ -25,28 +25,14 @@ namespace extensions {
 
 using api::extension_types::ImageDetails;
 
-bool CaptureWebContentsFunction::HasPermission() {
-  return true;
-}
-
-bool CaptureWebContentsFunction::RunAsync() {
-  EXTENSION_FUNCTION_VALIDATE(args_);
-
-  context_id_ = extension_misc::kCurrentWindowId;
-  args_->GetInteger(0, &context_id_);
-
-  scoped_ptr<ImageDetails> image_details;
-  if (args_->GetSize() > 1) {
-    base::Value* spec = NULL;
-    EXTENSION_FUNCTION_VALIDATE(args_->Get(1, &spec) && spec);
-    image_details = ImageDetails::FromValue(*spec);
-  }
-
-  if (!IsScreenshotEnabled())
+bool WebContentsCaptureClient::CaptureAsync(
+    WebContents* web_contents,
+    const ImageDetails* image_details,
+    const content::ReadbackRequestCallback callback) {
+  if (!web_contents)
     return false;
 
-  WebContents* contents = GetWebContentsForID(context_id_);
-  if (!contents)
+  if (!IsScreenshotEnabled())
     return false;
 
   // The default format and quality setting used when encoding jpegs.
@@ -65,7 +51,7 @@ bool CaptureWebContentsFunction::RunAsync() {
   }
 
   // TODO(miu): Account for fullscreen render widget?  http://crbug.com/419878
-  RenderWidgetHostView* const view = contents->GetRenderWidgetHostView();
+  RenderWidgetHostView* const view = web_contents->GetRenderWidgetHostView();
   RenderWidgetHost* const host = view ? view->GetRenderWidgetHost() : nullptr;
   if (!view || !host) {
     OnCaptureFailure(FAILURE_REASON_VIEW_INVISIBLE);
@@ -84,26 +70,41 @@ bool CaptureWebContentsFunction::RunAsync() {
   if (scale > 1.0f)
     bitmap_size = gfx::ScaleToCeiledSize(view_size, scale);
 
-  host->CopyFromBackingStore(
-      gfx::Rect(view_size),
-      bitmap_size,
-      base::Bind(&CaptureWebContentsFunction::CopyFromBackingStoreComplete,
-                 this),
-      kN32_SkColorType);
+  host->CopyFromBackingStore(gfx::Rect(view_size), bitmap_size, callback,
+                             kN32_SkColorType);
   return true;
 }
 
-void CaptureWebContentsFunction::CopyFromBackingStoreComplete(
+void WebContentsCaptureClient::CopyFromBackingStoreComplete(
     const SkBitmap& bitmap,
     content::ReadbackResponse response) {
   if (response == content::READBACK_SUCCESS) {
     OnCaptureSuccess(bitmap);
     return;
   }
+  // TODO(wjmaclean): Improve error reporting. Why aren't we passing more
+  // information here?
+  std::string reason;
+  switch (response) {
+    case content::READBACK_FAILED:
+      reason = "READBACK_FAILED";
+      break;
+    case content::READBACK_SURFACE_UNAVAILABLE:
+      reason = "READBACK_SURFACE_UNAVAILABLE";
+      break;
+    case content::READBACK_BITMAP_ALLOCATION_FAILURE:
+      reason = "READBACK_BITMAP_ALLOCATION_FAILURE";
+      break;
+    default:
+      reason = "<unknown>";
+  }
   OnCaptureFailure(FAILURE_REASON_UNKNOWN);
 }
 
-void CaptureWebContentsFunction::OnCaptureSuccess(const SkBitmap& bitmap) {
+// TODO(wjmaclean) can this be static?
+bool WebContentsCaptureClient::EncodeBitmap(const SkBitmap& bitmap,
+                                            std::string* base64_result) {
+  DCHECK(base64_result);
   std::vector<unsigned char> data;
   SkAutoLockPixels screen_capture_lock(bitmap);
   bool encoded = false;
@@ -112,12 +113,8 @@ void CaptureWebContentsFunction::OnCaptureSuccess(const SkBitmap& bitmap) {
     case api::extension_types::IMAGE_FORMAT_JPEG:
       encoded = gfx::JPEGCodec::Encode(
           reinterpret_cast<unsigned char*>(bitmap.getAddr32(0, 0)),
-          gfx::JPEGCodec::FORMAT_SkBitmap,
-          bitmap.width(),
-          bitmap.height(),
-          static_cast<int>(bitmap.rowBytes()),
-          image_quality_,
-          &data);
+          gfx::JPEGCodec::FORMAT_SkBitmap, bitmap.width(), bitmap.height(),
+          static_cast<int>(bitmap.rowBytes()), image_quality_, &data);
       mime_type = kMimeTypeJpeg;
       break;
     case api::extension_types::IMAGE_FORMAT_PNG:
@@ -131,20 +128,17 @@ void CaptureWebContentsFunction::OnCaptureSuccess(const SkBitmap& bitmap) {
       NOTREACHED() << "Invalid image format.";
   }
 
-  if (!encoded) {
-    OnCaptureFailure(FAILURE_REASON_ENCODING_FAILED);
-    return;
-  }
+  if (!encoded)
+    return false;
 
-  std::string base64_result;
   base::StringPiece stream_as_string(reinterpret_cast<const char*>(data.data()),
                                      data.size());
 
-  base::Base64Encode(stream_as_string, &base64_result);
-  base64_result.insert(
+  base::Base64Encode(stream_as_string, base64_result);
+  base64_result->insert(
       0, base::StringPrintf("data:%s;base64,", mime_type.c_str()));
-  SetResult(new base::StringValue(base64_result));
-  SendResponse(true);
+
+  return true;
 }
 
 }  // namespace extensions
diff --git a/extensions/browser/api/capture_web_contents_function.h b/extensions/browser/api/web_contents_capture_client.h
index 53f78f7..4107ee4 100644
--- a/extensions/browser/api/capture_web_contents_function.h
+++ b/extensions/browser/api/web_contents_capture_client.h
@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef EXTENSIONS_BROWSER_API_CAPTURE_WEB_CONTENTS_FUNCTION_H_
-#define EXTENSIONS_BROWSER_API_CAPTURE_WEB_CONTENTS_FUNCTION_H_
+#ifndef EXTENSIONS_BROWSER_API_WEB_CONTENTS_CAPTURE_CLIENT_H_
+#define EXTENSIONS_BROWSER_API_WEB_CONTENTS_CAPTURE_CLIENT_H_
 
 #include "base/macros.h"
 #include "content/public/browser/readback_types.h"
@@ -18,50 +18,41 @@ class WebContents;
 
 namespace extensions {
 
-// Base class for capturing visibile area of a WebContents.
+// Base class for capturing visible area of a WebContents.
 // This is used by both webview.captureVisibleRegion and tabs.captureVisibleTab.
-class CaptureWebContentsFunction : public AsyncExtensionFunction {
+class WebContentsCaptureClient {
  public:
-  CaptureWebContentsFunction() {}
+  WebContentsCaptureClient() {}
 
  protected:
-  ~CaptureWebContentsFunction() override {}
-
-  // ExtensionFunction implementation.
-  bool HasPermission() override;
-  bool RunAsync() override;
+  virtual ~WebContentsCaptureClient() {}
 
   virtual bool IsScreenshotEnabled() = 0;
-  virtual content::WebContents* GetWebContentsForID(int context_id) = 0;
 
   enum FailureReason {
     FAILURE_REASON_UNKNOWN,
     FAILURE_REASON_ENCODING_FAILED,
     FAILURE_REASON_VIEW_INVISIBLE
   };
+  bool CaptureAsync(content::WebContents* web_contents,
+                    const api::extension_types::ImageDetails* image_detail,
+                    const content::ReadbackRequestCallback callback);
+  bool EncodeBitmap(const SkBitmap& bitmap, std::string* base64_result);
   virtual void OnCaptureFailure(FailureReason reason) = 0;
-
- private:
-
+  virtual void OnCaptureSuccess(const SkBitmap& bitmap) = 0;
   void CopyFromBackingStoreComplete(const SkBitmap& bitmap,
                                     content::ReadbackResponse response);
-  void OnCaptureSuccess(const SkBitmap& bitmap);
-
-  // |context_id_| is the ID used to find the relevant WebContents. In the
-  // |tabs.captureVisibleTab()| api, this represents the window-id, and in the
-  // |webview.captureVisibleRegion()| api, this represents the instance-id of
-  // the guest.
-  int context_id_;
 
+ private:
   // The format (JPEG vs PNG) of the resulting image.  Set in RunAsync().
   api::extension_types::ImageFormat image_format_;
 
   // Quality setting to use when encoding jpegs.  Set in RunAsync().
   int image_quality_;
 
-  DISALLOW_COPY_AND_ASSIGN(CaptureWebContentsFunction);
+  DISALLOW_COPY_AND_ASSIGN(WebContentsCaptureClient);
 };
 
 }  // namespace extensions
 
-#endif  // EXTENSIONS_BROWSER_API_CAPTURE_WEB_CONTENTS_FUNCTION_H_
+#endif  // EXTENSIONS_BROWSER_API_WEB_CONTENTS_CAPTURE_CLIENT_H_