cc: Don't make tiles for mask layers that are too big for a texture.

cc: Don't make tiles for mask layers that are too big for a texture.

If a mask layer's width or height are larger than the max texture size,
we do not use the contents of the mask layer. So if we're not going to
use the contents, there's no reason to make tiles and allocate memory
for them.

This changes PictureLayerImpl to return an empty tile size for masks
that are too large to have tiles. Then changes PictureLayerTiling to
use the empty tile size as a signal to make the whole tiling an empty
size.

Last, we use the empty tile size to also signal not making a low res
tiling, so that masks continue to not get a low res tiling if they are
large.

R=enne, vmpstr
BUG=409984
Committed: https://crrev.com/132fdfe17dd03c0066e27fe876602122c522aa8b
Cr-Commit-Position: refs/heads/master@{#293656}

[danakj, 2014-09-04 23:27:59]

This fixes the memory problems on the github site at the cost of "correctness",
which is not at all visible in this case anyways.

[danakj, 2014-09-05 15:22:29]

On 2014/09/04 23:27:59, danakj wrote:
> This fixes the memory problems on the github site at the cost of
"correctness",
> which is not at all visible in this case anyways.

Vollick is turning off promotion to composited scrolling when a border radius is
present which will fix correctness in this case. But this still fixes the issue
of web authors making a large mask causing us to allocate a ton of memory and
then not use it.

[enne (OOO), 2014-09-05 17:41:46]

https://codereview.chromium.org/547463002/diff/20001/cc/layers/picture_layer_...
File cc/layers/picture_layer_impl_unittest.cc (right):

https://codereview.chromium.org/547463002/diff/20001/cc/layers/picture_layer_...
cc/layers/picture_layer_impl_unittest.cc:1218:
EXPECT_BOTH_EQ(HighResTiling()->AllTilesForTesting().size(), 1u);
"there should be no tiles" but 1u?

Also, can you call ContentsResourceId inside this unit test before and after to
make sure it returns non-zero and zero, respectively?

[vmpstr, 2014-09-05 17:44:57]

https://codereview.chromium.org/547463002/diff/20001/cc/layers/picture_layer_...
File cc/layers/picture_layer_impl.cc (right):

https://codereview.chromium.org/547463002/diff/20001/cc/layers/picture_layer_...
cc/layers/picture_layer_impl.cc:649:
layer_tree_impl()->resource_provider()->max_texture_size();
Is this max_texture_size the same as layer_tree_impl()->MaxTextureSize (which
was returned before) I guess it's the same since MaxTextureSize went away.

https://codereview.chromium.org/547463002/diff/20001/cc/layers/picture_layer_...
cc/layers/picture_layer_impl.cc:652: if (content_bounds.width() >
max_texture_size ||
Can you please drop a comment here saying that "blah blah tiled mask blah no".

[danakj, 2014-09-05 22:09:52]

PTAL!

While fixing my test, I cleaned up how we pass around the is_mask_ value.

We were passing it from PictureLayer to PictureLayerImpl, but this left this
setter method on PLI that tests could call, and it would reset tiles (but not
tilings.. but the tile sizes would be wrong..). And the code actually had to
ensure the pile was set before the mask value, or you can make tilings at the
wrong tile size and nothing fixes that later.

Anyhow.. so I put the bool on PicturePileBase instead, and it makes my test work
now without doing weird things, and should make the PictureLayerImpl get the
value more implicitly at the right time (ie impossible to mess up).

The ResetAllTiles() when mask changed, I didn't move anywhere else. If the mask
value changes, then the tile size changes. And if the tile size didn't change (a
small mask for instance that is a single tile when not a mask) there's no reason
to destroy tiles.

https://codereview.chromium.org/547463002/diff/20001/cc/layers/picture_layer_...
File cc/layers/picture_layer_impl.cc (right):

https://codereview.chromium.org/547463002/diff/20001/cc/layers/picture_layer_...
cc/layers/picture_layer_impl.cc:649:
layer_tree_impl()->resource_provider()->max_texture_size();
On 2014/09/05 17:44:57, vmpstr wrote:
> Is this max_texture_size the same as layer_tree_impl()->MaxTextureSize (which
> was returned before) I guess it's the same since MaxTextureSize went away.

Ya. RendererCaps is set based on the value from ResourceProvider (it's weird we
have this in 2 places)

https://codereview.chromium.org/547463002/diff/20001/cc/layers/picture_layer_...
cc/layers/picture_layer_impl.cc:652: if (content_bounds.width() >
max_texture_size ||
On 2014/09/05 17:44:57, vmpstr wrote:
> Can you please drop a comment here saying that "blah blah tiled mask blah no".

Done.

https://codereview.chromium.org/547463002/diff/20001/cc/layers/picture_layer_...
File cc/layers/picture_layer_impl_unittest.cc (right):

https://codereview.chromium.org/547463002/diff/20001/cc/layers/picture_layer_...
cc/layers/picture_layer_impl_unittest.cc:1218:
EXPECT_BOTH_EQ(HighResTiling()->AllTilesForTesting().size(), 1u);
On 2014/09/05 17:41:46, enne wrote:
> "there should be no tiles" but 1u?

Oh snap, thanks. My test is bad and I feel bad.

> Also, can you call ContentsResourceId inside this unit test before and after
to
> make sure it returns non-zero and zero, respectively?

Ya!

[enne (OOO), 2014-09-05 22:55:34]

lgtm

[vmpstr, 2014-09-05 23:05:30]

lgtm as well

[danakj, 2014-09-05 23:45:47]

The CQ bit was checked by danakj@chromium.org

[commit-bot: I haz the power, 2014-09-05 23:48:00]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/danakj@chromium.org/547463002/40001

[commit-bot: I haz the power, 2014-09-06 10:12:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-06 10:12:21]

Exceeded time limit waiting for builds to trigger.

[danakj, 2014-09-06 19:28:40]

The CQ bit was checked by danakj@chromium.org

[commit-bot: I haz the power, 2014-09-06 19:29:33]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/danakj@chromium.org/547463002/40001

[commit-bot: I haz the power, 2014-09-06 19:45:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-06 19:45:18]

Try jobs failed on following builders:
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/57554)

[danakj, 2014-09-06 22:51:09]

The CQ bit was checked by danakj@chromium.org

[commit-bot: I haz the power, 2014-09-06 22:51:20]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/danakj@chromium.org/547463002/60001

[commit-bot: I haz the power, 2014-09-07 00:51:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-07 00:51:39]

Exceeded time limit waiting for builds to trigger.

[danakj, 2014-09-07 05:30:56]

The CQ bit was checked by danakj@chromium.org

[commit-bot: I haz the power, 2014-09-07 05:31:10]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/danakj@chromium.org/547463002/60001

[commit-bot: I haz the power, 2014-09-07 06:09:03]

Committed patchset #4 (id:60001) as ad79a318f83ac3a7364d57753bbc0a1bfb5c85d6

[commit-bot: I haz the power, 2014-09-10 03:44:05]

Patchset 4 (id:??) landed as
https://crrev.com/132fdfe17dd03c0066e27fe876602122c522aa8b
Cr-Commit-Position: refs/heads/master@{#293656}