+aelias@, boliu@: Chrome for Android folks
+rjkroege@: FYI

+some cc folks because we're proposing reducing display compositor special casing for android-y things! Yay: piman@, danakj@, enne@

I notice that this is specifically for android, but for the record: we should do the same for aura too [eventually, if not at the same time as android].

works for chrome I think, but webview is still using native (ie non-composited) selection controls; updates are best effort rather than having some hard synchronization guarantees, but still has to have communication back to browser :/

Another problem is hit testing.  The handles are not only visually on top, but also first in line to absorb touch input.  They generate selection move events and also action popup show/hide events (on single-tap).  One of the reasons for the current architecture is we wanted to avoid cluttering LayerTreeHostImpl and creating a new impl -> main channel for that stuff.  I still feel that would be quite ugly to do and would prefer not to move in that direction.

I don't think small performance tunings on the order of 2 IPC per frame are worth worrying about because the handles aren't present 99% of the time.  The only thing I'm concerned about is the asynchronicity of display (slippage behind scroll).  If I understand correctly, you have an approach that avoids slippage, in that case can we WontFix this?

#6: What's the approach to avoid slippage?

"the renderer allocates a new LocalSurfaceId when |selection| changes so that the browser doesn't fall behind in updating the location of touch selection handles" and the (synchronous?) round-trip to the browser?  I'm not sure I totally understand the design but it sounds like machinery to avoid slippage?  Correct me if I'm wrong.

This assumes the display compositor lives on the same thread as browser UI, and that the renderer is allocating the LocalSurfaceId. This mechanism is not compatible with putting the compositor in the gpu (now called viz) process. That's the main issue.

 

OK.  I'd suggest taking the approach in #0 purely for display of handles in Chrome but retain most of the browser machinery for input handling and for WebView.  Also, note that this issue affects all touch platforms as the code is mostly shared.

Quick comment on #5: It looks like WebView actually hides selection handles when scrolling, so keeping things strictly in sync is not an issue, and allocating a new surface id every time we scroll is not necessary in WebView case.

Yes, WebView hides them specifically in order to conceal the slippage bug which it already incurs due to its using native Android PopupWindow instead of the compositor.

Note that WebView uses the PopupWindow in order for the handle to be able to stick out further than the extent of the WebView.  Although we're not certain whether or not any app actually benefits from this, so we're considering experimenting with deleting the WebView-specific path.

I wish we would just hide the handles while scrolling. It's not like the user will try to scroll and move the handles at the same time (I don't think it's even possible if the user tries). It just adds unnecessary complexity and overhead that is not too bad for the current architecture but will get worse.

What if the user is dragging the selection handle off the bottom of the screen? Scroll should happen while the selection extends and hence you must show the starting selection handle while you drag the ending handle down.

Well, I'm not entirely sure what will happen but I assume WebView already has a solution for it. Mikhail was also saying that on touch Chrome OS we hide the handles while scrolling, so I'm just suggesting maybe matching the behavior of other platforms is not too bad if it turns out the performance toll outweighs the benefits to users.

Sync'ing selection handles is a nice polish feature so I think we should do it right :-) Chromium / Chrome is intended to be a high quality product used by many, many people so the details matter in my personal opinion.

Agree with fsamuel. jdduke spent quite a bit of time to make it possible to show the handles when scrolling. It's a nice polish feature. 

This is an viz for Chrome Android blocker.

We're planning on initially handling this using the same method as the omnibox (see blocking  issue 689754 ). After that is resolved, we should follow up on the performance implications and likely implement a more optimized path for selection handles only - see this doc:
https://docs.google.com/document/d/1sZqQnJwvFyCiPjNsXq90ghaPtdSBaOnvDYlbjPOGEzw/edit#heading=h.4ioun51jsve1

as well as option 2 of this doc: https://docs.google.com/document/d/1nawFPSv7FMT1FjZ5rbm4aBVIfGND7ZtVZWSqVJe01L0/edit#, which summarizes a similar idea.

fsameul@ is working on this.

This is fixed so I'm marking it as so.