Clip the bitmap for creating the thumbnail on GPU when accelerated compositing is active.

Clip the bitmap for creating the thumbnail on GPU when accelerated compositing is active.

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

[mazda, 2012-07-28 02:01:23]

Hi Brett,
Could you review this change?

[mazda, 2012-08-02 19:50:02]

ping?

[brettw, 2012-08-05 05:38:39]

Sorry for the delay, busy with Flapper stuff.

http://codereview.chromium.org/10831063/diff/1/chrome/browser/tab_contents/th...
File chrome/browser/tab_contents/thumbnail_generator.cc (right):

http://codereview.chromium.org/10831063/diff/1/chrome/browser/tab_contents/th...
chrome/browser/tab_contents/thumbnail_generator.cc:68: static const int
kScrollbarThickness = 15;
I wonder, can we get the actual value from the theming code?

http://codereview.chromium.org/10831063/diff/1/chrome/browser/tab_contents/th...
chrome/browser/tab_contents/thumbnail_generator.cc:103: const float
desired_aspect =
Optional: IMO const in this case just adds noise (same with src_rect below).

Since you just moved this, I didn't re-review this logic.

http://codereview.chromium.org/10831063/diff/1/chrome/browser/tab_contents/th...
chrome/browser/tab_contents/thumbnail_generator.cc:151: // Clip the edgemost 15
pixels as that will commonly hold a scrollbar, which
I'd remove the 15 pixel comment here since the actual value is defined
elsewhere.

http://codereview.chromium.org/10831063/diff/1/chrome/browser/tab_contents/th...
chrome/browser/tab_contents/thumbnail_generator.cc:513: // Clip the edgemost 15
pixels as that will commonly hold a scrollbar, which
Ditto w/ "15 pixels."

[mazda, 2012-08-06 17:22:16]

Thank you for the review! Please take another look.

http://codereview.chromium.org/10831063/diff/1/chrome/browser/tab_contents/th...
File chrome/browser/tab_contents/thumbnail_generator.cc (right):

http://codereview.chromium.org/10831063/diff/1/chrome/browser/tab_contents/th...
chrome/browser/tab_contents/thumbnail_generator.cc:68: static const int
kScrollbarThickness = 15;
On 2012/08/05 05:38:39, brettw wrote:
> I wonder, can we get the actual value from the theming code?

I found a function to get scrollbar size in ui/gfx/scrollbar_size.h and replaced
this constant with that function.
Thanks.

http://codereview.chromium.org/10831063/diff/1/chrome/browser/tab_contents/th...
chrome/browser/tab_contents/thumbnail_generator.cc:103: const float
desired_aspect =
On 2012/08/05 05:38:39, brettw wrote:
> Optional: IMO const in this case just adds noise (same with src_rect below).
> 
> Since you just moved this, I didn't re-review this logic.

Removed const.

http://codereview.chromium.org/10831063/diff/1/chrome/browser/tab_contents/th...
chrome/browser/tab_contents/thumbnail_generator.cc:151: // Clip the edgemost 15
pixels as that will commonly hold a scrollbar, which
On 2012/08/05 05:38:39, brettw wrote:
> I'd remove the 15 pixel comment here since the actual value is defined
> elsewhere.

Removed the magic number (15) from the comment.

http://codereview.chromium.org/10831063/diff/1/chrome/browser/tab_contents/th...
chrome/browser/tab_contents/thumbnail_generator.cc:513: // Clip the edgemost 15
pixels as that will commonly hold a scrollbar, which
On 2012/08/05 05:38:39, brettw wrote:
> Ditto w/ "15 pixels."

Done.

[brettw, 2012-08-07 17:52:53]

lgtm

[commit-bot: I haz the power, 2012-08-07 18:10:37]

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

[commit-bot: I haz the power, 2012-08-07 21:04:18]

Change committed as 150398