Integrate custom paint with the DocumentLifecycle

Integrate custom paint with the DocumentLifecycle

Instead of running paint requests during microtasks, we run them after updating
layout for the current frame. After draining all the paint requests, we then
drain whatever commits are available.

It's still possible that requestPaint callbacks can dirty style and layout
information, so we need to clean that information after servicing the requests.
Ideally we'd block these callbacks from dirtying style or layout information.

R=ojan@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/7b2dab6dffc725f87491f045933a58a58e467c84

[ojan, 2015-03-20 02:35:14]

lgtm

https://codereview.chromium.org/1027563002/diff/1/sky/engine/core/page/PageAn...
File sky/engine/core/page/PageAnimator.cpp (right):

https://codereview.chromium.org/1027563002/diff/1/sky/engine/core/page/PageAn...
sky/engine/core/page/PageAnimator.cpp:56:
view->updateLayoutAndStyleForPainting();
Is this call needed?

[abarth-chromium, 2015-03-20 02:54:52]

https://codereview.chromium.org/1027563002/diff/1/sky/engine/core/page/PageAn...
File sky/engine/core/page/PageAnimator.cpp (right):

https://codereview.chromium.org/1027563002/diff/1/sky/engine/core/page/PageAn...
sky/engine/core/page/PageAnimator.cpp:56:
view->updateLayoutAndStyleForPainting();
On 2015/03/20 at 02:35:14, ojan wrote:
> Is this call needed?

serviceRequests returns if there are more requests, but I can change it to
return true iff the queue is empty, which will let us remove this call and save
the second trip through updateLayoutAndStyleForPainting in the case of not
having any custom painting requests.

[ojan, 2015-03-20 03:38:02]

https://codereview.chromium.org/1027563002/diff/1/sky/engine/core/page/PageAn...
File sky/engine/core/page/PageAnimator.cpp (right):

https://codereview.chromium.org/1027563002/diff/1/sky/engine/core/page/PageAn...
sky/engine/core/page/PageAnimator.cpp:56:
view->updateLayoutAndStyleForPainting();
On 2015/03/20 at 02:54:51, abarth wrote:
> On 2015/03/20 at 02:35:14, ojan wrote:
> > Is this call needed?
> 
> serviceRequests returns if there are more requests, but I can change it to
return true iff the queue is empty, which will let us remove this call and save
the second trip through updateLayoutAndStyleForPainting in the case of not
having any custom painting requests.

Don't know if this is intentional, but if you hit the footgun in one of the
paint callbacks inside serviceRequests, then the size of the PaintingContext can
be stale in the following paint callbacks inside serviceRequests. To be 100%
correct, you'd have to call updateLayoutAndStyleForPainting after each callback
in serviceRequests and then create the PaintingContext.

So, I'm not sure the loop here makes sense. Or am I reading the code wrong?

FWIW, in the case of line 56 here, I wasn't worried about performance. I just
thought it was noop code. I see now that it's not because the serviced requests
can dirty style/layout.

[abarth-chromium, 2015-03-20 04:34:05]

On 2015/03/20 at 03:38:02, ojan wrote:
> Don't know if this is intentional, but if you hit the footgun in one of the
paint callbacks inside serviceRequests, then the size of the PaintingContext can
be stale in the following paint callbacks inside serviceRequests.

Yeah, I think we want to block dirtying style and layout state.

> To be 100% correct, you'd have to call updateLayoutAndStyleForPainting after
each callback in serviceRequests and then create the PaintingContext.

I'd prefer not to call updateLayout for each request...

> So, I'm not sure the loop here makes sense. Or am I reading the code wrong?

The loop is needed because someone might request to paint during a requestPaint
callback.  The behavior currently is that we completely drain the request queue
before advancing.

> FWIW, in the case of line 56 here, I wasn't worried about performance. I just
thought it was noop code. I see now that it's not because the serviced requests
can dirty style/layout.

Correct.

[abarth-chromium, 2015-03-20 04:58:20]

Committed patchset #2 (id:20001) manually as
7b2dab6dffc725f87491f045933a58a58e467c84 (presubmit successful).