Revert of Implement webview.captureVisibleRegion() (patchset #9 id:160001 of https://codereview.chromium.org/1582053002/ )

Reason for revert:
The test WebViewAPITest.TestCaptureVisibleRegion is failing on several windows bots:

https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%281%29/builds/45040
https://build.chromium.org/p/chromium.win/builders/XP%20Tests%20%281%29/builds/42690
https://build.chromium.org/p/chromium.win/builders/Vista%20Tests%20%281%29/builds/64394

Original issue's description:
> 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.
>
> BUG=326755
>
> Committed: https://crrev.com/5b8cdcc82fc9da7bab15fd3dbf65df843dc73589
> Cr-Commit-Position: refs/heads/master@{#370565}

TBR=rockot@chromium.org,creis@chromium.org,kenrb@chromium.org,nick@chromium.org,fsamuel@chromium.org,wjmaclean@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=326755

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

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

5 files changed, 70 insertions, 119 deletions

diff --git a/extensions/browser/api/web_contents_capture_client.cc b/extensions/browser/api/capture_web_contents_function.cc
index 7a38202..e10842d 100644
--- a/extensions/browser/api/web_contents_capture_client.cc
+++ b/extensions/browser/api/capture_web_contents_function.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/web_contents_capture_client.h"
+#include "extensions/browser/api/capture_web_contents_function.h"
 
 #include "base/base64.h"
 #include "base/strings/stringprintf.h"
@@ -25,16 +25,30 @@ namespace extensions {
 
 using api::extension_types::ImageDetails;
 
-bool WebContentsCaptureClient::CaptureAsync(
-    WebContents* web_contents,
-    const ImageDetails* image_details,
-    const content::ReadbackRequestCallback callback) {
-  if (!web_contents)
-    return false;
+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())
     return false;
 
+  WebContents* contents = GetWebContentsForID(context_id_);
+  if (!contents)
+    return false;
+
   // The default format and quality setting used when encoding jpegs.
   const api::extension_types::ImageFormat kDefaultFormat =
       api::extension_types::IMAGE_FORMAT_JPEG;
@@ -51,7 +65,7 @@ bool WebContentsCaptureClient::CaptureAsync(
   }
 
   // TODO(miu): Account for fullscreen render widget?  http://crbug.com/419878
-  RenderWidgetHostView* const view = web_contents->GetRenderWidgetHostView();
+  RenderWidgetHostView* const view = contents->GetRenderWidgetHostView();
   RenderWidgetHost* const host = view ? view->GetRenderWidgetHost() : nullptr;
   if (!view || !host) {
     OnCaptureFailure(FAILURE_REASON_VIEW_INVISIBLE);
@@ -70,27 +84,26 @@ bool WebContentsCaptureClient::CaptureAsync(
   if (scale > 1.0f)
     bitmap_size = gfx::ScaleToCeiledSize(view_size, scale);
 
-  host->CopyFromBackingStore(gfx::Rect(view_size), bitmap_size, callback,
-                             kN32_SkColorType);
+  host->CopyFromBackingStore(
+      gfx::Rect(view_size),
+      bitmap_size,
+      base::Bind(&CaptureWebContentsFunction::CopyFromBackingStoreComplete,
+                 this),
+      kN32_SkColorType);
   return true;
 }
 
-void WebContentsCaptureClient::CopyFromBackingStoreComplete(
+void CaptureWebContentsFunction::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?
   OnCaptureFailure(FAILURE_REASON_UNKNOWN);
 }
 
-// TODO(wjmaclean) can this be static?
-bool WebContentsCaptureClient::EncodeBitmap(const SkBitmap& bitmap,
-                                            std::string* base64_result) {
-  DCHECK(base64_result);
+void CaptureWebContentsFunction::OnCaptureSuccess(const SkBitmap& bitmap) {
   std::vector<unsigned char> data;
   SkAutoLockPixels screen_capture_lock(bitmap);
   bool encoded = false;
@@ -99,8 +112,12 @@ bool WebContentsCaptureClient::EncodeBitmap(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:
@@ -114,17 +131,20 @@ bool WebContentsCaptureClient::EncodeBitmap(const SkBitmap& bitmap,
       NOTREACHED() << "Invalid image format.";
   }
 
-  if (!encoded)
-    return false;
+  if (!encoded) {
+    OnCaptureFailure(FAILURE_REASON_ENCODING_FAILED);
+    return;
+  }
 
+  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()));
-
-  return true;
+  SetResult(new base::StringValue(base64_result));
+  SendResponse(true);
 }
 
 }  // namespace extensions
diff --git a/extensions/browser/api/web_contents_capture_client.h b/extensions/browser/api/capture_web_contents_function.h
index 4107ee4..53f78f7 100644
--- a/extensions/browser/api/web_contents_capture_client.h
+++ b/extensions/browser/api/capture_web_contents_function.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_WEB_CONTENTS_CAPTURE_CLIENT_H_
-#define EXTENSIONS_BROWSER_API_WEB_CONTENTS_CAPTURE_CLIENT_H_
+#ifndef EXTENSIONS_BROWSER_API_CAPTURE_WEB_CONTENTS_FUNCTION_H_
+#define EXTENSIONS_BROWSER_API_CAPTURE_WEB_CONTENTS_FUNCTION_H_
 
 #include "base/macros.h"
 #include "content/public/browser/readback_types.h"
@@ -18,41 +18,50 @@ class WebContents;
 
 namespace extensions {
 
-// Base class for capturing visible area of a WebContents.
+// Base class for capturing visibile area of a WebContents.
 // This is used by both webview.captureVisibleRegion and tabs.captureVisibleTab.
-class WebContentsCaptureClient {
+class CaptureWebContentsFunction : public AsyncExtensionFunction {
  public:
-  WebContentsCaptureClient() {}
+  CaptureWebContentsFunction() {}
 
  protected:
-  virtual ~WebContentsCaptureClient() {}
+  ~CaptureWebContentsFunction() override {}
+
+  // ExtensionFunction implementation.
+  bool HasPermission() override;
+  bool RunAsync() override;
 
   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;
-  virtual void OnCaptureSuccess(const SkBitmap& bitmap) = 0;
+
+ private:
+
   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(WebContentsCaptureClient);
+  DISALLOW_COPY_AND_ASSIGN(CaptureWebContentsFunction);
 };
 
 }  // namespace extensions
 
-#endif  // EXTENSIONS_BROWSER_API_WEB_CONTENTS_CAPTURE_CLIENT_H_
+#endif  // EXTENSIONS_BROWSER_API_CAPTURE_WEB_CONTENTS_FUNCTION_H_
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 1899d42..9e77cbd 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,6 +6,7 @@
 #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 3e3a8ed..bba531d 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,62 +259,6 @@ 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 388491e..dcdb828 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,29 +37,6 @@ 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: