Use SurfaceID to prevent stale content from unloaded pages from being displayed |
||||||||||
Issue descriptionIn bug 672847 , there is an issue whereby a compositor frame is sent from the renderer process to the browser even though the page that it draws has been unloaded from the renderer due to a navigation having had committed. Although that is being fixed with a temporary solution, at some point we would like to use SurfaceIDs to link a compositor frame to the loaded page, so that once the omnibar has been updated with the new URL (i.e. at commit time), we will not redisplay any new compositor frames linked to the old SurfaceID. This is blocked on SurfaceIDs being available in the renderer process so that they might be included in the CompositorFrameMetadata.
,
Feb 23 2017
,
Apr 12 2017
On every navigation, the browser should allocate a new LocalSurfaceId for the renderer, and the renderer should submit to the new LocalSurfaceId. We can produce the timeout behavior in the browser currently by dropping the fallback SurfaceID (the old page) after some timeout.
,
Apr 13 2017
I'm going to tackle this problem by taking advantage of the incremental part of LocalSurfaceId. In RenderFrameImpl::DidCommitProvisionalLoad, I'm going to allocate a new LocalSurfaceId and send the local_id (the incremental part) to the browser. The browser will clear the page if it does not receive a frame with a local_id of at least that value within 4 seconds. It will also ignore frames with smaller local_ids. This is blocked on allocating LocalSurfaceId on the main thread.
,
Apr 13 2017
#4: That is fine from the perspective of retaining existing behavior. I don't know if it fits with existing plans to allocate the LocalSurfaceId in the renderer process, but I would defer to others on that.
,
Apr 13 2017
LocalSurfaceId is already allocated in the renderer process, and this proposal only moves the allocation to main thread.
,
Apr 17 2017
This approach takes us one step closer to what I'd like to achieve with surface sync anyway, so go ahead Saman! Thanks! :-)
,
Apr 18 2017
In a nutshell, I think what you're proposing is moving all LocalSurfaceId allocation in the renderer today out of RendererCompositorFrameSink to the main thread then we just tell RendererCompositorFrameSink via the cc::CompositorFrameSink what LocalSurfaceId to use to submit a CompositorFrame. This sounds good to me. Ultimately, the new LocalSurfaceId will not be generated in the main thread though. When some parameter such as size or navigation, or device scale factor or top bar controls etc changes, a new LocalSurfaceId will be bundled along with the change in the IPC message from the browser and that will be passed along to LayerTreeHost. Baby steps...
,
Apr 19 2017
,
May 3 2017
,
May 23 2017
,
Jun 13 2017
,
Oct 24 2017
This is blocked on surface synchronization so I'm marking as Available.
,
Jan 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ac97cd080598d6c960ac42322e1ef24a05094b10 commit ac97cd080598d6c960ac42322e1ef24a05094b10 Author: Saman Sami <samans@chromium.org> Date: Fri Jan 19 19:04:10 2018 Fix race in WaitForResizeComplete WaitForResizeComplete is racy. Every time RenderWidgetHostImpl receives a resize ack, it checks whether we're holding pointer moves or not, and if not reports that resize is completed. However, if releasing pointer moves happens after the last resize ack, it blocks forever. This race doesn't seem to happen right now but I have another CL that exposes it. This CL fixes this issue by adding a callback to WindowEventDispatcher that would be called when held events are dispatched, which allows WaitForResizeComplete to block on releasing pointer moves as well. Bug: 695579 , 775030 , 777881 Change-Id: I841899af2a9bf3cae8b58dd21ea236e652b33790 Reviewed-on: https://chromium-review.googlesource.com/872453 Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org> Reviewed-by: Antoine Labour <piman@chromium.org> Reviewed-by: Fady Samuel <fsamuel@chromium.org> Commit-Queue: Saman Sami <samans@chromium.org> Cr-Commit-Position: refs/heads/master@{#530580} [modify] https://crrev.com/ac97cd080598d6c960ac42322e1ef24a05094b10/content/public/test/browser_test_utils.cc [modify] https://crrev.com/ac97cd080598d6c960ac42322e1ef24a05094b10/ui/aura/test/window_event_dispatcher_test_api.cc [modify] https://crrev.com/ac97cd080598d6c960ac42322e1ef24a05094b10/ui/aura/test/window_event_dispatcher_test_api.h [modify] https://crrev.com/ac97cd080598d6c960ac42322e1ef24a05094b10/ui/aura/window_event_dispatcher.cc [modify] https://crrev.com/ac97cd080598d6c960ac42322e1ef24a05094b10/ui/aura/window_event_dispatcher.h
,
Jan 24 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f7731345b34fd9f87ce5792f5f952f8e5cdfaac8 commit f7731345b34fd9f87ce5792f5f952f8e5cdfaac8 Author: Saman Sami <samans@chromium.org> Date: Wed Jan 24 22:18:44 2018 Surface synchronization: Allocate new LocalSurfaceId on navigation In order to be able to tell the difference between the graphical output of the renderer before and after navigation, we should allocate a new LocalSurfaceId. Currently we rely on content_source_id to achieve this, but that is not compatible with Viz. When navigation happens, browser allocates a new id for the child and immediately embeds it. After 4 seconds it drops the fallback SurfaceId if it was generated before navigation. TBR=lazyboy@chromium.org Bug: 695579 , 775030 , 777881 Change-Id: I8ae11dd3de2e8d6cc889ad31f94bdb0bb5f6a5d4 Reviewed-on: https://chromium-review.googlesource.com/855256 Commit-Queue: Saman Sami <samans@chromium.org> Reviewed-by: Fady Samuel <fsamuel@chromium.org> Reviewed-by: Saman Sami <samans@chromium.org> Reviewed-by: Ken Buchanan <kenrb@chromium.org> Reviewed-by: Antoine Labour <piman@chromium.org> Cr-Commit-Position: refs/heads/master@{#531713} [modify] https://crrev.com/f7731345b34fd9f87ce5792f5f952f8e5cdfaac8/chrome/browser/extensions/api/tabs/tabs_api_unittest.cc [modify] https://crrev.com/f7731345b34fd9f87ce5792f5f952f8e5cdfaac8/chrome/browser/extensions/extension_context_menu_model_unittest.cc [modify] https://crrev.com/f7731345b34fd9f87ce5792f5f952f8e5cdfaac8/content/browser/frame_host/render_frame_host_impl.cc [modify] https://crrev.com/f7731345b34fd9f87ce5792f5f952f8e5cdfaac8/content/browser/renderer_host/browser_compositor_view_mac.h [modify] https://crrev.com/f7731345b34fd9f87ce5792f5f952f8e5cdfaac8/content/browser/renderer_host/browser_compositor_view_mac.mm [modify] https://crrev.com/f7731345b34fd9f87ce5792f5f952f8e5cdfaac8/content/browser/renderer_host/delegated_frame_host.cc [modify] https://crrev.com/f7731345b34fd9f87ce5792f5f952f8e5cdfaac8/content/browser/renderer_host/delegated_frame_host.h [modify] https://crrev.com/f7731345b34fd9f87ce5792f5f952f8e5cdfaac8/content/browser/renderer_host/delegated_frame_host_client_aura.cc [modify] https://crrev.com/f7731345b34fd9f87ce5792f5f952f8e5cdfaac8/content/browser/renderer_host/delegated_frame_host_client_aura.h [modify] https://crrev.com/f7731345b34fd9f87ce5792f5f952f8e5cdfaac8/content/browser/renderer_host/render_widget_host_impl.cc [modify] https://crrev.com/f7731345b34fd9f87ce5792f5f952f8e5cdfaac8/content/browser/renderer_host/render_widget_host_impl.h [modify] https://crrev.com/f7731345b34fd9f87ce5792f5f952f8e5cdfaac8/content/browser/renderer_host/render_widget_host_unittest.cc [modify] https://crrev.com/f7731345b34fd9f87ce5792f5f952f8e5cdfaac8/content/browser/renderer_host/render_widget_host_view_aura.cc [modify] https://crrev.com/f7731345b34fd9f87ce5792f5f952f8e5cdfaac8/content/browser/renderer_host/render_widget_host_view_aura.h [modify] https://crrev.com/f7731345b34fd9f87ce5792f5f952f8e5cdfaac8/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc [modify] https://crrev.com/f7731345b34fd9f87ce5792f5f952f8e5cdfaac8/content/browser/renderer_host/render_widget_host_view_base.h [modify] https://crrev.com/f7731345b34fd9f87ce5792f5f952f8e5cdfaac8/content/browser/renderer_host/render_widget_host_view_child_frame.cc [modify] https://crrev.com/f7731345b34fd9f87ce5792f5f952f8e5cdfaac8/content/browser/renderer_host/render_widget_host_view_child_frame.h [modify] https://crrev.com/f7731345b34fd9f87ce5792f5f952f8e5cdfaac8/content/browser/renderer_host/render_widget_host_view_mac.h [modify] https://crrev.com/f7731345b34fd9f87ce5792f5f952f8e5cdfaac8/content/browser/renderer_host/render_widget_host_view_mac.mm [modify] https://crrev.com/f7731345b34fd9f87ce5792f5f952f8e5cdfaac8/content/browser/screen_orientation/screen_orientation_browsertest.cc [modify] https://crrev.com/f7731345b34fd9f87ce5792f5f952f8e5cdfaac8/content/common/resize_params.cc [modify] https://crrev.com/f7731345b34fd9f87ce5792f5f952f8e5cdfaac8/content/common/resize_params.h [modify] https://crrev.com/f7731345b34fd9f87ce5792f5f952f8e5cdfaac8/content/common/view_messages.h [modify] https://crrev.com/f7731345b34fd9f87ce5792f5f952f8e5cdfaac8/content/public/test/render_view_test.cc [modify] https://crrev.com/f7731345b34fd9f87ce5792f5f952f8e5cdfaac8/content/renderer/render_frame_impl.cc [modify] https://crrev.com/f7731345b34fd9f87ce5792f5f952f8e5cdfaac8/content/renderer/render_frame_impl_browsertest.cc [modify] https://crrev.com/f7731345b34fd9f87ce5792f5f952f8e5cdfaac8/content/renderer/render_view_browsertest.cc [modify] https://crrev.com/f7731345b34fd9f87ce5792f5f952f8e5cdfaac8/content/renderer/render_view_impl.cc [modify] https://crrev.com/f7731345b34fd9f87ce5792f5f952f8e5cdfaac8/content/renderer/render_view_impl.h [modify] https://crrev.com/f7731345b34fd9f87ce5792f5f952f8e5cdfaac8/content/renderer/render_widget.cc [modify] https://crrev.com/f7731345b34fd9f87ce5792f5f952f8e5cdfaac8/content/renderer/render_widget.h
,
Jan 25 2018
|
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by fsam...@chromium.org
, Feb 23 2017Components: Internals>MUS Internals>Compositing
Labels: displaycompositor