New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 689754 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocked on:
issue 672962
issue 810213

Blocking:
issue 732555
issue 801350



Sign in to add a comment

Handle top bar controls with Surface Sync / Viz on Android

Project Member Reported by fsam...@chromium.org, Feb 8 2017

Issue description

Chrome for Android top bar controls looked like a complex special case that would make refactoring cc/surfaces for Mus difficult so I was hoping to de-special-case some of this.

My understanding of the code is currently, we send input events to the renderer, which scrolls, and sends an offset to slide the top bar controls in CompositorFrameMetadata. The browser then deals with positioning the top bar controls when it receives the CompositorFrame. You can imagine in a future world where the display compositor moves to the GPU process, this doesn't make sense.

https://cs.chromium.org/chromium/src/cc/output/compositor_frame_metadata.h?type=cs&q=CompositorFrameMetadata&l=59 

Since the renderer can set the position of the top bar controls, it's conceivable that it could already put the top bar controls entirely offscreen, and display its own version to the user, fooling them into believing they're at a different site.

My suggestion is then, to avoid some special casing, the browser simply sends the renderer the surface ID for the top bar controls and lets the renderer scroll the top bar controls. Surface IDs are opaque identifiers. They're not pixels and so readback is not possible. It seems like the security model here is similar to what we already have and this could simplify the code quite a bit.

 
As discussed offline, SGTM.
Blocking: 696199
Blockedon: 672962
Cc: piman@chromium.org
This is likely blocked on surface synchronization:

1. Browser submits a CompositorFrame corresponding to the top bar controls with a given LocalSurfaceId. The SurfaceId = (FrameSinkId, LocalSurfaceId) pair that is then passed to the renderer. With an out-of-process "display compositor", that message will not be delivered to viz (the new name for the gpu process) immediately.

2. The browser sends the SurfaceId for the top bar controls to the renderer.

3. The rendrerer embeds that surface ID and submits a CompositorFrame.

If 3 arrives to the display compositor before 1 then we'll have missing content. Surface synchronization will take care of this by blocking activation of the renderer's CompositorFrame until the top bar controls have been received by the display compositor.

Comment 4 by samans@chromium.org, Apr 13 2017

We allocate a new local surface id not only on top and bottom bar changes but also when selection and has_transparent_background change. Can someone explain why we do that? Is that actually necessary?
https://cs.chromium.org/chromium/src/content/renderer/gpu/renderer_compositor_frame_sink.cc?rcl=cceb830695c05acfa0c6a4fbcba8ecf8efb9551e&l=193

Comment 5 by fsamuel@google.com, Apr 13 2017

Cc: mfomitchev@chromium.org
My guess would be to sync up with touch selection handles but that's just a guess? 
Owner: samans@chromium.org
Status: Assigned (was: Available)

Comment 7 by danakj@chromium.org, Jun 12 2017

Blocking: 732555

Comment 8 by danakj@chromium.org, Jun 12 2017

Blocking: -601863
Blocking: -696199
Owner: ----
Status: Available (was: Assigned)
Components: Internals>Compositing
Cc: cblume@chromium.org ericrk@chromium.org
Per offline discussion, it seems like a slightly different approach may better handle the security concerns around Omnibox positioning. See the doc here: (see doc here: https://docs.google.com/document/d/1nawFPSv7FMT1FjZ5rbm4aBVIfGND7ZtVZWSqVJe01L0/edit#)

To summarize:
1) If the renderer requires a browser UI update, it generates a new Surface ID and sends this surface ID with metadata to the browser.
2) The renderer sends its compositor frame (using the new surface ID) to the Viz/GPU proc.
3) The Browser generates a new compositor frame with elements updated according to the metadata. This frame embeds the new renderer surface ID. This compositor frame is sent to the Viz/GPU process.
4) Viz/GPU process composites the visual output using GL.

The main challenge with this approach is that we need a new system which allows a renderer to generate new surface IDs (for step 1).
Blocking: 713696
Cc: -mfomitchev@chromium.org
Blocking: 801350
Labels: -Pri-3 Pri-2
Owner: cblume@chromium.org
Status: Assigned (was: Available)
Summary: Handle top bar controls with Surface Sync / Viz on Android (was: Remove display compositor special casing of top bar controls on Chrome for Android)
Blocking: -801350 -713696
Blockedon: 810213
Blocking: 801350
Owner: ericrk@chromium.org
Owner: fsam...@chromium.org
Status: Fixed (was: Assigned)
Assigning myself and marking as FIXED as top bar controls now use surface sync.

Sign in to add a comment