cc: Fix tiling raster iterator to process all tiles correctly.

cc: Fix tiling raster iterator to process all tiles correctly.

This is a fix for the following situation: If we have no skewport,
then in operator++ we call advance phase after we process all visible
tiles, however because the skewport yields no tiles, we immediately
skip to the eventually bin, since the soon iterator construction
was in operator++ instead of advance phase.

The reason this passed unittests is that there was a second bug in
which we would process the eventually rect without ignoring the soon
rect, which caused the same tiles to show up.

This patch fixes both bugs. The first one is fixed by moving tiling
data iterator construction to be in advancephase in all cases. 
The second bug changes the iterator to iterate around the soon
rect instead of the visible rect.

R=reveman

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

[vmpstr, 2014-07-23 23:39:26]

PTAL

[reveman, 2014-07-24 12:50:17]

https://codereview.chromium.org/413053004/diff/1/cc/resources/picture_layer_t...
File cc/resources/picture_layer_tiling.cc (right):

https://codereview.chromium.org/413053004/diff/1/cc/resources/picture_layer_t...
cc/resources/picture_layer_tiling.cc:890: }
I find this code a bit hard to understand. It might just be that I'm not very
familiar with the phases we go through in this iterator. Could you add a comment
above the loop that lists each phase and then some comments next to the code in
the loop that makes us enter a new phase and why?

[reveman, 2014-07-24 13:12:23]

https://codereview.chromium.org/413053004/diff/1/cc/resources/picture_layer_t...
File cc/resources/picture_layer_tiling.cc (right):

https://codereview.chromium.org/413053004/diff/1/cc/resources/picture_layer_t...
cc/resources/picture_layer_tiling.cc:890: }
On 2014/07/24 12:50:16, reveman wrote:
> I find this code a bit hard to understand. It might just be that I'm not very
> familiar with the phases we go through in this iterator. Could you add a
comment
> above the loop that lists each phase and then some comments next to the code
in
> the loop that makes us enter a new phase and why?

See my comment on the other patch. Maybe an enum for the phase can clean this up
and remove the need for comments.

[vmpstr, 2014-07-24 16:33:14]

PTAL

https://codereview.chromium.org/413053004/diff/1/cc/resources/picture_layer_t...
File cc/resources/picture_layer_tiling.cc (right):

https://codereview.chromium.org/413053004/diff/1/cc/resources/picture_layer_t...
cc/resources/picture_layer_tiling.cc:890: }
On 2014/07/24 13:12:23, reveman wrote:
> On 2014/07/24 12:50:16, reveman wrote:
> > I find this code a bit hard to understand. It might just be that I'm not
very
> > familiar with the phases we go through in this iterator. Could you add a
> comment
> > above the loop that lists each phase and then some comments next to the code
> in
> > the loop that makes us enter a new phase and why?
> 
> See my comment on the other patch. Maybe an enum for the phase can clean this
up
> and remove the need for comments.

Done.

[reveman, 2014-07-24 16:38:49]

thanks! made it much easier for me to understand the code.

lgtm with nit

https://codereview.chromium.org/413053004/diff/20001/cc/resources/picture_lay...
File cc/resources/picture_layer_tiling.cc (right):

https://codereview.chromium.org/413053004/diff/20001/cc/resources/picture_lay...
cc/resources/picture_layer_tiling.cc:874: if (phase_ == SOON_BORDER_RECT) {
nit: I would prefer a switch statement with all phases explicitly listed.

[vmpstr, 2014-07-24 16:44:25]

https://codereview.chromium.org/413053004/diff/20001/cc/resources/picture_lay...
File cc/resources/picture_layer_tiling.cc (right):

https://codereview.chromium.org/413053004/diff/20001/cc/resources/picture_lay...
cc/resources/picture_layer_tiling.cc:874: if (phase_ == SOON_BORDER_RECT) {
On 2014/07/24 16:38:49, reveman wrote:
> nit: I would prefer a switch statement with all phases explicitly listed.

Can I push this nit off into the optimization patch? I'm already doing it there.
I've uploaded it, so you can take a look.

[reveman, 2014-07-24 16:54:19]

https://codereview.chromium.org/413053004/diff/20001/cc/resources/picture_lay...
File cc/resources/picture_layer_tiling.cc (right):

https://codereview.chromium.org/413053004/diff/20001/cc/resources/picture_lay...
cc/resources/picture_layer_tiling.cc:874: if (phase_ == SOON_BORDER_RECT) {
On 2014/07/24 16:44:25, vmpstr wrote:
> On 2014/07/24 16:38:49, reveman wrote:
> > nit: I would prefer a switch statement with all phases explicitly listed.
> 
> Can I push this nit off into the optimization patch? I'm already doing it
there.
> I've uploaded it, so you can take a look.

sure, part of that patch is fine.

[vmpstr, 2014-07-24 21:50:12]

The CQ bit was checked by vmpstr@chromium.org

[commit-bot: I haz the power, 2014-07-24 21:51:22]

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

[commit-bot: I haz the power, 2014-07-25 01:29:10]

Change committed as 285448