Slimming Clank 11/16 meeting notes

[Alexandre]

Slimming Clank is the working group to clean up cruft in Android-specific (Clank and WebView) code in content/browser/, ui/, and (later) chrome/.  We aim for net negative lines of code, and to establish clear dependencies, lifetimes, and lines of authority.

We had a syncup of what everyone is working on.  jinsukkim@ has been working on ActionMode, mdjones@ on viewport simplification (design doc), amaralp@ on mouse and selection logic, aelias@ on thumbnail layers and background color, rlanday@ on swipe refresh handler, changwan@ on IME, wychen@ on drag-and-drop events.  There were a few design discussions to come out of that:

1. ViewAndroid/WindowAndroid hierarchy.  We agreed input events should go from the embedder to WindowAndroid, which should resolve positions forward to ViewAndroid, which then are forwarded to individual web contents or Blimp contents via a base class interface called something like ViewClient.  (This will invert the current flow where events go directly from embedder to ContentViewCore and from there to views.)

Action item for follow-up: research how Aura structures input event and resize forwarding and (probably) match the model.

2. IME and focus behavior.  We agreed to try to detach IME and focus-related objects almost completely from the main blob of Android embedder objects and have it held on the side by the WebContents instead.  (Although IME APIs force us arbitrarily to use an Android View, it has no need to be attached to the main View hierarchy.)

[Jinsuk Kim]

I took a brief look at aura/mus version of WCV/RWHV, and verified that they also own a native view (aura::Window and ui::Window, respectively) which form a hierarchy just like ViewAndroid instances do in Clank. This clearly indicates that sievers@ used other platforms as reference when extending ViewAndroid.

{aura, ui}::Window are closer to the abstraction of view used in in underlying platform - i.e. it was designed to handle stacked structure of views, visibility, size, layout, hit test, etc, and often forms a recursive flow to find the right window in the hierarchy or dispatch notification to all the elements in the tree.

There is no handlers for mouse/key/motion events there. It makes me think that the association of key events and view is somewhat unique on Android platform. 

I reopened https://codereview.chromium.org/2502763003/ to outline the idea on how ViewAndroid is used to forward calls down to native based on the discussion we had. Note that the dispatch still starts from ViewAndroidDelegate to top ViewAndroid rather than from WindowAndroid, since boliu@ later found that WebView instances all share one WindowAndroid object, which makes it hard to dispatch the event to the intended ViewAndroid - he said he would think about it. 

[Bo Liu]

On Sun, Nov 20, 2016 at 10:24 PM, 'Jinsuk Kim' via Slimming Clank <slimmin...@chromium.org> wrote:

I took a brief look at aura/mus version of WCV/RWHV, and verified that they also own a native view (aura::Window and ui::Window, respectively) which form a hierarchy just like ViewAndroid instances do in Clank. This clearly indicates that sievers@ used other platforms as reference when extending ViewAndroid.

{aura, ui}::Window are closer to the abstraction of view used in in underlying platform - i.e. it was designed to handle stacked structure of views, visibility, size, layout, hit test, etc, and often forms a recursive flow to find the right window in the hierarchy or dispatch notification to all the elements in the tree.

There is no handlers for mouse/key/motion events there. It makes me think that the association of key events and view is somewhat unique on Android platform.

ui::EventHandler (implemented by ui::WindowDelegate which is implemented by RenderWidgetHostViewAura) is how input event is passed from aura layer to content layer.

 

I reopened https://codereview.chromium.org/2502763003/ to outline the idea on how ViewAndroid is used to forward calls down to native based on the discussion we had. Note that the dispatch still starts from ViewAndroidDelegate to top ViewAndroid rather than from WindowAndroid

Delegate is still wrong though. Delegate (or whatever it will be named) is used to talk from ui to upper layer, not the other way around.

 

since boliu@ later found that WebView instances all share one WindowAndroid object, which makes it hard to dispatch the event to the intended ViewAndroid - he said he would think about it. 

To elaborate idea of sending input events starting at the WindowAndroid, which is the root of the ViewAndroid tree, doesn't work for webview. Multiple webviews can be attached to the same WindowAndroid; so webview doesn't have a single java View that represents the root of the ViewAndroid tree.

The only way I see this working is webview has the ability to pick out the right ViewAndroid (that's owned by WebContentsViewAndroid), and either forward the event directly to that ViewAndroid, or have some way to tell WindowAndroid to directly forward to that child.

 

On Thursday, November 17, 2016 at 10:46:47 AM UTC+9, Alexandre Elias wrote:

Slimming Clank is the working group to clean up cruft in Android-specific (Clank and WebView) code in content/browser/, ui/, and (later) chrome/.  We aim for net negative lines of code, and to establish clear dependencies, lifetimes, and lines of authority.

We had a syncup of what everyone is working on.  jinsukkim@ has been working on ActionMode, mdjones@ on viewport simplification (design doc), amaralp@ on mouse and selection logic, aelias@ on thumbnail layers and background color, rlanday@ on swipe refresh handler, changwan@ on IME, wychen@ on drag-and-drop events.  There were a few design discussions to come out of that:

1. ViewAndroid/WindowAndroid hierarchy.  We agreed input events should go from the embedder to WindowAndroid, which should resolve positions forward to ViewAndroid, which then are forwarded to individual web contents or Blimp contents via a base class interface called something like ViewClient.  (This will invert the current flow where events go directly from embedder to ContentViewCore and from there to views.)

Action item for follow-up: research how Aura structures input event and resize forwarding and (probably) match the model.

2. IME and focus behavior.  We agreed to try to detach IME and focus-related objects almost completely from the main blob of Android embedder objects and have it held on the side by the WebContents instead.  (Although IME APIs force us arbitrarily to use an Android View, it has no need to be attached to the main View hierarchy.)

--
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/e45e481d-63bc-408f-b91b-a72265dfe057%40chromium.org.

[Jinsuk Kim]

On Tuesday, November 22, 2016 at 8:22:50 AM UTC+9, Bo Liu wrote:

On Sun, Nov 20, 2016 at 10:24 PM, 'Jinsuk Kim' via Slimming Clank <slimmin...@chromium.org> wrote:

I took a brief look at aura/mus version of WCV/RWHV, and verified that they also own a native view (aura::Window and ui::Window, respectively) which form a hierarchy just like ViewAndroid instances do in Clank. This clearly indicates that sievers@ used other platforms as reference when extending ViewAndroid.

{aura, ui}::Window are closer to the abstraction of view used in in underlying platform - i.e. it was designed to handle stacked structure of views, visibility, size, layout, hit test, etc, and often forms a recursive flow to find the right window in the hierarchy or dispatch notification to all the elements in the tree.

There is no handlers for mouse/key/motion events there. It makes me think that the association of key events and view is somewhat unique on Android platform.

ui::EventHandler (implemented by ui::WindowDelegate which is implemented by RenderWidgetHostViewAura) is how input event is passed from aura layer to content layer.

 

I reopened https://codereview.chromium.org/2502763003/ to outline the idea on how ViewAndroid is used to forward calls down to native based on the discussion we had. Note that the dispatch still starts from ViewAndroidDelegate to top ViewAndroid rather than from WindowAndroid

Delegate is still wrong though. Delegate (or whatever it will be named) is used to talk from ui to upper layer, not the other way around.

Not sure how to proceed from here - I could add ViewAndroid.java to define the path from embedder to content/ui layer and leave the ViewAndroidDelegate alone. Is that what you would suggest?

 

[Bo Liu]

I did type this

> The only way I see this working is webview has the ability to pick out the right ViewAndroid (that's owned by WebContentsViewAndroid), and either forward the event directly to that ViewAndroid, or have some way to tell WindowAndroid to directly forward to that child.

though I guess that's not enough detail.

Ideally webview code creates the ViewAndroid and set it WebContentsAndroid, then can hold a pointer to it, and forward events. But that means potentially duplicating the java->native code, since we don't want a java ViewAndroid.

Or, can get ViewAndroid from WebContentsAndroid, but that has the same java->native issue.

Or do some hack that forces WindowAndroid to forward to the correct child, but I don't see how that really works.

So perhaps should solve the java->native issue, eg ViewAndroid create a java object that does the event forwarding to itself.

That's a lot of rambling.. not sure if it's clear. Also maybe others have better ideas..

 

 

--
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/816901f5-9afa-4814-8f4c-1173a36ef3b1%40chromium.org.

[Jinsuk Kim]

> The only way I see this working is webview has the ability to pick out the right ViewAndroid (that's owned by WebContentsViewAndroid), and either forward the event directly to that ViewAndroid, or have some way to tell WindowAndroid to directly forward to that child.

though I guess that's not enough detail.

Ideally webview code creates the ViewAndroid and set it WebContentsAndroid, then can hold a pointer to it, and forward events. But that means potentially duplicating the java->native code, since we don't want a java ViewAndroid.

Or, can get ViewAndroid from WebContentsAndroid, but that has the same java->native issue.

Or do some hack that forces WindowAndroid to forward to the correct child, but I don't see how that really works.

So perhaps should solve the java->native issue, eg ViewAndroid create a java object that does the event forwarding to itself.

That's a lot of rambling.. not sure if it's clear. Also maybe others have better ideas..

 

​

I think it is possible by having ViewAndroidDelegate provide

​ a Java API returning​

a token associated with

​the 

ViewAndroid owned by WCVA

​ ​

. ​

​

​

​

Callsites then invoke WindowAndroid.onPhysicalBackingSizeChanged(token, width, height). WindowAndroid needs to make sure the passed token is associated with one of its children,

​find the matching VA, ​

and forward the call down. WIndowAndroid also needs to maintain a map

​of token​

 

​-> ​

struct ViewData where it needs to keep

​VA*, ​

view-related parameters like physical backing size, top control height, etc. which are currently maintained by CVC.

​ The map entry will be deleted when the corresponding child VA is removed from WA.​

I'll try this idea if it sounds plausible. Let me have your thought.

​

 

[Bo Liu]

On Tuesday, November 22, 2016, 'Jinsuk Kim' via Slimming Clank <slimmin...@chromium.org> wrote:

> The only way I see this working is webview has the ability to pick out the right ViewAndroid (that's owned by WebContentsViewAndroid), and either forward the event directly to that ViewAndroid, or have some way to tell WindowAndroid to directly forward to that child.

though I guess that's not enough detail.

Ideally webview code creates the ViewAndroid and set it WebContentsAndroid, then can hold a pointer to it, and forward events. But that means potentially duplicating the java->native code, since we don't want a java ViewAndroid.

Or, can get ViewAndroid from WebContentsAndroid, but that has the same java->native issue.

Or do some hack that forces WindowAndroid to forward to the correct child, but I don't see how that really works.

So perhaps should solve the java->native issue, eg ViewAndroid create a java object that does the event forwarding to itself.

That's a lot of rambling.. not sure if it's clear. Also maybe others have better ideas..

 

​

I think it is possible by having ViewAndroidDelegate provide

​ a Java API returning​

a token associated with

​the 

ViewAndroid owned by WCVA

​ ​

. ​

Still feels wrong here because embedder is asking ViewAndroidDelegate for things, and that's not right. VAD is for ViewAndroid to call up, not the other way around. It might be easier to just get WebContents to return the token.

 

Callsites then invoke WindowAndroid.onPhysicalBackingSizeChanged(token, width, height). WindowAndroid needs to make sure the passed token is associated with one of its children,

​find the matching VA, ​

and forward the call down. WIndowAndroid also needs to maintain a map

​of token​

 

​-> ​

struct ViewData where it needs to keep

​VA*, ​

view-related parameters like physical backing size, top control height, etc. which are currently maintained by CVC.

​ The map entry will be deleted when the corresponding child VA is removed from WA.​

Following that train of thought, alternatives to keeping token->ViewAndroid map

* what if WebContents returns the viewandroid directly? not really, because there is no ViewAndroid counterpart

* what if WebContents returns a java object that can forward input events to the right ViewAndroid? I think that might be a better idea. add an generic java interface that can receive input for the ViewAndroid tree

thoughts?

 

I'll try this idea if it sounds plausible. Let me have your thought.

​

 

--

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/CAC4m%2BbGzLHZtymg4o%3DFt4ZnAE%3DAch1G3%2BS7OcdOY946iH3HpPg%40mail.gmail.com.

[Jinsuk Kim]

I'm find with your suggestion. Two points I'd like to make: 

- I think the java object returned by WebContents is virtually java version of 'ViewAndroid' in the sense that it's the Java counterpart that interact with ui::VA for java -> native calls. I tried to avoid adding one if possible.

- It's simpler - will save us from having to maintain the map. But it takes WindowAndroid out of the picture and let the flow start from the top ViewAndroid (owned by WCVA) and downward.  I've been thinking the discussion was geared toward having WA as the starting point, which I had in mind for the approach above. 

[Jinsuk Kim]

On Thu, Nov 24, 2016 at 6:31 AM, Bo Liu <bo...@chromium.org> wrote:

​Updated ​

 

​https://crrev.com/2502763003. WebContents returns EventHandler introduced to forward calls to native. ​

[Bo Liu]

I think that's mostly convention. If we don't call it ViewAndroid.java, and they don't necessarily have the same lifetime, then it's probably fine :)