sync compositor: Keep draw after ComputeScroll synchronous

sync compositor: Keep draw after ComputeScroll synchronous

Async draw will asynchronously send scroll updates that happen between
BeginFrame (which is synchronous) and Draw to the UI thread. Although
the frame is synchronously delivered to the render thread with the frame
future.

The problem with any such scroll is that any children of webview is
scrolled according to the position of the webview will appear to be out
of sync with webview content itself.

THere are two sources of such scrolls:
1) ComputeScroll, which only happen during fling animations
2) Blink main scroll being committed/activated.

This CL fixes 1) by making draws after ComputeScroll always synchronous.
Could reduce this further by checking of webview actually has children.
But this CL is good enough to ship.

Case 2) is generally rarer. In theory, commits should not happen between
BeginFrame and Draw, but webview can't make this guarantee due to other
constraints.

BUG=636164
Committed: https://crrev.com/0be8dd04aef3283ea273061512f53a0f7df9bc69
Cr-Commit-Position: refs/heads/master@{#428734}

[boliu, 2016-10-29 22:47:33]

The CQ bit was checked by boliu@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-29 22:47:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[boliu, 2016-10-29 23:00:18]

Description was changed from

==========
sync compositor: Keep draw after ComputeScroll synchronous

Async draw will asynchronously send scroll updates that happen between
BeginFrame (which is synchronous) and Draw to the UI thread. Although
the frame is synchronous delivered to the render thread with the frame
future.

The problem with any such scroll is that any children of webview is
scrolled according to the position of the webview will appear to be out
of sync with webview content itself.

THere are two sources of such scrolls:
1) ComputeScroll, which only happen during fling animations
2) Blink main scroll being committed/activated.

This CL fixes 1) by making draws after ComputeScroll always synchronous.
Could reduce this future by checking of webview actually has children.
But this CL is good enough to ship.

Case 2) is generally rarer. In theory, commits should not happen between
BeginFrame and Draw, but webview can't make this guarantee due to other
constraints.

BUG=636164
==========

to

==========
sync compositor: Keep draw after ComputeScroll synchronous

Async draw will asynchronously send scroll updates that happen between
BeginFrame (which is synchronous) and Draw to the UI thread. Although
the frame is synchronously delivered to the render thread with the frame
future.

The problem with any such scroll is that any children of webview is
scrolled according to the position of the webview will appear to be out
of sync with webview content itself.

THere are two sources of such scrolls:
1) ComputeScroll, which only happen during fling animations
2) Blink main scroll being committed/activated.

This CL fixes 1) by making draws after ComputeScroll always synchronous.
Could reduce this future by checking of webview actually has children.
But this CL is good enough to ship.

Case 2) is generally rarer. In theory, commits should not happen between
BeginFrame and Draw, but webview can't make this guarantee due to other
constraints.

BUG=636164
==========

[commit-bot: I haz the power, 2016-10-29 23:22:17]

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

[commit-bot: I haz the power, 2016-10-29 23:22:18]

Dry run: This issue passed the CQ dry run.

[boliu, 2016-10-29 23:30:03]

Description was changed from

==========
sync compositor: Keep draw after ComputeScroll synchronous

Async draw will asynchronously send scroll updates that happen between
BeginFrame (which is synchronous) and Draw to the UI thread. Although
the frame is synchronously delivered to the render thread with the frame
future.

The problem with any such scroll is that any children of webview is
scrolled according to the position of the webview will appear to be out
of sync with webview content itself.

THere are two sources of such scrolls:
1) ComputeScroll, which only happen during fling animations
2) Blink main scroll being committed/activated.

This CL fixes 1) by making draws after ComputeScroll always synchronous.
Could reduce this future by checking of webview actually has children.
But this CL is good enough to ship.

Case 2) is generally rarer. In theory, commits should not happen between
BeginFrame and Draw, but webview can't make this guarantee due to other
constraints.

BUG=636164
==========

to

==========
sync compositor: Keep draw after ComputeScroll synchronous

Async draw will asynchronously send scroll updates that happen between
BeginFrame (which is synchronous) and Draw to the UI thread. Although
the frame is synchronously delivered to the render thread with the frame
future.

The problem with any such scroll is that any children of webview is
scrolled according to the position of the webview will appear to be out
of sync with webview content itself.

THere are two sources of such scrolls:
1) ComputeScroll, which only happen during fling animations
2) Blink main scroll being committed/activated.

This CL fixes 1) by making draws after ComputeScroll always synchronous.
Could reduce this further by checking of webview actually has children.
But this CL is good enough to ship.

Case 2) is generally rarer. In theory, commits should not happen between
BeginFrame and Draw, but webview can't make this guarantee due to other
constraints.

BUG=636164
==========

[boliu, 2016-10-30 01:14:32]

boliu@chromium.org changed reviewers:
+ tobiasjs@chromium.org

[boliu, 2016-10-30 01:14:33]

for monday..

[Tobias Sargeant, 2016-10-31 10:15:25]

On 2016/10/30 01:14:33, boliu wrote:
> for monday..

Couldn't BVR set allow_async_draw_ = false in its OnComputeScroll?

[boliu, 2016-10-31 13:46:00]

On 2016/10/31 10:15:25, Tobias Sargeant wrote:
> On 2016/10/30 01:14:33, boliu wrote:
> > for monday..
> 
> Couldn't BVR set allow_async_draw_ = false in its OnComputeScroll?

practically, no, because BVR doesn't have need_animate_scroll_

then stylistically, these are different things. bvr's one is to workaround the
fact that webview doesn't synchronize binding initialization, and should
eventually be removed. this one is for correctness, and need to stay

[Tobias Sargeant, 2016-10-31 15:24:05]

On 2016/10/31 13:46:00, boliu wrote:
> On 2016/10/31 10:15:25, Tobias Sargeant wrote:
> > On 2016/10/30 01:14:33, boliu wrote:
> > > for monday..
> > 
> > Couldn't BVR set allow_async_draw_ = false in its OnComputeScroll?
> 
> practically, no, because BVR doesn't have need_animate_scroll_
> 
> then stylistically, these are different things. bvr's one is to workaround the
> fact that webview doesn't synchronize binding initialization, and should
> eventually be removed. this one is for correctness, and need to stay

I guess this LGTM, but it seems somewhat strange that BVR requests an async
draw, and then SCH overrides that decision to turn an async draw into a sync
one.

[boliu, 2016-10-31 15:51:45]

On 2016/10/31 15:24:05, Tobias Sargeant wrote:
> On 2016/10/31 13:46:00, boliu wrote:
> > On 2016/10/31 10:15:25, Tobias Sargeant wrote:
> > > On 2016/10/30 01:14:33, boliu wrote:
> > > > for monday..
> > > 
> > > Couldn't BVR set allow_async_draw_ = false in its OnComputeScroll?
> > 
> > practically, no, because BVR doesn't have need_animate_scroll_
> > 
> > then stylistically, these are different things. bvr's one is to workaround
the
> > fact that webview doesn't synchronize binding initialization, and should
> > eventually be removed. this one is for correctness, and need to stay
> 
> I guess this LGTM, but it seems somewhat strange that BVR requests an async
> draw, and then SCH overrides that decision to turn an async draw into a sync
> one.

might make more sense once DemandDrawHw is just the async path, and there would
be no way for BVR to call the synchronous path anymore

then the allow_async_draw_ thing will need to be addressed in some other way..

[boliu, 2016-10-31 15:55:03]

The CQ bit was checked by boliu@chromium.org

[commit-bot: I haz the power, 2016-10-31 15:55:23]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Tobias Sargeant, 2016-10-31 15:56:37]

On 2016/10/31 15:51:45, boliu wrote:
> On 2016/10/31 15:24:05, Tobias Sargeant wrote:
> > On 2016/10/31 13:46:00, boliu wrote:
> > > On 2016/10/31 10:15:25, Tobias Sargeant wrote:
> > > > On 2016/10/30 01:14:33, boliu wrote:
> > > > > for monday..
> > > > 
> > > > Couldn't BVR set allow_async_draw_ = false in its OnComputeScroll?
> > > 
> > > practically, no, because BVR doesn't have need_animate_scroll_
> > > 
> > > then stylistically, these are different things. bvr's one is to workaround
> the
> > > fact that webview doesn't synchronize binding initialization, and should
> > > eventually be removed. this one is for correctness, and need to stay
> > 
> > I guess this LGTM, but it seems somewhat strange that BVR requests an async
> > draw, and then SCH overrides that decision to turn an async draw into a sync
> > one.
> 
> might make more sense once DemandDrawHw is just the async path, and there
would
> be no way for BVR to call the synchronous path anymore
> 
> then the allow_async_draw_ thing will need to be addressed in some other way..

WDYT about passing down allow_async_draw_ to a DemandDrawHw method that chooses
sync/async depending on that flag + local state, and either way returns a
future?

[boliu, 2016-10-31 16:01:39]

On 2016/10/31 15:56:37, Tobias Sargeant wrote:
> On 2016/10/31 15:51:45, boliu wrote:
> > On 2016/10/31 15:24:05, Tobias Sargeant wrote:
> > > On 2016/10/31 13:46:00, boliu wrote:
> > > > On 2016/10/31 10:15:25, Tobias Sargeant wrote:
> > > > > On 2016/10/30 01:14:33, boliu wrote:
> > > > > > for monday..
> > > > > 
> > > > > Couldn't BVR set allow_async_draw_ = false in its OnComputeScroll?
> > > > 
> > > > practically, no, because BVR doesn't have need_animate_scroll_
> > > > 
> > > > then stylistically, these are different things. bvr's one is to
workaround
> > the
> > > > fact that webview doesn't synchronize binding initialization, and should
> > > > eventually be removed. this one is for correctness, and need to stay
> > > 
> > > I guess this LGTM, but it seems somewhat strange that BVR requests an
async
> > > draw, and then SCH overrides that decision to turn an async draw into a
sync
> > > one.
> > 
> > might make more sense once DemandDrawHw is just the async path, and there
> would
> > be no way for BVR to call the synchronous path anymore
> > 
> > then the allow_async_draw_ thing will need to be addressed in some other
way..
> 
> WDYT about passing down allow_async_draw_ to a DemandDrawHw method that
chooses
> sync/async depending on that flag + local state, and either way returns a
> future?

That works, but I'd rather just remove allow_async_draw_ instead.. There's a
bunch of clean ups that I can do after async becomes default

[commit-bot: I haz the power, 2016-10-31 16:26:04]

Description was changed from

==========
sync compositor: Keep draw after ComputeScroll synchronous

Async draw will asynchronously send scroll updates that happen between
BeginFrame (which is synchronous) and Draw to the UI thread. Although
the frame is synchronously delivered to the render thread with the frame
future.

The problem with any such scroll is that any children of webview is
scrolled according to the position of the webview will appear to be out
of sync with webview content itself.

THere are two sources of such scrolls:
1) ComputeScroll, which only happen during fling animations
2) Blink main scroll being committed/activated.

This CL fixes 1) by making draws after ComputeScroll always synchronous.
Could reduce this further by checking of webview actually has children.
But this CL is good enough to ship.

Case 2) is generally rarer. In theory, commits should not happen between
BeginFrame and Draw, but webview can't make this guarantee due to other
constraints.

BUG=636164
==========

to

==========
sync compositor: Keep draw after ComputeScroll synchronous

Async draw will asynchronously send scroll updates that happen between
BeginFrame (which is synchronous) and Draw to the UI thread. Although
the frame is synchronously delivered to the render thread with the frame
future.

The problem with any such scroll is that any children of webview is
scrolled according to the position of the webview will appear to be out
of sync with webview content itself.

THere are two sources of such scrolls:
1) ComputeScroll, which only happen during fling animations
2) Blink main scroll being committed/activated.

This CL fixes 1) by making draws after ComputeScroll always synchronous.
Could reduce this further by checking of webview actually has children.
But this CL is good enough to ship.

Case 2) is generally rarer. In theory, commits should not happen between
BeginFrame and Draw, but webview can't make this guarantee due to other
constraints.

BUG=636164
==========

[commit-bot: I haz the power, 2016-10-31 16:26:06]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-10-31 16:45:49]

Description was changed from

==========
sync compositor: Keep draw after ComputeScroll synchronous

Async draw will asynchronously send scroll updates that happen between
BeginFrame (which is synchronous) and Draw to the UI thread. Although
the frame is synchronously delivered to the render thread with the frame
future.

The problem with any such scroll is that any children of webview is
scrolled according to the position of the webview will appear to be out
of sync with webview content itself.

THere are two sources of such scrolls:
1) ComputeScroll, which only happen during fling animations
2) Blink main scroll being committed/activated.

This CL fixes 1) by making draws after ComputeScroll always synchronous.
Could reduce this further by checking of webview actually has children.
But this CL is good enough to ship.

Case 2) is generally rarer. In theory, commits should not happen between
BeginFrame and Draw, but webview can't make this guarantee due to other
constraints.

BUG=636164
==========

to

==========
sync compositor: Keep draw after ComputeScroll synchronous

Async draw will asynchronously send scroll updates that happen between
BeginFrame (which is synchronous) and Draw to the UI thread. Although
the frame is synchronously delivered to the render thread with the frame
future.

The problem with any such scroll is that any children of webview is
scrolled according to the position of the webview will appear to be out
of sync with webview content itself.

THere are two sources of such scrolls:
1) ComputeScroll, which only happen during fling animations
2) Blink main scroll being committed/activated.

This CL fixes 1) by making draws after ComputeScroll always synchronous.
Could reduce this further by checking of webview actually has children.
But this CL is good enough to ship.

Case 2) is generally rarer. In theory, commits should not happen between
BeginFrame and Draw, but webview can't make this guarantee due to other
constraints.

BUG=636164

Committed: https://crrev.com/0be8dd04aef3283ea273061512f53a0f7df9bc69
Cr-Commit-Position: refs/heads/master@{#428734}
==========

[commit-bot: I haz the power, 2016-10-31 16:45:50]

Patchset 1 (id:??) landed as
https://crrev.com/0be8dd04aef3283ea273061512f53a0f7df9bc69
Cr-Commit-Position: refs/heads/master@{#428734}