Support in-browser thumbnailing on Windows XP.

chrome/browser/tab_contents/thumbnail_generator.cc

Index: chrome/browser/tab_contents/thumbnail_generator.cc
diff --git a/chrome/browser/tab_contents/thumbnail_generator.cc b/chrome/browser/tab_contents/thumbnail_generator.cc
index c2d5f91696166f5b8edfa0d9af100a47de9c7e95..6911bb61ec83fc84b24567ddb47228c0e8658976 100644
--- a/chrome/browser/tab_contents/thumbnail_generator.cc
+++ b/chrome/browser/tab_contents/thumbnail_generator.cc
@@ -32,6 +32,10 @@
 #include "ui/gfx/rect.h"
 #include "ui/gfx/skbitmap_operations.h"
 
+#if defined(OS_WIN)
+#include "base/win/windows_version.h"
+#endif
+
 // Overview
 // --------
 // This class provides current thumbnails for tabs. The simplest operation is
@@ -439,13 +443,6 @@ SkBitmap ThumbnailGenerator::GetClippedBitmap(const SkBitmap& bitmap,
 
 void ThumbnailGenerator::UpdateThumbnailIfNecessary(
     WebContents* web_contents) {
-  content::RenderWidgetHostView* view = web_contents->GetRenderWidgetHostView();
-  if (!view)
-    return;
-  bool surface_available = view->IsSurfaceAvailableForCopy();
-  // Skip if we can't update the thumbnail.
-  if (!surface_available)
-    return;
   // Skip if a pending entry exists. WidgetHidden can be called while navigaing
   // pages and this is not a timing when thumbnails should be generated.
   if (web_contents->GetController().GetPendingEntry())
@@ -495,6 +492,27 @@ void ThumbnailGenerator::AsyncUpdateThumbnail(
   content::RenderWidgetHostView* view = render_widget_host->GetView();
   if (!view)
     return;
+  const bool surface_available = view->IsSurfaceAvailableForCopy();

[brettw, 2012/07/12 19:20:26]

I don't usually use "const" for local vars like this. Except in special cases, I
feel like it's extra clutter and doesn't really add anything. Same with "size"
below.

[mazda, 2012/07/12 21:23:25]

Done for |size|.
I deleted |surface_available| instead as this does not seem to improve
readability.

+  if (!surface_available) {
+#if defined(OS_WIN)
+    // On Windows XP, neither the backing store nor the compositing surface is
+    // available in the browser when accelerated compositing is active, so ask
+    // the renderer to send a snapshot for creating the thumbnail.
+    if (base::win::GetVersion() < base::win::VERSION_VISTA) {
+      web_contents_weak_factory_.reset(

[brettw, 2012/07/12 19:20:26]

This pattern is a bit unusual. If possible, I think it would be clearer to have
the weak factory be a regular member and just call InvalidateWeakPtrs() when you
want to cancel the out-standing requests.

[mazda, 2012/07/12 21:23:25]

I changed the code to initialize |web_contents_weak_factory_| in
StartThumbnailing not to reset it every time AsyncUpdateThumbnail is called.
the weak factory is invalidated when the web contents is disconnected.
This subtly changes the behavior of the thumbnailer in that it will allow
multiple requests to be made before the precedented request completes, but there
should be no problem.

+          new base::WeakPtrFactory<WebContents>(web_contents));
+      const gfx::Size view_size =
+          render_widget_host->GetView()->GetViewBounds().size();
+      AskForSnapshot(render_widget_host,
+                     base::Bind(&ThumbnailGenerator::UpdateThumbnailWithBitmap,
+                                weak_factory_.GetWeakPtr(),
+                                web_contents_weak_factory_->GetWeakPtr()),
+                     view_size,
+                     view_size);
+    }
+#endif
+    return;
+  }
 
   const gfx::Size copy_size =
       GetCopySizeForThumbnail(view->GetViewBounds().size(),
@@ -504,13 +522,31 @@ void ThumbnailGenerator::AsyncUpdateThumbnail(
       new base::WeakPtrFactory<WebContents>(web_contents));
   render_widget_host->CopyFromBackingStore(
       gfx::Rect(), copy_size, temp_canvas,
-      base::Bind(&ThumbnailGenerator::AsyncUpdateThumbnailFinish,
+      base::Bind(&ThumbnailGenerator::UpdateThumbnailWithCanvas,
                  weak_factory_.GetWeakPtr(),
                  web_contents_weak_factory_->GetWeakPtr(),
                  base::Owned(temp_canvas)));
 }
 
-void ThumbnailGenerator::AsyncUpdateThumbnailFinish(
+void ThumbnailGenerator::UpdateThumbnailWithBitmap(
+    base::WeakPtr<WebContents> web_contents,
+    const SkBitmap& bitmap) {
+  if (!web_contents.get())
+    return;
+
+  if (bitmap.isNull() || bitmap.empty())
+    return;
+
+  ClipResult clip_result;
+  SkBitmap thumbnail = CreateThumbnail(bitmap,
+                                       kThumbnailWidth,
+                                       kThumbnailHeight,
+                                       ThumbnailGenerator::kClippedThumbnail,
+                                       &clip_result);
+  UpdateThumbnail(web_contents.get(), thumbnail, clip_result);
+}
+
+void ThumbnailGenerator::UpdateThumbnailWithCanvas(
     base::WeakPtr<WebContents> web_contents,
     skia::PlatformCanvas* temp_canvas,
     bool succeeded) {
@@ -524,13 +560,7 @@ void ThumbnailGenerator::AsyncUpdateThumbnailFinish(
 
   SkBitmap bmp_with_scrollbars =
       skia::GetTopDevice(*temp_canvas)->accessBitmap(false);
-  ClipResult clip_result;
-  SkBitmap thumbnail = CreateThumbnail(bmp_with_scrollbars,
-                                       kThumbnailWidth,
-                                       kThumbnailHeight,
-                                       ThumbnailGenerator::kClippedThumbnail,
-                                       &clip_result);
-  UpdateThumbnail(web_contents.get(), thumbnail, clip_result);
+  UpdateThumbnailWithBitmap(web_contents, bmp_with_scrollbars);
 }
 
 bool ThumbnailGenerator::ShouldUpdateThumbnail(Profile* profile,