ViewAndroid Hit Testing - Invitation to edit

15 views
Skip to first unread message

Jinsuk Kim (via Google Docs)

unread,
Jan 31, 2017, 6:08:41 PM1/31/17
to slimmin...@chromium.org
Jinsuk Kim has invited you to edit the following document:
Sender's profile photoA brief summary on how the hit testing for ViewAndroid is implemented. This is to clarify ideas/assumption made in it. Any comments/corrections will be welcome. https://codereview.chromium.org/2645353004/
Google Docs: Create and edit documents online.
Google Inc. 1600 Amphitheatre Parkway, Mountain View, CA 94043, USA
You have received this email because someone shared a document with you from Google Docs.		Logo for Google Docs

Bo Liu

unread,
Jan 31, 2017, 6:26:31 PM1/31/17
to Jinsuk Kim, Slimming Clank
Let's just use this tread to discuss.

Jinsuk is starting to implement hit testing in ViewAndroid, and I don't feel confident in reviewing some of the design decisions. Examples of things that I feel are design decisions:
* the position and size of child is stored in the child object
* Children have "z order", ie order events are delivered
* how should z order be expressed? right now it's just order of addChild
* event is delivered to children before parent
* match parent is implemented as a special case for dimension

Anyone has strong feelings for anything in particular here? Please to reply here or jump on the code review.

--
You received this message because you are subscribed to the Google Groups "Slimming Clank" group.
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clank+unsubscribe@chromium.org.
To post to this group, send email to slimmin...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/slimming-clank/001a114d72344da70005476c0314%40google.com.

Jinsuk Kim

unread,
Jan 31, 2017, 7:19:13 PM1/31/17
to bo...@chromium.org, Slimming Clank
As mentioned in other thread, I'm thinking of keeping all the layout + event filter logic doing their job. This makes overall hit testing at ViewAndroid much simpler than what Java Android view hierarchy does. 

On Wed, Feb 1, 2017 at 8:25 AM, Bo Liu <bo...@chromium.org> wrote:
Let's just use this tread to discuss.

Jinsuk is starting to implement hit testing in ViewAndroid, and I don't feel confident in reviewing some of the design decisions. Examples of things that I feel are design decisions:
* the position and size of child is stored in the child object
* Children have "z order", ie order events are delivered
* how should z order be expressed? right now it's just order of addChild

​Chrome gets the right content to route events to from current tabs' view. Now native also needs to keep track of the ViewAndroid that matches with it. The ViewAndroids are being managed with std::list, which I think is enough for the purpose. I'm handling it by adding a new child at the front of the list, and calling ViewAndroid::MoveToFront if users switch the current tab.  Tested it in https://crrev.com/2595263002 and it seems to work as expected though there can be corner cases I'm not aware of. 

Alexandre Elias

unread,
Jan 31, 2017, 8:11:36 PM1/31/17
to bo...@chromium.org, Jinsuk Kim, Slimming Clank
On Tue, Jan 31, 2017 at 3:25 PM, Bo Liu <bo...@chromium.org> wrote:
Let's just use this tread to discuss.

Jinsuk is starting to implement hit testing in ViewAndroid, and I don't feel confident in reviewing some of the design decisions. Examples of things that I feel are design decisions:
* the position and size of child is stored in the child object
* Children have "z order", ie order events are delivered
* how should z order be expressed? right now it's just order of addChild
* event is delivered to children before parent
* match parent is implemented as a special case for dimension

All those decisions sound OK to me.  They seem similar enough to other view frameworks I'm familiar with.
 

Anyone has strong feelings for anything in particular here? Please to reply here or jump on the code review.

I'm a bit surprised there are no use cases for having views overlapping transparently or side-by-side.  Isn't the contextual search bar at the bottom a C++-based view thing that would require at least hit testing for x/y position instead of blindly passing to the z-topmost view?
 

On Tue, Jan 31, 2017 at 3:08 PM, 'Jinsuk Kim (via Google Docs)' via Slimming Clank <slimmin...@chromium.org> wrote:
Jinsuk Kim has invited you to edit the following document:
Sender's profile photoA brief summary on how the hit testing for ViewAndroid is implemented. This is to clarify ideas/assumption made in it. Any comments/corrections will be welcome. https://codereview.chromium.org/2645353004/
Google Docs: Create and edit documents online.
Google Inc. 1600 Amphitheatre Parkway, Mountain View, CA 94043, USA
You have received this email because someone shared a document with you from Google Docs.		Logo for Google Docs

--
You received this message because you are subscribed to the Google Groups "Slimming Clank" group.
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clank+unsubscribe@chromium.org.
To post to this group, send email to slimmin...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/slimming-clank/001a114d72344da70005476c0314%40google.com.

--
You received this message because you are subscribed to the Google Groups "Slimming Clank" group.
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clank+unsubscribe@chromium.org.
To post to this group, send email to slimmin...@chromium.org.

Jinsuk Kim

unread,
Jan 31, 2017, 8:44:52 PM1/31/17
to Alexandre Elias, bo...@chromium.org, Slimming Clank
On Wed, Feb 1, 2017 at 10:11 AM, Alexandre Elias <ael...@chromium.org> wrote:
On Tue, Jan 31, 2017 at 3:25 PM, Bo Liu <bo...@chromium.org> wrote:
Let's just use this tread to discuss.

Jinsuk is starting to implement hit testing in ViewAndroid, and I don't feel confident in reviewing some of the design decisions. Examples of things that I feel are design decisions:
* the position and size of child is stored in the child object
* Children have "z order", ie order events are delivered
* how should z order be expressed? right now it's just order of addChild
* event is delivered to children before parent
* match parent is implemented as a special case for dimension

All those decisions sound OK to me.  They seem similar enough to other view frameworks I'm familiar with.
 

Anyone has strong feelings for anything in particular here? Please to reply here or jump on the code review.

I'm a bit surprised there are no use cases for having views overlapping transparently or side-by-side.  Isn't the contextual search bar at the bottom a C++-based view thing that would require at least hit testing for x/y position instead of blindly passing to the z-topmost view?

​​I implemented hit testing for x/y position in case a certain embedder asks native ViewAndroid to figure out which component to route the event to, but at least Chrome doesn't do it that way. Contextual search for instance is handled such that events on its top bar (used to slide it up/down) + the area outside the panel are all handled by Layout/EventFilter and consumed at Java layer, passing only those bound to content view down to native.  

It may be possible to just pass everything to ViewAndroid and let it route from there, passing some back to Java via delegate interfaces or forwarding them to WCVA/RWHVA. I believe that's what David outlined in his first response to the refactoring.  It would however ask for substantial changes in how layout manager and event filter works. My goal is a bit humble - removing CVC first. I'm hoping my change makes the overall structure closer to that, and eventually looking into that direction with the help of others in the future.

David Trainor

unread,
Feb 1, 2017, 1:18:15 AM2/1/17
to Jinsuk Kim, bo...@chromium.org, mdj...@chromium.org, Slimming Clank, khusha...@chromium.org
Thanks for the doc Jinsuk!

As an FYI for the future, in docs you can click on comments, notifications, and select "All" to be notified of all comments on a doc you didn't author (thanks to nyquist@ for figuring this out).

On Tue, Jan 31, 2017 at 4:19 PM 'Jinsuk Kim' via Slimming Clank <slimmin...@chromium.org> wrote:
As mentioned in other thread, I'm thinking of keeping all the layout + event filter logic doing their job. This makes overall hit testing at ViewAndroid much simpler than what Java Android view hierarchy does. 

I agree that it isn't in the scope of removing CVC, but I've been talking with +mdj...@chromium.org and +khush...@chromium.org about what a more comprehensive front end refactor of the Layout system with ViewAndroids would look like.  It would be great to build on your work here and see if we can use it for more of the front end.

Re complexity, it looks like we'll already have ViewAndroid bounds hit testing and Z ordering.  The hit testing logic we'd need for the rest of the UI isn't much more complex.  I think the main missing component is gesture persistence.  Once a ViewAndroid starts getting a touch gesture it gets all the events for it (we probably want this anyway or we'll get DCHECK failures in the gesture detector code in RWHVA).

If we don't want to (or aren't willing to) build out ViewAndroid to use it for more of our UI, I don't really see the point of building a hierarchy concept at all.  If the hierarchy is always always Parent -> Child (WebContents) -> Child (RWHV), the sizes always match, and we aren't automatically hooking up the CC layers it feels like this is overkill (I do get that you could have two RWHV attached for a short period of time, but still!).

To Alex's earlier point about contextual search and the rest of the UI, a lot of this stuff lets us solve problems like the contextual search bar hit testing just by virtue of building out the ViewAndroid hierarchy for display.  Right now we have to build a native CC layer tree, but still have these other classes for event filters and hit testing in Java to figure out where the user clicked.  So we're completely decoupling and duplicating logic for display and hit testing even though the concepts seem pretty tightly intertwined.


On Wed, Feb 1, 2017 at 8:25 AM, Bo Liu <bo...@chromium.org> wrote:
Let's just use this tread to discuss.

Jinsuk is starting to implement hit testing in ViewAndroid, and I don't feel confident in reviewing some of the design decisions. Examples of things that I feel are design decisions:
* the position and size of child is stored in the child object
* Children have "z order", ie order events are delivered
* how should z order be expressed? right now it's just order of addChild

This is a good approach.  It's easy for people setting up ViewAndroid hierarchies to understand the concept.  I think the important thing that we want to guarantee is that the hit testing order matches the visual stacking of the children on the screen.  But (and this is something I'd like to talk about more in another thread/meeting) while ViewAndroids can hold a CC layer, it looks like nothing hooks them up in a hierarchy, so the visual ordering of the hierarchy doesn't really mean anything yet.

 

​Chrome gets the right content to route events to from current tabs' view. Now native also needs to keep track of the ViewAndroid that matches with it. The ViewAndroids are being managed with std::list, which I think is enough for the purpose. I'm handling it by adding a new child at the front of the list, and calling ViewAndroid::MoveToFront if users switch the current tab.  Tested it in https://crrev.com/2595263002 and it seems to work as expected though there can be corner cases I'm not aware of. 

I think adding to the back by default is more intuitive to me.  That's how most hierarchy/list APIs work.  If you added a "addChildAt" method that people could optionally use that would fit with the tree/list API as well.

 ​
* event is delivered to children before parent
 
The way Android gets around this is by giving each parent a chance to steal the event first via another delegate method (onInterceptTouchEvent).  If we need this we can add it to the API. 

* match parent is implemented as a special case for dimension
ViewAndroid.LayoutParams incoming!?!?

Anyone has strong feelings for anything in particular here? Please to reply here or jump on the code review.
On Tue, Jan 31, 2017 at 3:08 PM, 'Jinsuk Kim (via Google Docs)' via Slimming Clank <slimmin...@chromium.org> wrote:
Jinsuk Kim has invited you to edit the following document:
Sender's profile photoA brief summary on how the hit testing for ViewAndroid is implemented. This is to clarify ideas/assumption made in it. Any comments/corrections will be welcome. https://codereview.chromium.org/2645353004/
Google Docs: Create and edit documents online.
Google Inc. 1600 Amphitheatre Parkway, Mountain View, CA 94043, USA
You have received this email because someone shared a document with you from Google Docs.		Logo for Google Docs

--
You received this message because you are subscribed to the Google Groups "Slimming Clank" group.
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clan...@chromium.org.

--
You received this message because you are subscribed to the Google Groups "Slimming Clank" group.
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clan...@chromium.org.

To post to this group, send email to slimmin...@chromium.org.

Jinsuk Kim

unread,
Feb 1, 2017, 8:55:17 PM2/1/17
to David Trainor, bo...@chromium.org, mdj...@chromium.org, Slimming Clank, khusha...@chromium.org
On Wed, Feb 1, 2017 at 3:18 PM, David Trainor <dtra...@chromium.org> wrote:
Thanks for the doc Jinsuk!

As an FYI for the future, in docs you can click on comments, notifications, and select "All" to be notified of all comments on a doc you didn't author (thanks to nyquist@ for figuring this out).

On Tue, Jan 31, 2017 at 4:19 PM 'Jinsuk Kim' via Slimming Clank <slimmin...@chromium.org> wrote:
As mentioned in other thread, I'm thinking of keeping all the layout + event filter logic doing their job. This makes overall hit testing at ViewAndroid much simpler than what Java Android view hierarchy does. 

I agree that it isn't in the scope of removing CVC, but I've been talking with +mdj...@chromium.org and +khush...@chromium.org about what a more comprehensive front end refactor of the Layout system with ViewAndroids would look like.  It would be great to build on your work here and see if we can use it for more of the front end.


​It's nice to know more comprehensive refactoring is in consideration.  Let us keep in sync to make sure all the refactoring efforts are aligned. For those who haven't read or might have forgotten, my work is largely based on this doc written by sievers@ about content layer refactoring.
​
Re complexity, it looks like we'll already have ViewAndroid bounds hit testing and Z ordering.  The hit testing logic we'd need for the rest of the UI isn't much more complex.  I think the main missing component is gesture persistence.  Once a ViewAndroid starts getting a touch gesture it gets all the events for it (we probably want this anyway or we'll get DCHECK failures in the gesture detector code in RWHVA).

If we don't want to (or aren't willing to) build out ViewAndroid to use it for more of our UI, I don't really see the point of building a hierarchy concept at all.  If the hierarchy is always always Parent -> Child (WebContents) -> Child (RWHV), the sizes always match, and we aren't automatically hooking up the CC layers it feels like this is overkill (I do get that you could have two RWHV attached for a short period of time, but still!).


​I'm still interested in how overall work can be done in an incremental way - or rather in a way that allows content layer / front end refactoring to proceed in parallel. I agree that having the hit testing logic in place in VA, though it won't be in immediate use, is debatable. I'd like to hear more about how the works items in the front end refactoring will be prioritized once it gets more concrete.
​
To Alex's earlier point about contextual search and the rest of the UI, a lot of this stuff lets us solve problems like the contextual search bar hit testing just by virtue of building out the ViewAndroid hierarchy for display.  Right now we have to build a native CC layer tree, but still have these other classes for event filters and hit testing in Java to figure out where the user clicked.  So we're completely decoupling and duplicating logic for display and hit testing even though the concepts seem pretty tightly intertwined.


​So far this has been Chrome-only logic. One thing to think about would be if/how this, 
as
​it comes down to the
 content layer
​,​
 will benefit the other embedders from the perspective of keeping content layer slim if possible. 
​
​
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clank+unsubscribe@chromium.org.

--
You received this message because you are subscribed to the Google Groups "Slimming Clank" group.
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clank+unsubscribe@chromium.org.

To post to this group, send email to slimmin...@chromium.org.

--
You received this message because you are subscribed to the Google Groups "Slimming Clank" group.
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clank+unsubscribe@chromium.org.

To post to this group, send email to slimmin...@chromium.org.

Bo Liu

unread,
Feb 5, 2017, 10:05:42 AM2/5/17
to Jinsuk Kim, David Trainor, mdj...@chromium.org, Slimming Clank, khusha...@chromium.org
I think ViewAndroid tree is absolutely built to support contextual search. What's less clear to me is the tab switcher will also be built out of ViewAndroid trees or not; tab switcher seems a lot more complex to me.

I think Dave brought up a good point that event order should match visual order, which will eventually be based on cc::Layer or whatever we come up wit, so maybe we should draw inspiration from cc::Layer.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/slimming-clank/CAC4m%2BbEZAo0JNRUSe0om5qvHFSrXnHgamzwaHDs7aJpHqyTdmg%40mail.gmail.com.

Jinsuk Kim

unread,
Feb 13, 2017, 1:11:35 AM2/13/17
to bo...@chromium.org, David Trainor, mdj...@chromium.org, Slimming Clank, khusha...@chromium.org
I've been experimenting one approach which I think is feasible - largely based on David's suggestion - and had got the initial version working, though I'm sure there will still be many corner cases I haven't run into yet. 

A short summary is that it works by adding a ViewAndroid that represents a Java view at the top of the VA list when you want the events to be handled by Java layer, such as tab switcher, overlay panel bar, new tab page, etc. 

CompositorViewHolder always intercepts touch events and route them to native via ViewRoot.  The children of ViewRoot are for WebContents and the one at the top is the target to consume them in most cases. But if there is a Java UI component that should get the events,  you create a VA and place it at the top via ViewRoot.addViewAndroid(). Its bounds should match the area on screen, and when matched in hit testing, it returns false back to Compositor.onTouchEvent so that the Java layer can consume them. No need to find the target in event filters any more, and EventTarget.CONTENT would have been already handled by the time flow reaches there. 

For tab switcher you create a ViewAndroid whose bounds covers the entire screen (except toolbar at the top), and same for new tab page (haven't test this yet). For overlay panel you define a horizontal rect whose bounds matches that of bar control and updates its position whenever animation that changes its state (peek/expanded...etc) finishes.  And remove the VA when it's not necessary any more (exiting tab switcher or overlay panel..)

It seems to be working as expected so far. Let me know if you see any potential problems in this approach. I'd appreciate them and will try to figure out how to address them - or give up and turn to looking into other ways  if I can't :P



To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/slimming-clank/CAPCJOUr%2BG4eziS1oRKpZUgfn-xEE%2BY9vZrexaLra42oMHdgZ7Q%40mail.gmail.com.

Bo Liu

unread,
Feb 13, 2017, 5:36:22 PM2/13/17
to Jinsuk Kim, David Trainor, mdj...@chromium.org, Slimming Clank, khusha...@chromium.org
No feedback.. just \o/

David Trainor

unread,
Feb 13, 2017, 6:48:37 PM2/13/17
to bo...@chromium.org, Jinsuk Kim, mdj...@chromium.org, Slimming Clank, khusha...@chromium.org
Cool!  Thanks for doing a lot of this investigation!  Are you able to route the touch event to the contextual search panel's ViewAndroid, or is that still giving you trouble?

I talked to Khushal and we had another idea that might work too.  This might be a super simple way to migrate the content layer over without worrying about the Chrome UI.  Let me know what you think:

Right now the main issue is passing touch events to a ViewAndroid without a Java class or a built out view hierarchy right?  What if we did this intermediate refactor to help you get rid of ContentViewCore without the other headaches of worrying about the LayoutManager:

- ViewRoot is basically a JNI bridge to a ViewAndroid.
- We make WebContents have a ViewRoot and we expose it in Java.
- We migrate all of the code that passes touch events to the ContentViewCore/ContentView to hit WebContents' ViewRoot.
- As we refactor the front end and we no longer need the Java hook for the ViewAndroid WebContents we simply make it a ViewAndroid and no longer a ViewRoot.

That would let us keep basically the same approaches we have now... although the only difference is we have more than one ViewRoot in the system.  I'm not sure if this is a big deal or not in the short term.

On Mon, Feb 13, 2017 at 2:36 PM Bo Liu <bo...@chromium.org> wrote:
No feedback.. just \o/

On Sun, Feb 12, 2017 at 10:11 PM, Jinsuk Kim <jins...@google.com> wrote:
I've been experimenting one approach which I think is feasible - largely based on David's suggestion - and had got the initial version working, though I'm sure there will still be many corner cases I haven't run into yet. 

A short summary is that it works by adding a ViewAndroid that represents a Java view at the top of the VA list when you want the events to be handled by Java layer, such as tab switcher, overlay panel bar, new tab page, etc. 
Just to clarify: so we always steal from Java and route back to Java to these views in the hierarchy? 

CompositorViewHolder always intercepts touch events and route them to native via ViewRoot.  The children of ViewRoot are for WebContents and the one at the top is the target to consume them in most cases. But if there is a Java UI component that should get the events,  you create a VA and place it at the top via ViewRoot.addViewAndroid(). Its bounds should match the area on screen, and when matched in hit testing, it returns false back to Compositor.onTouchEvent so that the Java layer can consume them. No need to find the target in event filters any more, and EventTarget.CONTENT would have been already handled by the time flow reaches there. 
Could you elaborate a bit on how this matches the event to the panel's WebContents and delegates it? 
​
​
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clan...@chromium.org.

--
You received this message because you are subscribed to the Google Groups "Slimming Clank" group.
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clan...@chromium.org.

To post to this group, send email to slimmin...@chromium.org.

--
You received this message because you are subscribed to the Google Groups "Slimming Clank" group.
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clan...@chromium.org.

To post to this group, send email to slimmin...@chromium.org.

--
You received this message because you are subscribed to the Google Groups "Slimming Clank" group.
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clan...@chromium.org.

To post to this group, send email to slimmin...@chromium.org.

--
You received this message because you are subscribed to the Google Groups "Slimming Clank" group.
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clan...@chromium.org.

To post to this group, send email to slimmin...@chromium.org.

--
You received this message because you are subscribed to the Google Groups "Slimming Clank" group.
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clan...@chromium.org.

To post to this group, send email to slimmin...@chromium.org.

Bo Liu

unread,
Feb 13, 2017, 6:58:22 PM2/13/17
to David Trainor, Jinsuk Kim, mdj...@chromium.org, Slimming Clank, khusha...@chromium.org
On Mon, Feb 13, 2017 at 3:48 PM, David Trainor <dtra...@chromium.org> wrote:
Cool!  Thanks for doing a lot of this investigation!  Are you able to route the touch event to the contextual search panel's ViewAndroid, or is that still giving you trouble?

I talked to Khushal and we had another idea that might work too.  This might be a super simple way to migrate the content layer over without worrying about the Chrome UI.  Let me know what you think:

Right now the main issue is passing touch events to a ViewAndroid without a Java class or a built out view hierarchy right?  What if we did this intermediate refactor to help you get rid of ContentViewCore without the other headaches of worrying about the LayoutManager:

- ViewRoot is basically a JNI bridge to a ViewAndroid.
- We make WebContents have a ViewRoot and we expose it in Java. 
- We migrate all of the code that passes touch events to the ContentViewCore/ContentView to hit WebContents' ViewRoot.
- As we refactor the front end and we no longer need the Java hook for the ViewAndroid WebContents we simply make it a ViewAndroid and no longer a ViewRoot.

That would let us keep basically the same approaches we have now... although the only difference is we have more than one ViewRoot in the system.  I'm not sure if this is a big deal or not in the short term.

Having multiple ViewRoot in the same tree doesn't make much sense, for all the reasons we've discussed previously in meetings. If we go with this idea, how about we start with having multiple ViewAndroid trees (one per WebContents for now), and gradually merge them into a single tree as refactor in chrome is done?
 

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

--
You received this message because you are subscribed to the Google Groups "Slimming Clank" group.
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clank+unsubscribe@chromium.org.

To post to this group, send email to slimmin...@chromium.org.

--
You received this message because you are subscribed to the Google Groups "Slimming Clank" group.
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clank+unsubscribe@chromium.org.

To post to this group, send email to slimmin...@chromium.org.

--
You received this message because you are subscribed to the Google Groups "Slimming Clank" group.
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clank+unsubscribe@chromium.org.

To post to this group, send email to slimmin...@chromium.org.

--
You received this message because you are subscribed to the Google Groups "Slimming Clank" group.
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clank+unsubscribe@chromium.org.

To post to this group, send email to slimmin...@chromium.org.

--
You received this message because you are subscribed to the Google Groups "Slimming Clank" group.
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clank+unsubscribe@chromium.org.

To post to this group, send email to slimmin...@chromium.org.

--
You received this message because you are subscribed to the Google Groups "Slimming Clank" group.
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clank+unsubscribe@chromium.org.

To post to this group, send email to slimmin...@chromium.org.

David Trainor

unread,
Feb 13, 2017, 7:02:00 PM2/13/17
to bo...@chromium.org, Jinsuk Kim, mdj...@chromium.org, Slimming Clank, khusha...@chromium.org
I would be fine with that.  I was thinking of a ViewRoot as a "ViewAndroid with a JNI bridge."  But if it's a bit more complex I'd be fine with a few tweaks to the approach.  But yeah having multiple trees/entry points was what I was thinking makes the most sense in the short term :).  Just let the Java code forward the event to the correct one as it does already, but change it to forward to the correct ViewAndroid instead of ContentView.

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

--
You received this message because you are subscribed to the Google Groups "Slimming Clank" group.
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clan...@chromium.org.

To post to this group, send email to slimmin...@chromium.org.

--
You received this message because you are subscribed to the Google Groups "Slimming Clank" group.
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clan...@chromium.org.

To post to this group, send email to slimmin...@chromium.org.

--
You received this message because you are subscribed to the Google Groups "Slimming Clank" group.
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clan...@chromium.org.

To post to this group, send email to slimmin...@chromium.org.

--
You received this message because you are subscribed to the Google Groups "Slimming Clank" group.
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clan...@chromium.org.

To post to this group, send email to slimmin...@chromium.org.

--
You received this message because you are subscribed to the Google Groups "Slimming Clank" group.
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clan...@chromium.org.

To post to this group, send email to slimmin...@chromium.org.

--
You received this message because you are subscribed to the Google Groups "Slimming Clank" group.
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clan...@chromium.org.

To post to this group, send email to slimmin...@chromium.org.

Jinsuk Kim

unread,
Feb 14, 2017, 2:54:02 AM2/14/17
to David Trainor, bo...@chromium.org, mdj...@chromium.org, Slimming Clank, khusha...@chromium.org
On Tue, Feb 14, 2017 at 8:48 AM, David Trainor <dtra...@chromium.org> wrote:
Cool!  Thanks for doing a lot of this investigation!  Are you able to route the touch event to the contextual search panel's ViewAndroid, or is that still giving you trouble?

Now ​I'm having no problem routing the event to CS panel VA in my experiment. It's slightly more involved because overlay panel content is lazily created (i.e. overlay panel can be visible with or without actual content) though.
 
I talked to Khushal and we had another idea that might work too.  This might be a super simple way to migrate the content layer over without worrying about the Chrome UI.  Let me know what you think:

Right now the main issue is passing touch events to a ViewAndroid without a Java class or a built out view hierarchy right?  What if we did this intermediate refactor to help you get rid of ContentViewCore without the other headaches of worrying about the LayoutManager:

​​Interesting because I also started with a somewhat similar approach assigning a ViewRoot ​for each WebContent. That certainly (I think) was a way to go for an intermediate step, without having to introduce much changes in compositor/layout. First patch of https://codereview.chromium.org/2595263002 is based on per-content ViewRoot with an intent to replacing ContentView{Core} with ViewRoot as a path to native.  ​

I'd have loved this idea much more if it had been suggested a month sooner. I came a long way from that since then, following the general feedback of the reviewers ​preferring single ViewRoot. So it feels like going back in time..  but it's not entirely same as what I did, so probably worth consideration.
 
- ViewRoot is basically a JNI bridge to a ViewAndroid.
​
​
I think ViewRoot is already made in this way.​

​
- We make WebContents have a ViewRoot and we expose it in Java.
- We migrate all of the code that passes touch events to the ContentViewCore/ContentView to hit WebContents' ViewRoot.

I take it that layout will pass touch events through WebContents rather than ContentView{Core}.  WebContents ​already (its corresponding WebContentsViewAndroid)​  has a ViewAndroid. So touch events can be passed straight to that - I think the suggestion actually does not need ViewRoot at all. 

- As we refactor the front end and we no longer need the Java hook for the ViewAndroid WebContents we simply make it a ViewAndroid and no longer a ViewRoot.

That would let us keep basically the same approaches we have now... although the only difference is we have more than one ViewRoot in the system.  I'm not sure if this is a big deal or not in the short term.


If single ViewRoot is the goal we'll have in the end then ​the question to ask is what we benefit from this short-term, multiple ViewRoot step. Does it help decouple content / chrome refactoring work?

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

--
You received this message because you are subscribed to the Google Groups "Slimming Clank" group.
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clank+unsubscribe@chromium.org.

To post to this group, send email to slimmin...@chromium.org.

--
You received this message because you are subscribed to the Google Groups "Slimming Clank" group.
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clank+unsubscribe@chromium.org.

To post to this group, send email to slimmin...@chromium.org.

--
You received this message because you are subscribed to the Google Groups "Slimming Clank" group.
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clank+unsubscribe@chromium.org.

To post to this group, send email to slimmin...@chromium.org.

--
You received this message because you are subscribed to the Google Groups "Slimming Clank" group.
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clank+unsubscribe@chromium.org.

To post to this group, send email to slimmin...@chromium.org.

--
You received this message because you are subscribed to the Google Groups "Slimming Clank" group.
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clank+unsubscribe@chromium.org.

--
You received this message because you are subscribed to the Google Groups "Slimming Clank" group.
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clank+unsubscribe@chromium.org.

To post to this group, send email to slimmin...@chromium.org.

Jinsuk Kim

unread,
Feb 14, 2017, 3:04:01 AM2/14/17
to David Trainor, bo...@chromium.org, mdj...@chromium.org, Slimming Clank, khusha...@chromium.org
Oops I missed some of you questions below.

On Tue, Feb 14, 2017 at 8:48 AM, David Trainor <dtra...@chromium.org> wrote:
Cool!  Thanks for doing a lot of this investigation!  Are you able to route the touch event to the contextual search panel's ViewAndroid, or is that still giving you trouble?

I talked to Khushal and we had another idea that might work too.  This might be a super simple way to migrate the content layer over without worrying about the Chrome UI.  Let me know what you think:

Right now the main issue is passing touch events to a ViewAndroid without a Java class or a built out view hierarchy right?  What if we did this intermediate refactor to help you get rid of ContentViewCore without the other headaches of worrying about the LayoutManager:

- ViewRoot is basically a JNI bridge to a ViewAndroid.
- We make WebContents have a ViewRoot and we expose it in Java.
- We migrate all of the code that passes touch events to the ContentViewCore/ContentView to hit WebContents' ViewRoot.
- As we refactor the front end and we no longer need the Java hook for the ViewAndroid WebContents we simply make it a ViewAndroid and no longer a ViewRoot.

That would let us keep basically the same approaches we have now... although the only difference is we have more than one ViewRoot in the system.  I'm not sure if this is a big deal or not in the short term.

On Mon, Feb 13, 2017 at 2:36 PM Bo Liu <bo...@chromium.org> wrote:
No feedback.. just \o/

On Sun, Feb 12, 2017 at 10:11 PM, Jinsuk Kim <jins...@google.com> wrote:
I've been experimenting one approach which I think is feasible - largely based on David's suggestion - and had got the initial version working, though I'm sure there will still be many corner cases I haven't run into yet. 

A short summary is that it works by adding a ViewAndroid that represents a Java view at the top of the VA list when you want the events to be handled by Java layer, such as tab switcher, overlay panel bar, new tab page, etc. 
Just to clarify: so we always steal from Java and route back to Java to these views in the hierarchy? 

​Yes previously it was always Java first and then native but I'm reversing the order - by default all the events are routed to native. GIving Java layer a chance to handle them is kind of optional, possible by having a ViewAndroid for Java layer.
​

CompositorViewHolder always intercepts touch events and route them to native via ViewRoot.  The children of ViewRoot are for WebContents and the one at the top is the target to consume them in most cases. But if there is a Java UI component that should get the events,  you create a VA and place it at the top via ViewRoot.addViewAndroid(). Its bounds should match the area on screen, and when matched in hit testing, it returns false back to Compositor.onTouchEvent so that the Java layer can consume them. No need to find the target in event filters any more, and EventTarget.CONTENT would have been already handled by the time flow reaches there. 
Could you elaborate a bit on how this matches the event to the panel's WebContents and delegates it? 

​panel == overlay panel?

​If overlay WebContents is not present (since it is created lazily) but we have a panel bar only (in peeked state), you defined a VA with the same bounds as the bar, and places it at the top of VA hierarchy. Any touch event on it will be matched in hit testing, and the OnTouchEvent will return false, indicating that the native didn't consume it - so Java layer should handle it.

Once overlay WebContents is created, we need to have a ViewAndroid whose bounds covers covers panel bar + background.

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

--
You received this message because you are subscribed to the Google Groups "Slimming Clank" group.
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clank+unsubscribe@chromium.org.

To post to this group, send email to slimmin...@chromium.org.

--
You received this message because you are subscribed to the Google Groups "Slimming Clank" group.
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clank+unsubscribe@chromium.org.

To post to this group, send email to slimmin...@chromium.org.

--
You received this message because you are subscribed to the Google Groups "Slimming Clank" group.
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clank+unsubscribe@chromium.org.

To post to this group, send email to slimmin...@chromium.org.

--
You received this message because you are subscribed to the Google Groups "Slimming Clank" group.
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clank+unsubscribe@chromium.org.

To post to this group, send email to slimmin...@chromium.org.

--
You received this message because you are subscribed to the Google Groups "Slimming Clank" group.
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clank+unsubscribe@chromium.org.

--
You received this message because you are subscribed to the Google Groups "Slimming Clank" group.
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clank+unsubscribe@chromium.org.

To post to this group, send email to slimmin...@chromium.org.

Khushal Sagar

unread,
Feb 14, 2017, 3:02:47 PM2/14/17
to Jinsuk Kim, David Trainor, bo...@chromium.org, mdj...@chromium.org, Slimming Clank, khusha...@chromium.org
On Mon, Feb 13, 2017 at 11:53 PM, 'Jinsuk Kim' via Slimming Clank <slimmin...@chromium.org> wrote:


On Tue, Feb 14, 2017 at 8:48 AM, David Trainor <dtra...@chromium.org> wrote:
Cool!  Thanks for doing a lot of this investigation!  Are you able to route the touch event to the contextual search panel's ViewAndroid, or is that still giving you trouble?

Now ​I'm having no problem routing the event to CS panel VA in my experiment. It's slightly more involved because overlay panel content is lazily created (i.e. overlay panel can be visible with or without actual content) though.

That's cool. Do you mind sharing the patch? I was mostly curious about what the tree looked like, and how much plumbing was involved to get that set up for getting something like the content overscroll logic working (its here right now).
 
 
I talked to Khushal and we had another idea that might work too.  This might be a super simple way to migrate the content layer over without worrying about the Chrome UI.  Let me know what you think:

Right now the main issue is passing touch events to a ViewAndroid without a Java class or a built out view hierarchy right?  What if we did this intermediate refactor to help you get rid of ContentViewCore without the other headaches of worrying about the LayoutManager:

​​Interesting because I also started with a somewhat similar approach assigning a ViewRoot ​for each WebContent. That certainly (I think) was a way to go for an intermediate step, without having to introduce much changes in compositor/layout. First patch of https://codereview.chromium.org/2595263002 is based on per-content ViewRoot with an intent to replacing ContentView{Core} with ViewRoot as a path to native.  ​

I'd have loved this idea much more if it had been suggested a month sooner. I came a long way from that since then, following the general feedback of the reviewers ​preferring single ViewRoot. So it feels like going back in time..  but it's not entirely same as what I did, so probably worth consideration.
 
- ViewRoot is basically a JNI bridge to a ViewAndroid.
​
​
I think ViewRoot is already made in this way.​

​
- We make WebContents have a ViewRoot and we expose it in Java.
- We migrate all of the code that passes touch events to the ContentViewCore/ContentView to hit WebContents' ViewRoot.

I take it that layout will pass touch events through WebContents rather than ContentView{Core}.  WebContents ​already (its corresponding WebContentsViewAndroid)​  has a ViewAndroid. So touch events can be passed straight to that - I think the suggestion actually does not need ViewRoot at all.

This was for layout being able to have a java hook to pass events to a WebContents. Right now CVC effectively acts as that JNI bridge. We do have ViewAndroid in WCVA which represents the NativeView for it but nothing to interact with it in java.


- As we refactor the front end and we no longer need the Java hook for the ViewAndroid WebContents we simply make it a ViewAndroid and no longer a ViewRoot.

That would let us keep basically the same approaches we have now... although the only difference is we have more than one ViewRoot in the system.  I'm not sure if this is a big deal or not in the short term.


If single ViewRoot is the goal we'll have in the end then ​the question to ask is what we benefit from this short-term, multiple ViewRoot step. Does it help decouple content / chrome refactoring work?


Right now all layout code is set up to do most routing in java until the event needs to go to WebContents and then it is sent to native. Like you said, the thought was that having a java bridge directly to a WebContents will decouple content / chrome refactoring work, since we can incrementally move functionality from CVC to VA and understand how that should fit into it. Much of that should be the same irrespective of how events are routed to content's view from the embedder right? From chrome's perspective, the change is easier to understand then.

Its just that without having a clearer picture of how Layouts are going to move to/use VA, its hard to know whether we are just shoehorning the single ViewRoot right now, with only the content requirements in mind, or is it actually a step towards how chrome UI is going to change. May be we can take a look at your patch and see which approach is simpler?
 

Jinsuk Kim

unread,
Feb 15, 2017, 8:25:11 AM2/15/17
to Khushal Sagar, David Trainor, bo...@chromium.org, mdj...@chromium.org, Slimming Clank, khusha...@chromium.org
On Wed, Feb 15, 2017 at 5:02 AM, Khushal Sagar <khusha...@google.com> wrote:


On Mon, Feb 13, 2017 at 11:53 PM, 'Jinsuk Kim' via Slimming Clank <slimmin...@chromium.org> wrote:


On Tue, Feb 14, 2017 at 8:48 AM, David Trainor <dtra...@chromium.org> wrote:
Cool!  Thanks for doing a lot of this investigation!  Are you able to route the touch event to the contextual search panel's ViewAndroid, or is that still giving you trouble?

Now ​I'm having no problem routing the event to CS panel VA in my experiment. It's slightly more involved because overlay panel content is lazily created (i.e. overlay panel can be visible with or without actual content) though.

That's cool. Do you mind sharing the patch? I was mostly curious about what the tree looked like, and how much plumbing was involved to get that set up for getting something like the content overscroll logic working (its here right now).

​When is it triggered? I was not conscious of this part. Common user interaction with context search panel seems to work okay. I'll take a look if this was not taken into account in my CL. The CL is still messy,  not complete (that's why I'd like to get your thoughts on overall direction), and depends on other CL which is not landed yet. Will share it with you guys once it gets into decent state. I also needs to make it work against native pages.
​
 
 
I talked to Khushal and we had another idea that might work too.  This might be a super simple way to migrate the content layer over without worrying about the Chrome UI.  Let me know what you think:

Right now the main issue is passing touch events to a ViewAndroid without a Java class or a built out view hierarchy right?  What if we did this intermediate refactor to help you get rid of ContentViewCore without the other headaches of worrying about the LayoutManager:

​​Interesting because I also started with a somewhat similar approach assigning a ViewRoot ​for each WebContent. That certainly (I think) was a way to go for an intermediate step, without having to introduce much changes in compositor/layout. First patch of https://codereview.chromium.org/2595263002 is based on per-content ViewRoot with an intent to replacing ContentView{Core} with ViewRoot as a path to native.  ​

I'd have loved this idea much more if it had been suggested a month sooner. I came a long way from that since then, following the general feedback of the reviewers ​preferring single ViewRoot. So it feels like going back in time..  but it's not entirely same as what I did, so probably worth consideration.
 
- ViewRoot is basically a JNI bridge to a ViewAndroid.
​
​
I think ViewRoot is already made in this way.​

​
- We make WebContents have a ViewRoot and we expose it in Java.
- We migrate all of the code that passes touch events to the ContentViewCore/ContentView to hit WebContents' ViewRoot.

I take it that layout will pass touch events through WebContents rather than ContentView{Core}.  WebContents ​already (its corresponding WebContentsViewAndroid)​  has a ViewAndroid. So touch events can be passed straight to that - I think the suggestion actually does not need ViewRoot at all.

This was for layout being able to have a java hook to pass events to a WebContents. Right now CVC effectively acts as that JNI bridge. We do have ViewAndroid in WCVA which represents the NativeView for it but nothing to interact with it in java.

​I see. I guess what I meant to say by 'straight to that' was pass the event through WebContents or define a new Java class (maybe ViewAndroid?) provided through WebContents, rather than repurposing ViewRoot for that.


- As we refactor the front end and we no longer need the Java hook for the ViewAndroid WebContents we simply make it a ViewAndroid and no longer a ViewRoot.

That would let us keep basically the same approaches we have now... although the only difference is we have more than one ViewRoot in the system.  I'm not sure if this is a big deal or not in the short term.


If single ViewRoot is the goal we'll have in the end then ​the question to ask is what we benefit from this short-term, multiple ViewRoot step. Does it help decouple content / chrome refactoring work?


Right now all layout code is set up to do most routing in java until the event needs to go to WebContents and then it is sent to native. Like you said, the thought was that having a java bridge directly to a WebContents will decouple content / chrome refactoring work, since we can incrementally move functionality from CVC to VA and understand how that should fit into it. Much of that should be the same irrespective of how events are routed to content's view from the embedder right? From chrome's perspective, the change is easier to understand then.

​The decoupling of refactoring tasks being done in parallel was one of the concerns raised in the beginning. One idea (credit goes to boliu@) is the keep my changes around single view root  behind a command flag so that it won't affect other works, until it reaches reliable, stable phase. 

 
Its just that without having a clearer picture of how Layouts are going to move to/use VA, its hard to know whether we are just shoehorning the single ViewRoot right now, with only the content requirements in mind, or is it actually a step towards how chrome UI is going to change. May be we can take a look at your patch and see which approach is simpler?

What we've been discussing so far on/offline led me to believe that single view root is the right concept we're heading toward. Per-content view root is a good intermediate step I also had looked into. What I'm trying  at this point is experiment the changes required to be made in Chrome/Content to apply the concept. I'm sure it will go through different implementations based on the feedback from you guys and my own test results.

Jinsuk Kim

unread,
Feb 16, 2017, 8:46:18 AM2/16/17
to Khushal Sagar, David Trainor, bo...@chromium.org, mdj...@chromium.org, Slimming Clank, khusha...@chromium.org
If I read it right, the overscroll here Khushal mentioned requires the target be determined not only by the event coordinate but also by a certain UI condition. Then it would get ugly because the flow can be something like native -> Java -> native again. I'll have to test it the first thing in the morning.

Khushal and I had a chat to discuss the matter. Basically I'd like to push the single viewroot idea as far as I can since I haven't found apparent issues yet,  but also agree with him on the point he made about the estimated amount of work on Chrome/Content it would require to migrate it.  I also want to hear from others in this thread.

...

[Message clipped]  

Jinsuk Kim

unread,
Feb 20, 2017, 5:35:35 PM2/20/17
to Khushal Sagar, David Trainor, bo...@chromium.org, mdj...@chromium.org, Slimming Clank, khusha...@chromium.org
Last week a few of us (boliu@, dtrainor@ khushalsagar@, jinsukkim@) had a meeting to discuss the approach to take for event routing. We agreed on the point that:

- Single ViewRoot may be the ideal goal in the end but involves lots of changes to get it to work
- It affects Chrome front end that also will go through refactoring 
- The prototype I built has a potential problem in handling panel overscroll

Until we have a better idea on the direction to reach the ultimate goal, it may be best to decouple the refactoring on Chrome/Content so that work on each side won't step across. 

I'll keep looking into refactoring ContentView{Core} but without ViewRoot for now. It'll partially takes into account the suggestion by dtrainor@ in this thread (The name of classes, detailed impl may be different) - quoting here again:

> - ViewRoot is basically a JNI bridge to a ViewAndroid.
> - We make WebContents have a ViewRoot and we expose it in Java.
> - We migrate all of the code that passes touch events to the ContentViewCore/ContentView to hit WebContents' ViewRoot.
> - As we refactor the front end and we no longer need the Java hook for the ViewAndroid WebContents we simply make it a ViewAndroid and no longer a ViewRoot. 

Sorry for confusion the change of direction might cause but I think it is a part of healthy dev process that considers overall situation and various perspectives. Feel free to let us know if you have questions.

Thanks,
Reply all
Reply to author
Forward
0 new messages