Deleting all main-thread input handling code

[Alexandre Elias]

In today's architecture we need to implement input handling code twice, on both main and impl.  Now that we're making a push for every scroll gesture to be supported equally well on the main thread, there's starting to be quite a bit of code duplication.  In the long run, it seems like this is going to be a continuing burden: every subtle bug, we'll need to fix in two different places.

Rick and I agreed it would be cool if the impl thread were the one and only place where touch gestures are handled, and we could just delete all code for handling input on the main thread.  I think this is possible to do.  The basic idea is that Blink would receive input events as it does today, but then instead of handling them immediately, queue them back to CC for execution against the impl trees during the next commit (after the main thread blocks, but before pushing the tree).  There are two major problems with that plan:

1) Some things are scrollable which are not cc::Layers, in particular some scrollable subregions when LCD text is enabled (because they cannot have a separate backing).  Therefore, CC needs to be able to scroll anything, even if it doesn't map to a CC layer.  The Slimming Paint project already has a plan to introduce a "scrolling tree": this project would ultimately be blocked on that shipping universally.

2) The new DistributeScroll APIs.  To address this, I would rely on the fact that the main thread is blocked during commit, therefore we can both run JS and modify the CC impl scrolling state during that interval.  CC impl code would taking care of bubbling and railing, and make synchronous calls across the Blink API to distribute individual scroll deltas to JS.  This may sound a bit odd at first, but I see nothing fundamentally evil about this idea -- we just need to ban all use of thread-locals in Blink, which seems to be already almost the case from a quick grep.

Because issue 1) in particular depends on phase 2 of slimming paint, we can't start on this immediately, so I don't think we need to block the current work on improving the main thread.  But I wanted to send this out to get everyone thinking about how we could reach this design later this year.

--
Alex

[Steve Kobes]

Are you thinking only of touch / gesture / wheel input, or would this also include other types of scroll-triggering input (keyboard, scrollbar dragging, etc.) that cc has never handled?

If the goal is just to avoid code duplication, it seems like that could be achieved without sending the event to a different thread.  Couldn't Blink and CC have a shared dependency on the input logic?

[Alexandre Elias]

On Mon, Feb 23, 2015 at 6:11 PM, Steve Kobes <sko...@chromium.org> wrote:

Are you thinking only of touch / gesture / wheel input, or would this also include other types of scroll-triggering input (keyboard, scrollbar dragging, etc.) that cc has never handled?

Mostly thinking of touch scroll/pinch.  Blink will still have the ability to set the scroll offset itself using the current codepaths, so we're not required to move over stuff like scrollbar dragging if we don't want to (although maybe we do for performance reasons!).  It may also be interesting to move over all hit-test related input events like mouse clicks, to avoid code duplication and races in that part of the logic.  

 

If the goal is just to avoid code duplication, it seems like that could be achieved without sending the event to a different thread.  Couldn't Blink and CC have a shared dependency on the input logic?

We did that for fling curves, and it ended up being a real mess of abstractions upon abstractions that also doesn't share quite as much code as we would like, and I find code duplication preferable to what that ended up as.  Stuff like bubbling would be even harder to code-share.  I think an approach that just works on one type of data structure in one place would be much cleaner.

[Timothy Dresser]

We already need to add the right layer of abstraction for sharing bubbling and scrolling logic in order to implement scroll customization.

We can't have "one type of data structure in one place," as scrolling can be implemented in main thread JS, UIWorker JS, or in CC, at minimum.

There's a good chance that moving the input handling logic completely off of main is the right thing to do, but if we believe that it's possible to implement scroll customization in a powerful and non-messy way, we can't really argue that we there's no clean abstraction around scrolling logic. 

[Alexandre Elias]

On Tue, Feb 24, 2015 at 6:35 AM, Timothy Dresser <tdre...@google.com> wrote:

We already need to add the right layer of abstraction for sharing bubbling and scrolling logic in order to implement scroll customization.

I think you have a point with respect to bubbling and railing.  But there's also top controls hiding, overscroll glow, overscroll rubberbanding, native pull-to-refresh, pinch zoom, and fling.  The implementations of that stuff aren't directly abstracted by the scroll customization, and putting them in a shared library called in by both Blink and CC would end up creating a lot of additional abstract base class and delegate cruft that isn't otherwise useful.

We can't have "one type of data structure in one place," as scrolling can be implemented in main thread JS, UIWorker JS, or in CC, at minimum.

In my proposal, all of those would operate on one single instance of a scrolling tree data structure: what would be different would be only the code deciding what to do to it.

[Timothy Dresser]

You're right, there are a lot of things that won't be abstracted by scroll customization.
Just to add to the list, computing the scroll chain is also not handled by scroll customization.

This idea sounds worth pursuing to me.

[Adrienne Walker]

2015-02-23 17:43 GMT-08:00 Alexandre Elias <ael...@google.com>:

> 2) The new DistributeScroll APIs. To address this, I would rely on the fact
> that the main thread is blocked during commit, therefore we can both run JS
> and modify the CC impl scrolling state during that interval. CC impl code
> would taking care of bubbling and railing, and make synchronous calls across
> the Blink API to distribute individual scroll deltas to JS. This may sound
> a bit odd at first, but I see nothing fundamentally evil about this idea --
> we just need to ban all use of thread-locals in Blink, which seems to be
> already almost the case from a quick grep.

I think this is the part that is most unclear to me. If I look at the
larger picture of things that Blink and cc duplicate, I see scrolling,
animation, and hit testing.

Even if I just look at scrolling, it's not clear to me how that code
could only run during commit. Are you implying that the commit needs
to block before running main thread raf and animations (losing some
parallelism). How do you handle Javascript scrolling?

The other unclear part is that you mention that you want a single
instance of this data, but you also mention "making synchronous calls
to distribute scroll deltas". Is the idea that this new data
structure would be authoritative about scrolls and Blink would just be
told what its scroll positions are?

Why do you feel so strongly about one instance of the data vs some
shared (more coarse) data structure that could be synchronized, but
operated on independently? For the hit testing model, I pictured that
both the main thread and the compositor thread have different
instances of the same display list type, and hit testing could operate
on that. I think that avoids the pitfalls of the animation curve
issues that you are worried about.

[Alexandre Elias]

On Wed, Feb 25, 2015 at 10:28 AM, Adrienne Walker <en...@chromium.org> wrote:

2015-02-23 17:43 GMT-08:00 Alexandre Elias <ael...@google.com>:

> 2) The new DistributeScroll APIs.  To address this, I would rely on the fact
> that the main thread is blocked during commit, therefore we can both run JS
> and modify the CC impl scrolling state during that interval.  CC impl code
> would taking care of bubbling and railing, and make synchronous calls across
> the Blink API to distribute individual scroll deltas to JS.  This may sound
> a bit odd at first, but I see nothing fundamentally evil about this idea --
> we just need to ban all use of thread-locals in Blink, which seems to be
> already almost the case from a quick grep.

I think this is the part that is most unclear to me.  If I look at the
larger picture of things that Blink and cc duplicate, I see scrolling,
animation, and hit testing.

Even if I just look at scrolling, it's not clear to me how that code
could only run during commit.  Are you implying that the commit needs
to block before running main thread raf and animations (losing some
parallelism).  How do you handle Javascript scrolling?

Javascript scrolls would still work as they do today.  It feels wrong for read-modify-write operations within JS to block on commits.  So left implicit in my proposal that we maintain the main-thread mirror instance of the state that can be written to by Javascript, with the same SyncedProperty reconciliation that exists today.  So we would only lose some parallelism in the case of scroll gestures (where I would argue it's not useful).

The other unclear part is that you mention that you want a single
instance of this data, but you also mention "making synchronous calls
to distribute scroll deltas".  Is the idea that this new data
structure would be authoritative about scrolls and Blink would just be
told what its scroll positions are?

Why do you feel so strongly about one instance of the data vs some
shared (more coarse) data structure that could be synchronized, but
operated on independently?

It's because various features like overscroll glow, top controls hiding and scroll bubbling have additional subtle mid-gesture state involved.  To have a main-thread mirror instance, we'd need to pull in the transitive closure of all the state that's touched by scroll gestures into the scrolling tree and come up with a story for synchronization of that stuff as well.  For example, if a scroll gesture were to happen partly on the main thread and partly on the impl thread (a situation that probably ends up created by the timeout mechanism we're planning), we'll currently see buggy behavior around:

- top_controls_scroll_begin_offset_

- CurrentlyScrollingLayer

- should_bubble_scrolls_

- did_lock_scrolling_layer_

- accumulated_root_overscroll_

My proposal would make all those subtleties "just work" with no need for careful thought.

For the hit testing model, I pictured that
both the main thread and the compositor thread have different
instances of the same display list type, and hit testing could operate
on that.  I think that avoids the pitfalls of the animation curve
issues that you are worried about.

I agree it would be good for the main-thread mirror data structure that we do retain to be the same data structure class(es), and potentially make Blink call out to main-thread CC code for JS reading or writing scroll DOM properties.

  

[Jared Duke]

I'm all for unifying input code, particularly as it relates to scrolling.

However, I'm still not convinced that forcing all (user) scrolls through impl-thread data structures is strictly necessary. Yes, it might require some state synchronization to ensure consistent treatment on both threads, but I'm not sure that's an unreasonable burden when we already have to maintain parallel trees.

Several questions I have about this approach are

1. Will this prevent frame-synchronized JS scroll listener dispatch for a given scroll event? Or would (can?) we run the scroll listeners during the commit phase?
2. Would this prevent the feasibility of JS injecting a scroll event into the proposed scroll customization pipeline?
   
3. Are we OK blocking the compositor thread during a commit while we have to execute arbitrarily expensive scroll customization JS (or scroll listener JS)?

[Alexandre Elias]

Sorry for the delays in replying.

On Wed, Feb 25, 2015 at 4:55 PM, Jared Duke <jdd...@chromium.org> wrote:

I'm all for unifying input code, particularly as it relates to scrolling.

However, I'm still not convinced that forcing all (user) scrolls through impl-thread data structures is strictly necessary. Yes, it might require some state synchronization to ensure consistent treatment on both threads, but I'm not sure that's an unreasonable burden when we already have to maintain parallel trees.

Several questions I have about this approach are

1. Will this prevent frame-synchronized JS scroll listener dispatch for a given scroll event? Or would (can?) we run the scroll listeners during the commit phase?

We should be able to run them synchronously during (or immediately after) commit, that doesn't seem fundamentally different from the DistributeScroll API.

> 2. Would this prevent the feasibility of JS injecting a scroll event into the proposed scroll customization pipeline?

It shouldn't affect the feasibility of the proposals in that doc, that's what I was referring to in my point 2) in my first email.

> 3. Are we OK blocking the compositor thread during a commit while we have to execute arbitrarily expensive scroll customization JS (or scroll listener JS)?

Yes, on the premise that scrolling is by far the most important thing that's being made asynchronous on the compositor thread, and we gave up that asynchronicity already.  The only remaining incremental benefit would be to avoid janking CSS animations that happen to be running at the same time as the scroll, but that seems like a really small benefit.

[Jared Duke]

> 2. Would this prevent the feasibility of JS injecting a scroll event into the proposed scroll customization pipeline?
It shouldn't affect the feasibility of the proposals in that doc, that's what I was referring to in my point 2) in my first email.

Sorry, I didn't see in #2 where you addressed JS being able to, at any given time, inject a synthetic scroll event (with ScrollState) through the (potentially customized) scroll chain? This is somewhat different than JS scrolling just a single element, where all we have to do is sync the offset during commit. 

 So left implicit in my proposal that we maintain the main-thread mirror instance of the state that can be written to by Javascript, with the same SyncedProperty reconciliation that exists today.  So we would only lose some parallelism in the case of scroll gestures (where I would argue it's not useful).

For scroll chaining to work with a JS-injected scroll event, we'll need all the same machinery you're proposing we restrict to the impl thread.

> 3. Are we OK blocking the compositor thread during a commit while we have to execute arbitrarily expensive scroll customization JS (or scroll listener JS)?
Yes, on the premise that scrolling is by far the most important thing that's being made asynchronous on the compositor thread, and we gave up that asynchronicity already.  The only remaining incremental benefit would be to avoid janking CSS animations that happen to be running at the same time as the scroll, but that seems like a really small benefit.

The plan is to gate sync scroll rollout with a graceful timeout API. It seems like it would get quite messy to abort a commit when we find that the custom JS scroll code is taking too long. Is aborting even feasible if that JS relies on impl-thread only code/data?

[Timothy Dresser]

The scroll customization document currently doesn't currently mention manually constructing a ScrollState object and passing it to a distributeScroll method, but it is something we'd like to remain possible.

[Alexandre Elias]

OK, Jared explained to me offline that he meant synchronously calling the default implementation of distributeScroll from some regular JS outside of an event.  That does seem like something we couldn't do with this scheme.  Also, another thing that came up is that if we want to time out time-consuming JS and handle the event on the impl thread instead, that makes no sense if the impl thread is already blocked.

Given those problems, I'm warming to the idea of having two instances of the scrolling data structure after all.  We'll need to be very systematic about collecting and syncing the in-gesture state I mentioned, but it seems doable.  That world would still be a big improvement on the status quo of completely redundant code in different repos on different classes.

--
Alex