Use the asynchronous version of CopyFromBackingStore in CaptureVisibleTabFunction.

chrome/browser/extensions/extension_tabs_module.cc

Index: chrome/browser/extensions/extension_tabs_module.cc
diff --git a/chrome/browser/extensions/extension_tabs_module.cc b/chrome/browser/extensions/extension_tabs_module.cc
index 054c58e8c36eb20be6150066abf8bcb835bbcc2e..c5bbb39cf47bb1f8745a96ed6affc562d5145933 100644
--- a/chrome/browser/extensions/extension_tabs_module.cc
+++ b/chrome/browser/extensions/extension_tabs_module.cc
@@ -57,6 +57,7 @@
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/render_view_host_delegate.h"
+#include "content/public/browser/render_widget_host_view.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_view.h"
 #include "content/public/common/url_constants.h"
@@ -1662,27 +1663,17 @@ bool CaptureVisibleTabFunction::RunImpl() {
     return false;
 
   RenderViewHost* render_view_host = web_contents->GetRenderViewHost();
-
-  // If a backing store is cached for the tab we want to capture,
-  // and it can be copied into a bitmap, then use it to generate the image.
-  // For example, some uncommon X11 visual modes are not supported by
-  // CopyFromBackingStore().
-  skia::PlatformCanvas temp_canvas;
-  if (render_view_host->CopyFromBackingStore(
-          gfx::Rect(), gfx::Size(), &temp_canvas)) {
-    VLOG(1) << "captureVisibleTab() got image from backing store.";
-    SendResultFromBitmap(skia::GetTopDevice(temp_canvas)->accessBitmap(false));
-    return true;
-  }
-
-  // Ask the renderer for a snapshot of the tab.
-  wrapper->snapshot_tab_helper()->CaptureSnapshot();

[mazda, 2012/05/02 01:23:25]

This line looks redundant and I copied the code without this line.
Is it correct?

[Sam Kerner (Chrome), 2012/05/02 03:26:09]

It looks redundant to me, but the code has changed substantially since I last
touched it.  git blame shows that it was added in r128037.  Adding the committer
as a reviewer, in case there is a reason I am missing.

-  registrar_.Add(this,
-                 chrome::NOTIFICATION_TAB_SNAPSHOT_TAKEN,
-                 content::Source<WebContents>(wrapper->web_contents()));
-  AddRef();  // Balanced in CaptureVisibleTabFunction::Observe().
-  wrapper->snapshot_tab_helper()->CaptureSnapshot();
-
+  content::RenderWidgetHostView* view = render_view_host->GetView();
+  if (!view)
+    return false;
+  skia::PlatformCanvas* temp_canvas = new skia::PlatformCanvas;
+  render_view_host->AsyncCopyFromBackingStore(
+      gfx::Rect(),
+      view->GetViewBounds().size(),
+      temp_canvas,
+      base::Bind(&CaptureVisibleTabFunction::CopyFromBackingStoreComplete,
+                 this,
+                 base::Owned(temp_canvas)));
   return true;
 }
 
@@ -1758,6 +1749,33 @@ void CaptureVisibleTabFunction::SendResultFromBitmap(
   SendResponse(true);
 }
 
+void CaptureVisibleTabFunction::CopyFromBackingStoreComplete(

[Sam Kerner (Chrome), 2012/05/02 03:47:14]

In this file, the functions are in the same order that they will be run in. 
This new function is out of that order, because it runs before Observe(). 
Please move it between RunImpl() and Observe().

[mazda, 2012/05/02 06:07:39]

Done.

+    skia::PlatformCanvas* canvas,
+    bool succeeded) {
+  if (succeeded) {
+    VLOG(1) << "captureVisibleTab() got image from backing store.";
+    SendResultFromBitmap(skia::GetTopDevice(*canvas)->accessBitmap(false));
+    return;
+  }
+
+  // TODO(mazda): Remove the following code once AsyncCopyFromBackingStore is

[Sam Kerner (Chrome), 2012/05/02 03:47:14]

It has been two years since I looked at this in detail, but at the time there
were legitimate reasons why a backing store might not be available.

One reason was a limit on the number of backing stores.  If there were more tabs
than the limit, a tab was not focused for a while might not have a backing
store.  If the tab was not focused, and some other tab was focused in the same
window, a backing store would not be created until a user focused the tab.

 Unless that is no longer true, I don't think you can remove the code below.

[mazda, 2012/05/02 06:07:39]

Thanks for the useful comment.
I think your comment is still true on most platforms, so this TODO comment would
not be appropriate. I deleted the comment.
One exception at this time is Chrome OS. CopyFromBackingStore was modified so
that it read page image data from GPU when the page is composited by GPU. If
pages are always composited by GPU (that's true on Chrome OS), the code below
should not be necessary.

+  // supported on all platforms.
+  WebContents* web_contents = NULL;
+  TabContentsWrapper* wrapper = NULL;
+  if (!GetTabToCapture(&web_contents, &wrapper)) {
+    error_ = keys::kInternalVisibleTabCaptureError;
+    return;
+  }
+
+  // Ask the renderer for a snapshot of the tab.
+  registrar_.Add(this,
+                 chrome::NOTIFICATION_TAB_SNAPSHOT_TAKEN,
+                 content::Source<WebContents>(web_contents));
+  AddRef();  // Balanced in CaptureVisibleTabFunction::Observe().
+  wrapper->snapshot_tab_helper()->CaptureSnapshot();
+  return;
+}
+
 bool DetectTabLanguageFunction::RunImpl() {
   int tab_id = 0;
   Browser* browser = NULL;