cc: Early out sooner in UpdateTilePriorities when !CanHaveTilings().

cc: Early out sooner in UpdateTilePriorities when !CanHaveTilings().

Also, when we cleanup tilings on the active twin, we use the pending
twin's ideal contents scale. But when the twin !CanHavTilings() there
is no need to use that ideal scale and save tilings.

R=enne@chromium.org, enne

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

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

[danakj, 2014-06-18 17:35:50]

Did this while trying to understand
https://code.google.com/p/chromium/issues/detail?id=384730. I don't think this
fixes anything but makes things a bit more clear.

[danakj, 2014-06-18 17:40:40]

Oh.. fun, this breaks a mask test, where we expect the mask layer to get a
tiling even tho it !CanHaveTilings? But it wouldn't have a tiling, something
suspicious...

[danakj, 2014-06-18 18:19:20]

Fixed the mask test!

[enne (OOO), 2014-06-18 23:08:02]

https://codereview.chromium.org/334953003/diff/40001/cc/layers/picture_layer_...
File cc/layers/picture_layer_impl.cc (left):

https://codereview.chromium.org/334953003/diff/40001/cc/layers/picture_layer_...
cc/layers/picture_layer_impl.cc:1291: if (!CanHaveTilings()) {
I kind of liked this code being in UpdateIdealScales, because then the caller
doesn't have to think about whether something can or cannot have tilings.  I
think you can still call it prior to early-outting and setting
should_update_tile_priorities=true.

[danakj, 2014-06-19 01:03:52]

https://codereview.chromium.org/334953003/diff/40001/cc/layers/picture_layer_...
File cc/layers/picture_layer_impl.cc (left):

https://codereview.chromium.org/334953003/diff/40001/cc/layers/picture_layer_...
cc/layers/picture_layer_impl.cc:1291: if (!CanHaveTilings()) {
On 2014/06/18 23:08:02, enne wrote:
> I kind of liked this code being in UpdateIdealScales, because then the caller
> doesn't have to think about whether something can or cannot have tilings.  I
> think you can still call it prior to early-outting and setting
> should_update_tile_priorities=true.

ok, i can leave it there. I liked the idea of moving it because
a) removed one check for CanHaveTilings() meaning we don't have this agreement
needed between UpdateTilePrios and UpdateIdealScales anymore regarding
CanHaveTilings()
b) no early out needed in UpdateIdealScales made it simpler in my mind.

If those things don't convince you, I'm uploading a CL without moving this, so
we can do that.

[enne (OOO), 2014-06-19 01:05:22]

I'd be ok with leaving it, but then DCHECK CanHaveTilings so this assumption is
more clear?

[danakj, 2014-06-19 01:06:44]

ok sure!


On Wed, Jun 18, 2014 at 6:05 PM, <enne@chromium.org> wrote:

> I'd be ok with leaving it, but then DCHECK CanHaveTilings so this
> assumption is
> more clear?
>
> https://codereview.chromium.org/334953003/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[danakj, 2014-06-19 01:07:36]

Done, PTAL

[enne (OOO), 2014-06-19 01:15:43]

Thanks, lgtm

[danakj, 2014-06-19 01:16:14]

The CQ bit was checked by danakj@chromium.org

[danakj, 2014-06-19 01:17:28]

Committed patchset #4 manually as r278219 (presubmit successful).

[jackhou1, 2014-06-19 03:59:31]

A revert of this CL has been created in
https://codereview.chromium.org/348523005/ by jackhou@chromium.org.

The reason for reverting is: This appears to have broken: 
CreateTilingsEvenIfTwinHasNone
ManageTilingsWithNoRecording

http://build.chromium.org/p/chromium.mac/waterfall?builder=Mac%2010.7%20Tests...

http://build.chromium.org/p/chromium.win/waterfall?builder=Win7%20Tests%20(db...

http://build.chromium.org/p/chromium.linux/builders/Linux%20Tests%20%28dbg%29....

[danakj, 2014-06-19 15:31:14]

The CQ bit was checked by danakj@chromium.org

[commit-bot: I haz the power, 2014-06-19 15:32:45]

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

[danakj, 2014-06-19 17:52:31]

Committed patchset #5 manually as r278413 (presubmit successful).