cc: Separate tiling set functionality into explicit separate functions.

cc: Separate tiling set functionality into explicit separate functions.

This patch separates UpdateTilingsToCurrentRaster source into three
separate functions, so that it is clear when each one is meant to run:

- UpdateTilingsToCurrentRasterSourceForCommit(raster_source, invalidation)
    This function is called during PictureLayer::PushPropertiesTo to on
    the sync tree (either pending or active).
- UpdateTilingsToCurrentRasterSourceForActivation(
      raster_source, pending_twin_set, invalidation)
    This function is called during PictureLayerImpl::PushPropertiesTo on
    the active tree only.
- UpdateRasterSourceDueToLCDChange(raster_source, invalidation)
    This function is called on the sync tree when the LCD flag changes.
    The raster source in this case is of the same size as previously set
    but has lcd flag changed.

R=danakj, enne

Committed: https://crrev.com/074006a706461a22204fbdf183161819f416290b
Cr-Commit-Position: refs/heads/master@{#323550}

[vmpstr, 2015-03-30 19:26:49]

vmpstr@chromium.org changed reviewers:
+ danakj@chromium.org, enne@chromium.org

[vmpstr, 2015-03-30 19:26:59]

Please take a look.

[enne (OOO), 2015-03-31 19:53:02]

https://codereview.chromium.org/1041893003/diff/1/cc/layers/picture_layer_imp...
File cc/layers/picture_layer_impl.cc (right):

https://codereview.chromium.org/1041893003/diff/1/cc/layers/picture_layer_imp...
cc/layers/picture_layer_impl.cc:541:
tilings_->RemoveTilingsBelowScale(MinimumContentsScale());
I'm not 100% on moving this.  I agree that something called "UpdateRasterSource"
maybe shouldn't be removing tilings, but it really is all about adding tilings
from the pending set if needed, and so removing tilings that aren't needed also
seems like a legit operation to have there too.

What if the RemoveTilings code went into
UpdateTilingsToCurrentRasterSourceForCommit?

https://codereview.chromium.org/1041893003/diff/1/cc/resources/picture_layer_...
File cc/resources/picture_layer_tiling_set_unittest.cc (right):

https://codereview.chromium.org/1041893003/diff/1/cc/resources/picture_layer_...
cc/resources/picture_layer_tiling_set_unittest.cc:438: // Update to a new source
frame with a max content scale that will drop one
This test no longer really makes any sense, since the test itself is removing
the tilings and then verifying that it's removed.  Maybe the suggestion I made
elsewhere would make this better.

[vmpstr, 2015-04-01 23:53:41]

PTAL

https://codereview.chromium.org/1041893003/diff/1/cc/layers/picture_layer_imp...
File cc/layers/picture_layer_impl.cc (right):

https://codereview.chromium.org/1041893003/diff/1/cc/layers/picture_layer_imp...
cc/layers/picture_layer_impl.cc:541:
tilings_->RemoveTilingsBelowScale(MinimumContentsScale());
On 2015/03/31 19:53:02, enne wrote:
> I'm not 100% on moving this.  I agree that something called
"UpdateRasterSource"
> maybe shouldn't be removing tilings, but it really is all about adding tilings
> from the pending set if needed, and so removing tilings that aren't needed
also
> seems like a legit operation to have there too.
> 
> What if the RemoveTilings code went into
> UpdateTilingsToCurrentRasterSourceForCommit?

I moved back into both functions (as it would have been before). It makes sense
that since we're creating tilings in that function that we would be removing
them as well. I did add it to both, since I don't want to change behavior with
this patch, just restructure the calls a bit.

https://codereview.chromium.org/1041893003/diff/1/cc/resources/picture_layer_...
File cc/resources/picture_layer_tiling_set_unittest.cc (right):

https://codereview.chromium.org/1041893003/diff/1/cc/resources/picture_layer_...
cc/resources/picture_layer_tiling_set_unittest.cc:438: // Update to a new source
frame with a max content scale that will drop one
On 2015/03/31 19:53:02, enne wrote:
> This test no longer really makes any sense, since the test itself is removing
> the tilings and then verifying that it's removed.  Maybe the suggestion I made
> elsewhere would make this better.

Changed it back with RemoveTilings being a part of UpdateTilings.

[enne (OOO), 2015-04-02 00:13:47]

lgtm

[vmpstr, 2015-04-02 16:57:35]

The CQ bit was checked by vmpstr@chromium.org

[commit-bot: I haz the power, 2015-04-02 16:59:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1041893003/20001

[commit-bot: I haz the power, 2015-04-02 19:28:35]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-04-03 20:28:30]

Patchset 2 (id:??) landed as
https://crrev.com/074006a706461a22204fbdf183161819f416290b
Cr-Commit-Position: refs/heads/master@{#323550}