Support in-browser thumbnailing on Windows XP.

Support in-browser thumbnailing on Windows XP.

On Windows XP, neither the backing store nor the compositing surface is available in the browser when accelerated compositing is active.
This CL makes the thumbnailer ask the renderer to send a snapshot for creating the thumbnail in such a case.

BUG=130701
TEST=Manually check the new code path worked on Windows XP

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=146586

[mazda, 2012-07-12 18:32:54]

Could you review this change?

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

lgtm

http://codereview.chromium.org/10704190/diff/2001/chrome/browser/tab_contents...
File chrome/browser/tab_contents/thumbnail_generator.cc (right):

http://codereview.chromium.org/10704190/diff/2001/chrome/browser/tab_contents...
chrome/browser/tab_contents/thumbnail_generator.cc:495: const bool
surface_available = view->IsSurfaceAvailableForCopy();
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.

http://codereview.chromium.org/10704190/diff/2001/chrome/browser/tab_contents...
chrome/browser/tab_contents/thumbnail_generator.cc:502:
web_contents_weak_factory_.reset(
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.

http://codereview.chromium.org/10704190/diff/2001/chrome/browser/tab_contents...
File chrome/browser/tab_contents/thumbnail_generator.h (right):

http://codereview.chromium.org/10704190/diff/2001/chrome/browser/tab_contents...
chrome/browser/tab_contents/thumbnail_generator.h:148:
base::WeakPtr<content::WebContents> web_contents,
Can you pass this weak ptr by const ref? Otherwise the copy will have to involve
a bunch of bookkeeping.

http://codereview.chromium.org/10704190/diff/2001/chrome/browser/tab_contents...
chrome/browser/tab_contents/thumbnail_generator.h:154:
base::WeakPtr<content::WebContents> web_contents,
Ditto for const ref.

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

Thank you for the comments.

http://codereview.chromium.org/10704190/diff/2001/chrome/browser/tab_contents...
File chrome/browser/tab_contents/thumbnail_generator.cc (right):

http://codereview.chromium.org/10704190/diff/2001/chrome/browser/tab_contents...
chrome/browser/tab_contents/thumbnail_generator.cc:495: const bool
surface_available = view->IsSurfaceAvailableForCopy();
On 2012/07/12 19:20:26, brettw wrote:
> 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.

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

http://codereview.chromium.org/10704190/diff/2001/chrome/browser/tab_contents...
chrome/browser/tab_contents/thumbnail_generator.cc:502:
web_contents_weak_factory_.reset(
On 2012/07/12 19:20:26, brettw wrote:
> 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.

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.

http://codereview.chromium.org/10704190/diff/2001/chrome/browser/tab_contents...
File chrome/browser/tab_contents/thumbnail_generator.h (right):

http://codereview.chromium.org/10704190/diff/2001/chrome/browser/tab_contents...
chrome/browser/tab_contents/thumbnail_generator.h:148:
base::WeakPtr<content::WebContents> web_contents,
On 2012/07/12 19:20:26, brettw wrote:
> Can you pass this weak ptr by const ref? Otherwise the copy will have to
involve
> a bunch of bookkeeping.

Done.

http://codereview.chromium.org/10704190/diff/2001/chrome/browser/tab_contents...
chrome/browser/tab_contents/thumbnail_generator.h:154:
base::WeakPtr<content::WebContents> web_contents,
On 2012/07/12 19:20:26, brettw wrote:
> Ditto for const ref.

Done.

[commit-bot: I haz the power, 2012-07-13 00:08:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mazda@chromium.org/10704190/10001

[commit-bot: I haz the power, 2012-07-13 01:51:56]

Try job failure for 10704190-10001 (retry) on win_rel for step "browser_tests".
It's a second try, previously, step "browser_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2012-07-13 02:48:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mazda@chromium.org/10704190/10001

[commit-bot: I haz the power, 2012-07-13 03:46:32]

Try job failure for 10704190-10001 (retry) on mac_rel for step "browser_tests".
It's a second try, previously, step "browser_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2012-07-13 15:40:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mazda@chromium.org/10704190/10001

[commit-bot: I haz the power, 2012-07-13 16:55:21]

Change committed as 146586