Limit size of scaled layer rects in ScrollbarLayer to content_bounds().

Limit size of scaled layer rects in ScrollbarLayer to content_bounds().

ScrollbarLayer allocates textures for the back-/fore-tracks that are sized to
the content_bounds.

When a non-integer contents_scale is applied to a ScrollbarLayer at present, it
may paint bitmaps that are larger than the texture size. This can happen when
the following conditions are true:

1) The scrollbar location is non-zero, and
2) the contents scale is non-integer.

In this case, the bitmap is painted to a rect that is created from both a scaled
location and the content_bounds(), and then enlarged using ToEnclosingRect().
The enlarging operation can result in a paint rect that is larger-by-one than
the content_bounds() in one or two dimensions, leading to an error when
attempting to upload the bitmap to the texture.

Non-integer content_bounds() can occur either through non-integer page scales
encountered during pinch-zoom, or non-integer device scale factors (1.4 and
1.8 are common on Windows), or a combination of both.


This CL clamps the scaled layer rects produced internally in ScrollbarLayer so
they do not exceed the content_bounds().

BUG=229614
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=201392

[wjmaclean, 2013-05-17 19:17:28]

I think this CL explains the issue with scaled scrollbars having troubles
uploading their textures due to size mismatches.

Let me know what you think.

We should maybe consider adding the expectations conditions from the added unit
test,

EXPECT_LE(update.dest_offset.x() + update.content_rect.width(),
update.texture->size().width());

EXPECT_LE(update.dest_offset.y() + update.content_rect.height(),
update.texture->size().height());

to ResourceUpdate::Create() to help detect if similar situations arise in other
layer types.

[wjmaclean, 2013-05-17 19:36:25]

BTW, when fixing this I had to make a choice between clamping the size of the
paint rect to content_bounds(), which I think makes sense, vs. allocating a
texture to match the size of the paint rect. The latter seems wrong because it
could result in textures that exceed the size of the content_bounds(), but maybe
I'm wrong and we should paint the larger texture.

[enne (OOO), 2013-05-17 21:46:15]

Can you remind me, when is the scrollbar location non-zero? Is that something we
could get rid of?

[wjmaclean, 2013-05-18 00:18:00]

On 2013/05/17 21:46:15, enne wrote:
> Can you remind me, when is the scrollbar location non-zero? Is that something
we
> could get rid of?

It seems to be routinely non-zero ... ordinary main-frame scrollbars have
non-zero location, hence the problem in the first place.

As for whether we can get rid of it or not, that's a much bigger question ...
not sure off the top of my head what the answer is there.

[enne (OOO), 2013-05-20 17:27:21]

lgtm

[commit-bot: I haz the power, 2013-05-21 12:36:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wjmaclean@chromium.org/15294018/1

[commit-bot: I haz the power, 2013-05-21 12:36:59]

Failed to apply patch for cc/layers/scrollbar_layer.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file cc/layers/scrollbar_layer.cc
  Hunk #1 FAILED at 361.
  1 out of 1 hunk FAILED -- saving rejects to file
cc/layers/scrollbar_layer.cc.rej

Patch:       cc/layers/scrollbar_layer.cc
Index: cc/layers/scrollbar_layer.cc
diff --git a/cc/layers/scrollbar_layer.cc b/cc/layers/scrollbar_layer.cc
index
3d6f6f5819323b8b2b6c94924d445bc6810ca7d3..bae693cd7e14e47099dc73b2e690c6b5268dd257
100644
--- a/cc/layers/scrollbar_layer.cc
+++ b/cc/layers/scrollbar_layer.cc
@@ -361,7 +361,12 @@ gfx::Rect ScrollbarLayer::ScrollbarLayerRectToContentRect(
   gfx::RectF content_rect = gfx::ScaleRect(layer_rect,
                                            contents_scale_y(),
                                            contents_scale_y());
-  return gfx::ToEnclosingRect(content_rect);
+  gfx::Rect expanded_rect = gfx::ToEnclosingRect(content_rect);
+  // We should never return a rect bigger than the content_bounds().
+  gfx::Size clamped_size = expanded_rect.size();
+  clamped_size.ClampToMax(content_bounds());
+  expanded_rect.set_size(clamped_size);
+  return expanded_rect;
 }
 
 void ScrollbarLayer::SetTexturePriorities(

[commit-bot: I haz the power, 2013-05-21 13:31:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wjmaclean@chromium.org/15294018/9001

[commit-bot: I haz the power, 2013-05-21 13:39:58]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac&number...

[commit-bot: I haz the power, 2013-05-21 14:02:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wjmaclean@chromium.org/15294018/9001

[commit-bot: I haz the power, 2013-05-22 00:02:53]

Change committed as 201392