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

Issue 771331 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Oct 16
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature

Blocking:
issue 763452
issue 785013



Sign in to add a comment

Viz: Plumb FrameToken changes to the parent client

Project Member Reported by fsamuel@google.com, Oct 3 2017

Issue description

Sometimes the chrome renderer wishes to ship IPCs to the browser process after a CompositorFrame has been received by the display compositor.

The way we deal with this today is through a frame token mechanism. The renderer allocates a new Frame token stuffs it into the CompositorFrame. The renderer ships IPCs tied to that particular frame with ViewHostMsg_FrameSwapMessages.

Currently, RenderWidgetHostImpl::SubmitCompositorFrame synchronously inspects the frame token in the CompositorFrame and matches it up with the frame token of the FrameSwapMessages. Once the frame token in the CF catches up with the frame swap messages, then we start handling the IPCs.

This cannot work with an out-of-process display compositor.

Instead, I propose the following:

1. CompositorFrameSinkSupport tracks the last frame token seen.

2. If it changes, then it informs FrameSinkManagerImpl which informs HostFrameSinkManager.

3. HostFrameSinkManager walks up the hierarchy and informs the parent HostFrameSinkClient.

4. DelegatedFrameHost (for content) is a HostFrameSinkClient and ServerWindow is a HostFrameSinkClient. It should take that changed frame token and inform the appropriate RenderWidgetHostImpl in content...somehow. Perhaps through a new FrameTokenObserver interface?
 

Comment 1 by fsamuel@google.com, Oct 3 2017

Cc: piman@chromium.org danakj@chromium.org

Comment 2 by piman@chromium.org, Oct 3 2017

For #4, DFH knows about its DFHClient (DFHClientAura or BrowserCompositorMac) which knows about the RenderWidgetHostView which knows about the RWHI, so I think you can follow that path to signal the frame token. Similarly on Android, DelegatedFrameHostAndroid (which is a HostFrameSinkClient) knows about RenderWidgetHostViewAndoid (its Client), which knows about RWHI.

Comment 3 by ericrk@chromium.org, Oct 11 2017

Labels: -Type-Bug Type-Feature
Status: Available (was: Untriaged)

Comment 4 by samans@chromium.org, Oct 16 2017

Blocking: 775030

Comment 5 by fsamuel@google.com, Oct 16 2017

Owner: yiyix@chromium.org
Status: Assigned (was: Available)
Blocking: 763452
Project Member

Comment 7 by bugdroid1@chromium.org, Nov 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4

commit ae38c3385b4beb1ba585a647d1ce80f0966e8cc4
Author: yiyix <yiyix@chromium.org>
Date: Tue Nov 14 18:58:27 2017

Viz: Plumb FrameToken changes to the parent client

Currently, RenderWidgetHostImpl::SubmitCompositorFrame synchronously
inspects the frame token in the CompositorFrame and matches it up with
the frame token of the FrameSwapMessages. Once the frame token in the
CompositorFrame catches up with the frame swap messages, then we start
handling the IPCs. However, this architecture cannot work with an
out-of-process display compositor. In this patch, I check the validation
of frame token in Viz/Service/CompositorFrameSinkSupport and use it
to call RenderWidgetHostImpl to handle the IPCs.

Bug:  771331 
Change-Id: I40fe5d0a4bba05e0bc635199c5fe03e5fe641c23
Reviewed-on: https://chromium-review.googlesource.com/748771
Commit-Queue: Yi Xu <yiyix@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516371}
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/components/viz/host/host_frame_sink_client.h
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/components/viz/host/host_frame_sink_manager.cc
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/components/viz/host/host_frame_sink_manager.h
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/components/viz/host/host_frame_sink_manager_unittest.cc
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/components/viz/service/frame_sinks/compositor_frame_sink_support.cc
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/components/viz/service/frame_sinks/compositor_frame_sink_support_unittest.cc
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/components/viz/service/frame_sinks/frame_sink_manager_impl.cc
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/components/viz/service/frame_sinks/frame_sink_manager_impl.h
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/components/viz/test/fake_host_frame_sink_client.cc
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/components/viz/test/fake_host_frame_sink_client.h
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/content/browser/renderer_host/browser_compositor_view_mac.h
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/content/browser/renderer_host/browser_compositor_view_mac.mm
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/content/browser/renderer_host/compositor_impl_android.h
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/content/browser/renderer_host/delegated_frame_host.h
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/content/browser/renderer_host/delegated_frame_host_client_aura.cc
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/content/browser/renderer_host/delegated_frame_host_client_aura.h
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/content/browser/renderer_host/offscreen_canvas_surface_impl.cc
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/content/browser/renderer_host/offscreen_canvas_surface_impl.h
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/content/browser/renderer_host/render_widget_host_unittest.cc
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/content/browser/renderer_host/render_widget_host_view_base.cc
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/content/browser/renderer_host/render_widget_host_view_base.h
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/content/browser/renderer_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/content/browser/renderer_host/render_widget_host_view_child_frame.h
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/content/browser/renderer_host/render_widget_host_view_mac.h
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/content/browser/renderer_host/render_widget_host_view_mac.mm
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/content/test/test_render_view_host.cc
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/content/test/test_render_view_host.h
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/services/ui/ws/server_window.cc
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/services/ui/ws/server_window.h
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/services/viz/privileged/interfaces/compositing/frame_sink_manager.mojom
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/ui/android/delegated_frame_host_android.cc
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/ui/android/delegated_frame_host_android.h
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/ui/aura/local/layer_tree_frame_sink_local.cc
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/ui/aura/local/layer_tree_frame_sink_local.h
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/ui/compositor/compositor.cc
[modify] https://crrev.com/ae38c3385b4beb1ba585a647d1ce80f0966e8cc4/ui/compositor/compositor.h

Blocking: 785013
Status: Fixed (was: Assigned)
Status: Assigned (was: Fixed)
This doesn't seem done. It caused a problem with a ChromeVox spoken feedback CL I was working on for mash:

https://chromium-review.googlesource.com/c/chromium/src/+/956235

content::WebContentsObserver::DidFirstVisuallyNonEmptyPaint() isn't called for the spoken feedback window under mash.

This is the usual callstack under classic ash: https://paste.googleplex.com/6672203332452352

Fady says it's because of this todo in ui::ws::OnFrameTokenChanged:
https://cs.chromium.org/chromium/src/services/ui/ws/server_window.cc?type=cs&l=480

I've worked around this in my CL, but I wonder if it will break other users of DidFirstVisuallyNonEmptyPaint() in the browser.

Blocking: -775030
ServerWindow was deleted, so I think this can be marked as Fixed?
This is still broken with OOP-Ash.
Blocking: -730193
Owner: ----
Status: Available (was: Assigned)
Owner: fsam...@chromium.org
Status: Fixed (was: Available)

Sign in to add a comment