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

Issue 777881 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 787097



Sign in to add a comment

Frame eviction disabled with VizDisplayCompositor

Project Member Reported by fsam...@chromium.org, Oct 24 2017

Issue description

It seems like we skip DelegatedFrameHost::EvictDelegatedFrame with --enable-viz. We should investigate why and fix it.
 
Blocking: -601863 732572
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 18 2017

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

commit 5868bf0a1b0f6f23eb6f8a6fb08a88a4ba950f3d
Author: kylechar <kylechar@chromium.org>
Date: Mon Dec 18 15:59:35 2017

viz: Fix frame eviction logic with --enable-viz.

The FrameEvictionManager expects a response from
DelegatedFrameHost::EvictDelegatedFrame() otherwise it will DCHECK and
crash. Implement the correct logic in DelegatedFrameHost. There is no
CompositorFrameSinkSupport to clear, but we clear the SurfaceLayer which
will drop the surface reference to the evicted frame.

Bug:  777881 
Change-Id: Id6725d6cdb141c668b8217419535863ecf586a17
Reviewed-on: https://chromium-review.googlesource.com/829618
Commit-Queue: kylechar <kylechar@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524717}
[modify] https://crrev.com/5868bf0a1b0f6f23eb6f8a6fb08a88a4ba950f3d/content/browser/renderer_host/delegated_frame_host.cc

Project Member

Comment 3 by bugdroid1@chromium.org, 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

Project Member

Comment 4 by bugdroid1@chromium.org, 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

Project Member

Comment 5 by bugdroid1@chromium.org, Jan 31 2018

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

commit a438b798b2cbd89888bd2539335a0b790c2f799a
Author: Saman Sami <samans@chromium.org>
Date: Wed Jan 31 19:34:00 2018

content: Send ResizeParams in ViewMsg_WasShown

Don't send any WasResized messages when the RenderWidgetHostImpl is
hidden, and once it's visible, send the ResizeParams along with the
WasShown message. When a WasShown message that has ResizeParams
arrives in the renderer, the resize will be performed before setting
the visibility to true. This CL makes it easier to send a new
LocalSurfaceId to an evicted background tab when it becomes visible.
(Allocating a new ID will happen in a subsequent CL)

Bug:  777881 
Change-Id: Ibbc1a3604fb93806059d6beb3e51872f5aeaad4c
Reviewed-on: https://chromium-review.googlesource.com/894262
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Commit-Queue: Saman Sami <samans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533357}
[modify] https://crrev.com/a438b798b2cbd89888bd2539335a0b790c2f799a/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/a438b798b2cbd89888bd2539335a0b790c2f799a/content/browser/renderer_host/render_widget_host_impl.h
[modify] https://crrev.com/a438b798b2cbd89888bd2539335a0b790c2f799a/content/browser/renderer_host/render_widget_host_unittest.cc
[modify] https://crrev.com/a438b798b2cbd89888bd2539335a0b790c2f799a/content/common/view_messages.h
[modify] https://crrev.com/a438b798b2cbd89888bd2539335a0b790c2f799a/content/renderer/render_frame_impl_browsertest.cc
[modify] https://crrev.com/a438b798b2cbd89888bd2539335a0b790c2f799a/content/renderer/render_widget.cc
[modify] https://crrev.com/a438b798b2cbd89888bd2539335a0b790c2f799a/content/renderer/render_widget.h

Project Member

Comment 6 by bugdroid1@chromium.org, Feb 3 2018

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

commit d709592b7ce56fc6106d3a30cf28447c44f755f6
Author: Saman Sami <samans@chromium.org>
Date: Sat Feb 03 00:38:10 2018

Revert "content: Send ResizeParams in ViewMsg_WasShown"

This reverts commit a438b798b2cbd89888bd2539335a0b790c2f799a.

Reason for revert: Causes  crbug.com/808282 

Original change's description:
> content: Send ResizeParams in ViewMsg_WasShown
> 
> Don't send any WasResized messages when the RenderWidgetHostImpl is
> hidden, and once it's visible, send the ResizeParams along with the
> WasShown message. When a WasShown message that has ResizeParams
> arrives in the renderer, the resize will be performed before setting
> the visibility to true. This CL makes it easier to send a new
> LocalSurfaceId to an evicted background tab when it becomes visible.
> (Allocating a new ID will happen in a subsequent CL)
> 
> Bug:  777881 
> Change-Id: Ibbc1a3604fb93806059d6beb3e51872f5aeaad4c
> Reviewed-on: https://chromium-review.googlesource.com/894262
> Reviewed-by: Antoine Labour <piman@chromium.org>
> Reviewed-by: Tom Sepez <tsepez@chromium.org>
> Reviewed-by: Fady Samuel <fsamuel@chromium.org>
> Commit-Queue: Saman Sami <samans@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#533357}

TBR=fsamuel@chromium.org,tsepez@chromium.org,piman@chromium.org,samans@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  777881 , 808282 
Change-Id: I71691467ef1868042d43bc4280bb55a0d71e859d
Reviewed-on: https://chromium-review.googlesource.com/899924
Reviewed-by: Saman Sami <samans@chromium.org>
Commit-Queue: Saman Sami <samans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534211}
[modify] https://crrev.com/d709592b7ce56fc6106d3a30cf28447c44f755f6/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/d709592b7ce56fc6106d3a30cf28447c44f755f6/content/browser/renderer_host/render_widget_host_impl.h
[modify] https://crrev.com/d709592b7ce56fc6106d3a30cf28447c44f755f6/content/browser/renderer_host/render_widget_host_unittest.cc
[modify] https://crrev.com/d709592b7ce56fc6106d3a30cf28447c44f755f6/content/common/view_messages.h
[modify] https://crrev.com/d709592b7ce56fc6106d3a30cf28447c44f755f6/content/renderer/render_frame_impl_browsertest.cc
[modify] https://crrev.com/d709592b7ce56fc6106d3a30cf28447c44f755f6/content/renderer/render_widget.cc
[modify] https://crrev.com/d709592b7ce56fc6106d3a30cf28447c44f755f6/content/renderer/render_widget.h

Project Member

Comment 7 by bugdroid1@chromium.org, Feb 5 2018

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

commit 7de3a34e7bd34badc8265f43333e164abf6a8e52
Author: Saman Sami <samans@chromium.org>
Date: Mon Feb 05 20:17:04 2018

Surface synchronization: Allocate new LocalSurfaceId on eviction

By allocating a new LocalSurfaceId on eviction, we can mitigate the
flash that's seen by the user when switching to an evicted tab. This CL
also fixes the race with enable-viz where the renderer submits to a
surface that is getting destroyed and we end up not showing anything.

Bug:  777881 
Change-Id: I5d8ec017ea6a305607664a0f542f5b3abcf4f17f
Reviewed-on: https://chromium-review.googlesource.com/897997
Commit-Queue: Saman Sami <samans@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534481}
[modify] https://crrev.com/7de3a34e7bd34badc8265f43333e164abf6a8e52/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/7de3a34e7bd34badc8265f43333e164abf6a8e52/content/browser/renderer_host/delegated_frame_host.h
[modify] https://crrev.com/7de3a34e7bd34badc8265f43333e164abf6a8e52/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/7de3a34e7bd34badc8265f43333e164abf6a8e52/content/browser/renderer_host/render_widget_host_view_aura.h
[modify] https://crrev.com/7de3a34e7bd34badc8265f43333e164abf6a8e52/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Feb 5 2018

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

commit 6daa087496d308f6af09c21064126ba9c4bdb869
Author: Saman Sami <samans@chromium.org>
Date: Mon Feb 05 23:38:22 2018

viz: Add method to mojom::FrameSinkManager to evict surfaces

Send the SurfaceIds that we want evicted to HostFrameSinkManager instead
of directly to CompositorFrameSinkSupport. This will allow us to do frame
eviction with enable-viz. The added method takes an array of SurfaceIds
to enable evicting the main frame with all its OOPIFS in one go (will
happen in a separate CL).

Bug:  777881 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I6839f16718e9a9b169ae26d905cdade430c53d18
Reviewed-on: https://chromium-review.googlesource.com/897892
Commit-Queue: Saman Sami <samans@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534536}
[modify] https://crrev.com/6daa087496d308f6af09c21064126ba9c4bdb869/components/viz/host/host_frame_sink_manager.cc
[modify] https://crrev.com/6daa087496d308f6af09c21064126ba9c4bdb869/components/viz/host/host_frame_sink_manager.h
[modify] https://crrev.com/6daa087496d308f6af09c21064126ba9c4bdb869/components/viz/service/frame_sinks/frame_sink_manager_impl.cc
[modify] https://crrev.com/6daa087496d308f6af09c21064126ba9c4bdb869/components/viz/service/frame_sinks/frame_sink_manager_impl.h
[modify] https://crrev.com/6daa087496d308f6af09c21064126ba9c4bdb869/components/viz/service/frame_sinks/frame_sink_manager_unittest.cc
[modify] https://crrev.com/6daa087496d308f6af09c21064126ba9c4bdb869/components/viz/test/test_frame_sink_manager.h
[modify] https://crrev.com/6daa087496d308f6af09c21064126ba9c4bdb869/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/6daa087496d308f6af09c21064126ba9c4bdb869/services/viz/privileged/interfaces/compositing/frame_sink_manager.mojom

Blocking: -732572 -775030 787097
Components: -Internals>Compositing Internals>Services>Viz
Summary: Frame eviction disabled with VizDisplayCompositor (was: Frame eviction disabled with --enable-viz)
samans: Is this fixed?
I still have future plans for frame eviction but already it works with viz.
Status: Fixed (was: Assigned)

Sign in to add a comment