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

Issue 691649 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature



Sign in to add a comment

EvictFrame, EvictSurface => EvictCurrentSurface

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

Issue description

A CompositorFrameSink can receive different Surfaces throughout its lifetime but it only holds onto one at any given time.

We want a frame eviction API on CompositorFrameSink but we've been using the names EvictFrame, and EvictSurface interchangably, and there's also a ClearSurface which further confuses things.

Perhaps EvictFrame, and EvictSurface should be renamed to EvictCurrentSurface. ClearSurface maintains the surface ID but clears the active CompositorFrame, so maybe we can call this ForceReclaimResources where it is actually used today.
 

Comment 1 by danakj@chromium.org, Feb 13 2017

Status: Available (was: Untriaged)

Comment 2 by danakj@chromium.org, Feb 13 2017

Cc: ericrk@chromium.org
> so maybe we can call this ForceReclaimResources where it is actually used today.

Sure, ericrk has a patch that deletes all ForceReclaimResources anyhow, tho it was reverted.
Ahh OK, here's the CL for reference: https://codereview.chromium.org/2609253003/
Are there any plans to revisit this patch?

Components: Internals>MUS Internals>Compositing
Labels: Type-Feature
Owner: staraz@chromium.org
Status: Assigned (was: Available)
Assigning to staraz@
Cc: weiliangc@chromium.org

Comment 8 by xing...@intel.com, May 11 2017

Hi, base on FrameEvictor will finally DestroyCurrentSurface at a proper time, I am going to doing the following rename: 
FrameEvictionManagerClient=>SurfaceEvictionManagerClient
FrameEvictionManager=>SurfaceEvictionManager
FrameEvictor=>SurfaceEvictor
Are you OK with this?

Comment 9 by staraz@chromium.org, May 11 2017

Owner: xing...@intel.com
Status: Started (was: Assigned)
Assigning to xing.xu since he already has CLs for this bug in review.
Project Member

Comment 10 by bugdroid1@chromium.org, May 12 2017

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

commit cdc09d7490c92687a35ebcc1265b0c03c43e8410
Author: xing.xu <xing.xu@intel.com>
Date: Fri May 12 00:08:50 2017

Rename EvictFrame, EvictSurface to EvictCurrentSurface

A Surface may be submitted with many CompositorFrames, Surface and Frame
are not the same concept.
Here use CompositorFrameSinkSupport::EvictFrame to destroy a Surface is
misleading, so rename to CompositorFrameSinkSupport::EvictCurrentSurface.

BUG= 691649 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2872883002
Cr-Commit-Position: refs/heads/master@{#471131}

[modify] https://crrev.com/cdc09d7490c92687a35ebcc1265b0c03c43e8410/android_webview/browser/hardware_renderer.cc
[modify] https://crrev.com/cdc09d7490c92687a35ebcc1265b0c03c43e8410/cc/ipc/mojo_compositor_frame_sink.mojom
[modify] https://crrev.com/cdc09d7490c92687a35ebcc1265b0c03c43e8410/cc/surfaces/compositor_frame_sink_support.cc
[modify] https://crrev.com/cdc09d7490c92687a35ebcc1265b0c03c43e8410/cc/surfaces/compositor_frame_sink_support.h
[modify] https://crrev.com/cdc09d7490c92687a35ebcc1265b0c03c43e8410/cc/surfaces/compositor_frame_sink_support_unittest.cc
[modify] https://crrev.com/cdc09d7490c92687a35ebcc1265b0c03c43e8410/cc/surfaces/display_unittest.cc
[modify] https://crrev.com/cdc09d7490c92687a35ebcc1265b0c03c43e8410/cc/surfaces/surface_aggregator_perftest.cc
[modify] https://crrev.com/cdc09d7490c92687a35ebcc1265b0c03c43e8410/cc/surfaces/surface_aggregator_unittest.cc
[modify] https://crrev.com/cdc09d7490c92687a35ebcc1265b0c03c43e8410/cc/surfaces/surface_hittest_unittest.cc
[modify] https://crrev.com/cdc09d7490c92687a35ebcc1265b0c03c43e8410/cc/surfaces/surface_manager_ref_unittest.cc
[modify] https://crrev.com/cdc09d7490c92687a35ebcc1265b0c03c43e8410/cc/surfaces/surface_synchronization_unittest.cc
[modify] https://crrev.com/cdc09d7490c92687a35ebcc1265b0c03c43e8410/cc/surfaces/surface_unittest.cc
[modify] https://crrev.com/cdc09d7490c92687a35ebcc1265b0c03c43e8410/cc/surfaces/surfaces_pixeltest.cc
[modify] https://crrev.com/cdc09d7490c92687a35ebcc1265b0c03c43e8410/components/viz/frame_sinks/gpu_compositor_frame_sink.cc
[modify] https://crrev.com/cdc09d7490c92687a35ebcc1265b0c03c43e8410/components/viz/frame_sinks/gpu_compositor_frame_sink.h
[modify] https://crrev.com/cdc09d7490c92687a35ebcc1265b0c03c43e8410/components/viz/frame_sinks/gpu_root_compositor_frame_sink.cc
[modify] https://crrev.com/cdc09d7490c92687a35ebcc1265b0c03c43e8410/components/viz/frame_sinks/gpu_root_compositor_frame_sink.h
[modify] https://crrev.com/cdc09d7490c92687a35ebcc1265b0c03c43e8410/content/browser/frame_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/cdc09d7490c92687a35ebcc1265b0c03c43e8410/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/cdc09d7490c92687a35ebcc1265b0c03c43e8410/content/browser/renderer_host/render_widget_host_impl.h
[modify] https://crrev.com/cdc09d7490c92687a35ebcc1265b0c03c43e8410/ui/android/delegated_frame_host_android.cc
[modify] https://crrev.com/cdc09d7490c92687a35ebcc1265b0c03c43e8410/ui/aura/local/compositor_frame_sink_local.cc

Cc: varkha@chromium.org

Comment 12 by fsamuel@google.com, May 23 2017

Status: Fixed (was: Started)
Blocking: -601863
Components: -Internals>MUS Internals>Services>WindowService

Sign in to add a comment