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

Issue 657959 link

Starred by 7 users
Status: Started
Owner:
kylec...@chromium.org
Cc:
danakj@chromium.org
█ mhashmi@chromium.org
weiliangc@chromium.org
█ vollick@chromium.org
█ staraz@chromium.org
samans@chromium.org
rjkroege@chromium.org
sadrul@chromium.org
piman@chromium.org
riajiang@chromium.org
kylec...@chromium.org
enne@chromium.org
sky@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature

Blocked on:
issue 676384
issue 732900
issue 732903


Sign in to add a comment

Eliminate Direct Access To FrameSinkManagerImpl (from chrome UI and renderer)

Project Member Reported by fsam...@chromium.org, Oct 20 2016 
In order to pull the display compositor out of the browser process, we need to remove direct access to SurfaceManager among other things. We should identify how SurfaceManager is used today and wrap all accesses to SurfaceManager instead of having a "GetSurfaceManager()" accessor. I think this primarily means ui::ContextFactory should not have a GetSurfaceManager().

Maybe we can replace those with individual methods that delegate to calls on SurfaceManager methods?

Comment 1 by enne@chromium.org, Oct 20 2016 

I think there's two pieces here.  One is submitting things to SurfaceManager, which I think will be subsumed by mojo APIs for CompositorFrameSink-like things.  The second is read-only access to SurfaceManager, for all the things that use cc::SurfaceHitTest for input routing and transforming points into local space.  We probably need a second interface for this (SurfaceManagerProxy?) that will eventually get forwarded hit testing data from elsewhere and can live wherever input event routing lives.

Comment 2 by fsam...@chromium.org, Oct 20 2016 

rjkroege@, and sadrul@ are thinking about hit testing. I think a mechanical refactor for hit testing won't provide good performance. We effectively plan to cache hit regions in the browser/window server and update those regions once per Display::DrawAndSwap.

Comment 3 by piman@chromium.org, Nov 4 2016 

Took a look at how it's used today. Currently, it's either ui::ContextFactory::GetSurfaceManager() (i.e. GpuProcessTransportFactory::surface_manager_) for Aura + Mac, ui::ContextProviderFactory::GetSurfaceManager() (i.e. ContextProviderFactoryImpl::surface_manager_) for Clank/Blimp, and SurfacesInstance::surface_manager_ for Android WebView. It is used for 7 namely separate things:

1- registering/invalidating FrameSinkIds, for things than need or support a CompositorFrameSink: AW's HardwareRenderer and SurfacesInstance itself, Blimp's BlimpCompositor and BlimpEmbedderCompositor, renderer host (RenderWidgetHostViewChildFrame/DelegatedFrameHost/DelegatedFrameHostAndroid, as well as OffscreenCanvasCompositorFrameSink), browser compositor (ui::Compositor and android's CompositorImpl),  as well as exo's Surface (which doesn't use a CompositorFrameSink, instead manually creates CompositorFrames). There's also ServerWindowCompositorFrameSink, but that's for mus-ws so I think we don't need to worry about it for this specifically (?).

2- creating a SurfaceFactory, to put CompositorFrames into surfaces. Pretty much all the same things, possibly indirectly through DirectCompositorFrameSink for those that use it to do Display (BlimpEmbedderCompositor, ui::Compositor, CompositorImpl).

3- registering/unregistering SurfaceFactoryClients, to get the BeginFrameSource. Pretty much the same things again, except some things don't do it - they (currently) skip UBF: BlimpCompositor/BlimpEmbedderCompositor (use a DelayBasedBeginFrameSource), OffscreenCanvasCompositorFrameSink (punt), exo (punt).

4- implementing Satisfy/Require callbacks for cc::SurfaceLayer - basically all things that embed other things (copy-pasta everywhere): BlimpCompositor, exo's Surface, BrowserPluginGuest (<webview> tag), CrossProcessFrameConnector (OOPIF), DelegatedFrameHost, DelegatedFrameHostAndroid, OffscreenCanvasSurfaceImpl. There's also mus-ws things (DisplayCompositor, FrameGenerator)

5- registering/unregistering FrameSink hierarchy - things that embed other things that also need UBF: ui::Compositor (on behalf of DelegatedFrameHost), DelegatedFrameHostAndroid, and RenderWidgetHostViewChildFrame (for OOPIF and <webview> tag)

6- to give to the Display - virtually everything through DirectCompositorFrameSink, except SurfacesInstance

7- To implement SurfaceHittest - things that embed other things that also want to route input with surfaces, or want to do coordinate transforms: CrossProcessFrameConnector, DelegatedFrameHost



I think it would be possible conceptually to put 1+2+3+5 behind an interface ("CompositorFrameSinkSupport"? names...) and hopefully a single implementation, swallowed by the ContextFactory/ContextProviderFactory/SurfacesInstance, making the SurfaceManager+SurfaceFactory an implementation detail, so that it can be refactored to go OOP - maybe entirely subsumed by the eventual mojo interface.
Maybe 4 could be part of that same interface, though  crbug.com/659227  may make this irrelevant, or at the very least, different.

6 is maybe not much of a problem at this point, because things that create the Display or DirectCompositorFrameSink are the same things that own the SurfaceManager today. This is the part that will clearly move to the gpu process as a block. Well, except BlimpEmbedderCompositor, will need to figure out that one (maybe ContextProviderFactory needs to swallow more of this functionality).

7 is an entirely different problem as mentioned above

Comment 4 by fsam...@chromium.org, Nov 4 2016 

It would be nice to have a single "GpuCompositorFrameSink" implementation that handles the commonalities across all these clients. Instead of implementing almost the same thing in half a dozen or so clients, it would be nice to have all these clients get an implementation that is largely agnostic to what the client is doing with it. In fact, I am currently in the process of decoupling ServerWindowCompositorFrameSink from the window server, putting it in services/ui/gpu and calling it GpuCompositorFrameSInk.

1. This will change significantly / go away with surface references instead of SurfaceSequences. Registering FrameSinkIds exists to keep SurfaceSequences valid (they are a [FrameSinkId, sequence num] pair). References will be kept more explicitly with the new approach. kylechar@ is working out the implementation details.

2. I eventually want to merge SurfaceFactory and GpuCompositorFrameSink. samans@ is working towards that.

3. We plan to make exo use GpuCompositorFrameSink. OffscreenCanvasCompositorFrameSink will likely go away in favour of GpuCompositorFrameSink.

4. These will change with surface references.

5. MojoCompositorFrameSinkPrivate will take care of that.

6. These details we haven't thought about too much on Mus+Ash.

7. This is a significant amount of work.

What you're calling CompositorFrameSinkSupport, I was planning on calling it GpuCompositorFrameSink (and I'm currently morphing ServerWindowCompositorFrameSink into that), but yes.

Ideally, we'd like to replace gpu_child_thread, and what you've called CompositorFrameSupport in content/ today with code in services/ui/gpu making mus-gpu, and display-compositor-in-gpu share all the same code and thus unifying the efforts. Chrome can theoretically use services/ui/gpu without services/ui/ws in the interim. WDYT?

Note, that services/ui/ws is mus-ws, and services/ui/gpu is mus-gpu.

Comment 5 by fsam...@chromium.org, Nov 4 2016

Cc: kylec...@chromium.org staraz@chromium.org
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 7 2016 

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

commit 50ca188c4d5206a8475c76133d69acc066ee3856
Author: piman <piman@chromium.org>
Date: Mon Nov 07 19:50:01 2016

Add SurfaceFactory::Reset

This can be used to keep the SurfaceFactory alive across CompositorFrameSink
loss, to have its lifetime matchup better with the frame_sink_id_ registration,
allowing further refactoring / simplification

BUG=657959
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

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

[modify] https://crrev.com/50ca188c4d5206a8475c76133d69acc066ee3856/cc/surfaces/surface.cc
[modify] https://crrev.com/50ca188c4d5206a8475c76133d69acc066ee3856/cc/surfaces/surface.h
[modify] https://crrev.com/50ca188c4d5206a8475c76133d69acc066ee3856/cc/surfaces/surface_factory.cc
[modify] https://crrev.com/50ca188c4d5206a8475c76133d69acc066ee3856/cc/surfaces/surface_factory.h
[modify] https://crrev.com/50ca188c4d5206a8475c76133d69acc066ee3856/cc/surfaces/surface_factory_unittest.cc
[modify] https://crrev.com/50ca188c4d5206a8475c76133d69acc066ee3856/cc/surfaces/surface_resource_holder.cc
[modify] https://crrev.com/50ca188c4d5206a8475c76133d69acc066ee3856/cc/surfaces/surface_resource_holder.h
Project Member

Comment 7 by bugdroid1@chromium.org, Nov 7 2016 

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

commit dc6b7bfdc4cc7c84bf4b9efb15f7631125b3678d
Author: piman <piman@chromium.org>
Date: Mon Nov 07 22:42:27 2016

Make SurfaceFactory lifetime match frame_sink_id_ registration in Android WebView

This will allow further simplification/refactoring.

BUG=657959

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

[modify] https://crrev.com/dc6b7bfdc4cc7c84bf4b9efb15f7631125b3678d/android_webview/browser/hardware_renderer.cc
[modify] https://crrev.com/dc6b7bfdc4cc7c84bf4b9efb15f7631125b3678d/android_webview/browser/surfaces_instance.cc
Project Member

Comment 8 by bugdroid1@chromium.org, Nov 8 2016 

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

commit 65473635222ea5062169a2861e641019fcda1efb
Author: piman <piman@chromium.org>
Date: Tue Nov 08 00:48:54 2016

Make SurfaceFactory lifetime match frame_sink_id_ registration in DelegatedFrameHost

This will allow further simplification/refactoring.

BUG=657959

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

[modify] https://crrev.com/65473635222ea5062169a2861e641019fcda1efb/content/browser/renderer_host/delegated_frame_host.cc
Project Member

Comment 9 by bugdroid1@chromium.org, Nov 8 2016 

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

commit a5abb8e899397924617f1bac3c932146c2e5826e
Author: piman <piman@chromium.org>
Date: Tue Nov 08 01:59:01 2016

Make SurfaceFactory lifetime match frame_sink_id_ registration in BlimpCompositor

This will allow further simplification/refactoring.

BUG=657959

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

[modify] https://crrev.com/a5abb8e899397924617f1bac3c932146c2e5826e/blimp/client/core/compositor/blimp_compositor.cc
[modify] https://crrev.com/a5abb8e899397924617f1bac3c932146c2e5826e/blimp/client/core/compositor/blimp_compositor.h
Project Member

Comment 10 by bugdroid1@chromium.org, Nov 8 2016 

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

commit d6da33fd77e168bce08dfe17685e63785d4fb38f
Author: piman <piman@chromium.org>
Date: Tue Nov 08 18:35:35 2016

Make SurfaceFactory lifetime match frame_sink_id_ registration in DelegatedFrameHostAndroid

This will allow further simplification/refactoring.

BUG=657959

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

[modify] https://crrev.com/d6da33fd77e168bce08dfe17685e63785d4fb38f/ui/android/delegated_frame_host_android.cc
Project Member

Comment 11 by bugdroid1@chromium.org, Nov 8 2016 

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

commit ba306aec9d9b09a427c4f95f095a8c039c56691a
Author: piman <piman@chromium.org>
Date: Tue Nov 08 18:36:37 2016

Make SurfaceFactory lifetime match frame_sink_id registration in OffscreenCanvasCompositorFrameSink

This will allow further simplification/refactoring.

BUG=657959

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

[modify] https://crrev.com/ba306aec9d9b09a427c4f95f095a8c039c56691a/content/browser/renderer_host/offscreen_canvas_compositor_frame_sink.cc
Project Member

Comment 12 by bugdroid1@chromium.org, Nov 9 2016 

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

commit caabb8122c93adfa618f0acb8cf4ce8445c0e52a
Author: piman <piman@chromium.org>
Date: Wed Nov 09 16:29:33 2016

Make SurfaceFactory lifetime match frame_sink_id_ registration in RWHVChildFrame

This will allow further simplification/refactoring.

BUG=657959
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[modify] https://crrev.com/caabb8122c93adfa618f0acb8cf4ce8445c0e52a/content/browser/frame_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/caabb8122c93adfa618f0acb8cf4ce8445c0e52a/content/browser/frame_host/render_widget_host_view_guest.cc

Comment 13 by fsam...@chromium.org, Nov 10 2016

Cc: mhashmi@chromium.org
 Issue 659592  has been merged into this issue.

Comment 14 by fsam...@chromium.org, Nov 17 2016

Owner: fsam...@chromium.org
Status: Assigned (was: Untriaged)
Assigning to myself, but really this is a metabug, I'll try to dice this more tomorrow.

Comment 15 by fsam...@chromium.org, Dec 1 2016

Blocking: 601863
Components: MUS
Labels: Proj-Mustash-Mus displaycompositor
Project Member

Comment 16 by bugdroid1@chromium.org, Dec 3 2016 

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

commit 932aa65ca04713744f3166c973c19560c1faa75f
Author: fsamuel <fsamuel@chromium.org>
Date: Sat Dec 03 02:38:50 2016

Extract non-Mojo bits of GpuCompositorFrameSink to CompositorFrameSinkSupport

We would like to have all Chrome clients use a single service-side
CompositorFrameSink implementation as an incremental step towards pulling the
display compositor into the gpu process. This CL extract non-mojo-specific
code from GpuCompositorFrameSink in services/ui to CompositorFrameSinkSupport
in a central place: cc/surfaces that can be used by Chrome today. Individual
clients can transition to mojo after we carefully measure the performance
impact of the transition.

BUG= 668126 , 657959
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

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

[modify] https://crrev.com/932aa65ca04713744f3166c973c19560c1faa75f/cc/surfaces/BUILD.gn
[add] https://crrev.com/932aa65ca04713744f3166c973c19560c1faa75f/cc/surfaces/compositor_frame_sink_support.cc
[add] https://crrev.com/932aa65ca04713744f3166c973c19560c1faa75f/cc/surfaces/compositor_frame_sink_support.h
[add] https://crrev.com/932aa65ca04713744f3166c973c19560c1faa75f/cc/surfaces/compositor_frame_sink_support_client.h
[modify] https://crrev.com/932aa65ca04713744f3166c973c19560c1faa75f/services/ui/surfaces/gpu_compositor_frame_sink.cc
[modify] https://crrev.com/932aa65ca04713744f3166c973c19560c1faa75f/services/ui/surfaces/gpu_compositor_frame_sink.h

Comment 17 by fsam...@chromium.org, Dec 5 2016 

Both ImageTransportFactory and ContextFactory have a GetSurfaceManager. I'll see if I can get rid of the ImageTransportFactory one to centralize things that need to change a bit.
Project Member

Comment 18 by bugdroid1@chromium.org, Dec 7 2016 

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

commit f3812452a1a442002c99a61737ebfd06a963b9dd
Author: fsamuel <fsamuel@chromium.org>
Date: Wed Dec 07 01:53:47 2016

Get rid of ImageTransportFactory::GetSurfaceManager

SurfaceManager was accessible through both ImageTransportFactory AND
ContextFactory. This CL removes the accessor on ImageTransportFactory
which is redundant.

BUG=657959
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[modify] https://crrev.com/f3812452a1a442002c99a61737ebfd06a963b9dd/content/browser/compositor/image_transport_factory.h
[modify] https://crrev.com/f3812452a1a442002c99a61737ebfd06a963b9dd/content/browser/compositor/surface_utils.cc
[modify] https://crrev.com/f3812452a1a442002c99a61737ebfd06a963b9dd/content/browser/compositor/test/no_transport_image_transport_factory.cc
[modify] https://crrev.com/f3812452a1a442002c99a61737ebfd06a963b9dd/content/browser/compositor/test/no_transport_image_transport_factory.h
[modify] https://crrev.com/f3812452a1a442002c99a61737ebfd06a963b9dd/content/browser/frame_host/render_widget_host_view_child_frame_unittest.cc
[modify] https://crrev.com/f3812452a1a442002c99a61737ebfd06a963b9dd/content/browser/frame_host/render_widget_host_view_guest_unittest.cc
[modify] https://crrev.com/f3812452a1a442002c99a61737ebfd06a963b9dd/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/f3812452a1a442002c99a61737ebfd06a963b9dd/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc

Comment 19 by fsam...@chromium.org, Jan 16 2017

Labels: -Pri-3 Pri-2

Comment 20 by fsam...@chromium.org, Jan 22 2017

Blocking:

Comment 21 by fsam...@chromium.org, Jan 26 2017

Components: Internals>Compositing

Comment 22 by fsam...@chromium.org, Feb 6 2017 

There is a bit of weirdness here. When running in Mus+Ash currently, Chrome does not use ContextFactoryPrivate (only ContextFactory) and it does not use GpuProcessTransportFactory. DelegatedFrameHost is essentially dead code, as is GpuProcessTransportFactory and various other things in that configuration. I think there's a lot of value to unifying this code beyond simply code cleanup:

1. We get to test code shared with Mus+Ash in the wild sooner rather than later and develop a higher level of confidence that we've addressed corner cases.

2. We develop a higher level of confidence that we have the right architecture and have implemented all the features we need for launching Mus+Ash.

Comment 23 by fsam...@chromium.org, Apr 7 2017

Cc: piman@chromium.org
Project Member

Comment 24 by bugdroid1@chromium.org, Apr 10 2017 

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

commit 69bd9163b311213bb181832c104e4cab2c2e12b7
Author: kylechar <kylechar@chromium.org>
Date: Mon Apr 10 23:45:48 2017

Introduce FrameSinkManagerHost in content/browser/.

FrameSinkManagerHost is an object that will live in the browser process
and manage CompositorFrameSinks. The browser process accesses
SurfaceManager directly to do this now. The goal is eliminate all
direct access to SurfaceManager and move it to another process.

FrameSinkManagerHost owns MojoFrameSinkManager in the short term but
still uses Mojo to access it. The only impact of this change is the
ownership of SurfaceManager is moved. Subsequent CLs can start using
FrameSinkManagerHost to manage CompositorFrameSinks and eliminate direct
access to SurfaceManager.

BUG=657959, 664547 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[modify] https://crrev.com/69bd9163b311213bb181832c104e4cab2c2e12b7/content/browser/BUILD.gn
[modify] https://crrev.com/69bd9163b311213bb181832c104e4cab2c2e12b7/content/browser/DEPS
[add] https://crrev.com/69bd9163b311213bb181832c104e4cab2c2e12b7/content/browser/compositor/frame_sink_manager_host.cc
[add] https://crrev.com/69bd9163b311213bb181832c104e4cab2c2e12b7/content/browser/compositor/frame_sink_manager_host.h
[modify] https://crrev.com/69bd9163b311213bb181832c104e4cab2c2e12b7/content/browser/compositor/gpu_process_transport_factory.cc
[modify] https://crrev.com/69bd9163b311213bb181832c104e4cab2c2e12b7/content/browser/compositor/gpu_process_transport_factory.h
[modify] https://crrev.com/69bd9163b311213bb181832c104e4cab2c2e12b7/content/browser/compositor/image_transport_factory.h
[modify] https://crrev.com/69bd9163b311213bb181832c104e4cab2c2e12b7/content/browser/compositor/surface_utils.cc
[modify] https://crrev.com/69bd9163b311213bb181832c104e4cab2c2e12b7/content/browser/compositor/surface_utils.h
[modify] https://crrev.com/69bd9163b311213bb181832c104e4cab2c2e12b7/content/browser/compositor/test/no_transport_image_transport_factory.cc
[modify] https://crrev.com/69bd9163b311213bb181832c104e4cab2c2e12b7/content/browser/compositor/test/no_transport_image_transport_factory.h
[modify] https://crrev.com/69bd9163b311213bb181832c104e4cab2c2e12b7/content/browser/frame_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/69bd9163b311213bb181832c104e4cab2c2e12b7/content/browser/renderer_host/compositor_impl_android.cc
[modify] https://crrev.com/69bd9163b311213bb181832c104e4cab2c2e12b7/content/browser/renderer_host/compositor_impl_android.h

Comment 25 by fsam...@chromium.org, Apr 19 2017

Components: Internals>MUS
Labels: Type-Feature
Project Member

Comment 26 by bugdroid1@chromium.org, Apr 26 2017 

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

commit bfc507f8797d610a272dd3dbcf264825b1cc5384
Author: kylechar <kylechar@chromium.org>
Date: Wed Apr 26 00:13:08 2017

Add FrameSinkManagerHost to NoTransportImageTransportFactory.

This will allow FrameSinkManagerHost to be used in tests. Most tests
don't actually need the Mojo connection between FrameSinkManager and
MojoFrameSinkManager, so don't establish that connection in
FrameSinkManagerHost constructor.

BUG=657959

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

[modify] https://crrev.com/bfc507f8797d610a272dd3dbcf264825b1cc5384/components/viz/frame_sinks/mojo_frame_sink_manager.cc
[modify] https://crrev.com/bfc507f8797d610a272dd3dbcf264825b1cc5384/components/viz/frame_sinks/mojo_frame_sink_manager.h
[modify] https://crrev.com/bfc507f8797d610a272dd3dbcf264825b1cc5384/content/browser/compositor/frame_sink_manager_host.cc
[modify] https://crrev.com/bfc507f8797d610a272dd3dbcf264825b1cc5384/content/browser/compositor/frame_sink_manager_host.h
[modify] https://crrev.com/bfc507f8797d610a272dd3dbcf264825b1cc5384/content/browser/compositor/gpu_process_transport_factory.cc
[modify] https://crrev.com/bfc507f8797d610a272dd3dbcf264825b1cc5384/content/browser/compositor/test/no_transport_image_transport_factory.cc
[modify] https://crrev.com/bfc507f8797d610a272dd3dbcf264825b1cc5384/content/browser/compositor/test/no_transport_image_transport_factory.h
[modify] https://crrev.com/bfc507f8797d610a272dd3dbcf264825b1cc5384/content/browser/renderer_host/compositor_impl_android.cc
[modify] https://crrev.com/bfc507f8797d610a272dd3dbcf264825b1cc5384/services/ui/gpu/gpu_main.cc
Project Member

Comment 27 by bugdroid1@chromium.org, May 2 2017 

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

commit 8b18a094308ec78705a5aa9aaadd90456b011c61
Author: kylechar <kylechar@chromium.org>
Date: Tue May 02 22:10:55 2017

Add observers to FrameSinkManagerHost.

Add list of in process observers to FrameSinkManagerHost. These work the
same way as SurfaceManager observers, except there is an asynchronous
Mojo call from MojoFrameSinkManager to FrameSinkManagerHost first.

BUG=657959

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

[modify] https://crrev.com/8b18a094308ec78705a5aa9aaadd90456b011c61/content/browser/BUILD.gn
[modify] https://crrev.com/8b18a094308ec78705a5aa9aaadd90456b011c61/content/browser/compositor/OWNERS
[modify] https://crrev.com/8b18a094308ec78705a5aa9aaadd90456b011c61/content/browser/compositor/frame_sink_manager_host.cc
[modify] https://crrev.com/8b18a094308ec78705a5aa9aaadd90456b011c61/content/browser/compositor/frame_sink_manager_host.h
[add] https://crrev.com/8b18a094308ec78705a5aa9aaadd90456b011c61/content/browser/compositor/frame_sink_observer.h

Comment 28 by fsam...@chromium.org, May 3 2017

Cc: weiliangc@chromium.org

Comment 29 by fsam...@chromium.org, May 23 2017

Cc: varkha@chromium.org
Project Member

Comment 30 by bugdroid1@chromium.org, Jun 6 2017 

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

commit cb5882d827940028dcc22c75ff106ffa3ea3b177
Author: kylechar <kylechar@chromium.org>
Date: Tue Jun 06 14:47:42 2017

Make FrameSinkManagerHost usable from //ui/compositor.

The current location of FrameSinkManagerHost won't work if we want to
replace SurfaceManager with FrameSinkManagerHost. SurfaceManager is used
by //ui/compositor which cannot depend on //content/browser.

Move FrameSinkManagerHost to a new component in //components/viz/host.
Also move the singleton that provides a GetFrameSinkManagerHost()
function from ImageTransportFactory to ContextFactoryPrivate.

Bug: 657959
Change-Id: Ib1e9087e9acba2603a67ea478a7ad398730cbbb6
Reviewed-on: https://chromium-review.googlesource.com/521765
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#477284}
[add] https://crrev.com/cb5882d827940028dcc22c75ff106ffa3ea3b177/components/viz/host/BUILD.gn
[add] https://crrev.com/cb5882d827940028dcc22c75ff106ffa3ea3b177/components/viz/host/DEPS
[add] https://crrev.com/cb5882d827940028dcc22c75ff106ffa3ea3b177/components/viz/host/OWNERS
[rename] https://crrev.com/cb5882d827940028dcc22c75ff106ffa3ea3b177/components/viz/host/frame_sink_manager_host.cc
[rename] https://crrev.com/cb5882d827940028dcc22c75ff106ffa3ea3b177/components/viz/host/frame_sink_manager_host.h
[rename] https://crrev.com/cb5882d827940028dcc22c75ff106ffa3ea3b177/components/viz/host/frame_sink_observer.h
[add] https://crrev.com/cb5882d827940028dcc22c75ff106ffa3ea3b177/components/viz/host/viz_host_export.h
[modify] https://crrev.com/cb5882d827940028dcc22c75ff106ffa3ea3b177/content/browser/BUILD.gn
[modify] https://crrev.com/cb5882d827940028dcc22c75ff106ffa3ea3b177/content/browser/compositor/OWNERS
[modify] https://crrev.com/cb5882d827940028dcc22c75ff106ffa3ea3b177/content/browser/compositor/gpu_process_transport_factory.cc
[modify] https://crrev.com/cb5882d827940028dcc22c75ff106ffa3ea3b177/content/browser/compositor/gpu_process_transport_factory.h
[modify] https://crrev.com/cb5882d827940028dcc22c75ff106ffa3ea3b177/content/browser/compositor/image_transport_factory.h
[modify] https://crrev.com/cb5882d827940028dcc22c75ff106ffa3ea3b177/content/browser/compositor/surface_utils.cc
[modify] https://crrev.com/cb5882d827940028dcc22c75ff106ffa3ea3b177/content/browser/compositor/surface_utils.h
[modify] https://crrev.com/cb5882d827940028dcc22c75ff106ffa3ea3b177/content/browser/compositor/test/no_transport_image_transport_factory.cc
[modify] https://crrev.com/cb5882d827940028dcc22c75ff106ffa3ea3b177/content/browser/compositor/test/no_transport_image_transport_factory.h
[modify] https://crrev.com/cb5882d827940028dcc22c75ff106ffa3ea3b177/content/browser/renderer_host/compositor_impl_android.cc
[modify] https://crrev.com/cb5882d827940028dcc22c75ff106ffa3ea3b177/content/browser/renderer_host/compositor_impl_android.h
[modify] https://crrev.com/cb5882d827940028dcc22c75ff106ffa3ea3b177/content/browser/renderer_host/offscreen_canvas_provider_impl_unittest.cc
[modify] https://crrev.com/cb5882d827940028dcc22c75ff106ffa3ea3b177/content/browser/renderer_host/offscreen_canvas_surface_impl.cc
[modify] https://crrev.com/cb5882d827940028dcc22c75ff106ffa3ea3b177/content/browser/renderer_host/offscreen_canvas_surface_impl.h
[modify] https://crrev.com/cb5882d827940028dcc22c75ff106ffa3ea3b177/content/test/BUILD.gn
[modify] https://crrev.com/cb5882d827940028dcc22c75ff106ffa3ea3b177/ui/aura/BUILD.gn
[modify] https://crrev.com/cb5882d827940028dcc22c75ff106ffa3ea3b177/ui/aura/demo/DEPS
[modify] https://crrev.com/cb5882d827940028dcc22c75ff106ffa3ea3b177/ui/aura/demo/demo_main.cc
[modify] https://crrev.com/cb5882d827940028dcc22c75ff106ffa3ea3b177/ui/compositor/BUILD.gn
[modify] https://crrev.com/cb5882d827940028dcc22c75ff106ffa3ea3b177/ui/compositor/DEPS
[modify] https://crrev.com/cb5882d827940028dcc22c75ff106ffa3ea3b177/ui/compositor/compositor.h
[modify] https://crrev.com/cb5882d827940028dcc22c75ff106ffa3ea3b177/ui/compositor/test/context_factories_for_test.cc
[modify] https://crrev.com/cb5882d827940028dcc22c75ff106ffa3ea3b177/ui/compositor/test/in_process_context_factory.cc
[modify] https://crrev.com/cb5882d827940028dcc22c75ff106ffa3ea3b177/ui/compositor/test/in_process_context_factory.h
[modify] https://crrev.com/cb5882d827940028dcc22c75ff106ffa3ea3b177/ui/views/examples/BUILD.gn
[modify] https://crrev.com/cb5882d827940028dcc22c75ff106ffa3ea3b177/ui/views/examples/DEPS
[modify] https://crrev.com/cb5882d827940028dcc22c75ff106ffa3ea3b177/ui/views/examples/examples_main.cc

Comment 31 by danakj@chromium.org, Jun 6 2017

Blocking: 730193

Comment 32 by danakj@chromium.org, Jun 6 2017

Blocking: 676384

Comment 33 by fsam...@chromium.org, Jun 12 2017

Owner: kylec...@chromium.org

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

Blockedon: 676384
Spent some time understanding the state of this with fsamuel:

Surface reference APIs go thru MojoCompositorFrameSinkPrivate (which is an interface given by FrameSinkManagerHost), whereas surface sequences APIs go directly through SurfaceManager. So 676384 will eliminate many uses of SurfaceManager.

Some copy requests go thru Surface, but that is Android-only. Ignoring for now. But Android would need to request through MojoCompositorFrameSinkPrivate instead, which exists once the UI compositor uses a MojoCompositorFrameSink instead of DirectCompositorFrameSink.

Both ui::Compositor and android CompositorImpl need to migrate from DirectCompositorFrameSink to MojoCompositorFrameSink behind a flag in order to use viz. This is 730213 which is setting up a runtime flag to go thru mojo APIs. That'd unblock Android but isn't part of SurfaceManager problems on desktop AFAIK so not a blocker here for now.

ui::Compositor uses SurfaceManager to register frame sink hierarchies. This will be replaced by calls to FrameSinkManagerHost which is in progress by kylechar.

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

Status: Started (was: Assigned)

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

Blocking: -676384

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

Blocking: -601863

Comment 38 by danakj@chromium.org, Jun 13 2017

Summary: Eliminate Direct Access To SurfaceManager (from chrome UI and renderer) (was: Eliminate Direct Access To SurfaceManager)

Comment 39 by fsam...@chromium.org, Jun 13 2017

Components: -Internals>MUS

Comment 40 by kylec...@chromium.org, Jun 13 2017

Blockedon: 732900

Comment 41 by kylec...@chromium.org, Jun 13 2017

Blockedon: 732903
Project Member

Comment 42 by bugdroid1@chromium.org, Jun 22 2017 

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

commit 4952273aa8cf48001d7679babbc7d3fda1995d6e
Author: kylechar <kylechar@chromium.org>
Date: Thu Jun 22 15:43:12 2017

Add FrameSinkManagerHostTest fixture and test.

Add FrameSinkManagerHostTest fixture to be able to test
FrameSinkManagerHost. This also adds one simple test to use the fixture.

Bug: 657959
Change-Id: I5d878801c88f8396ee9bfcefe8fa45eb87db6a94
Reviewed-on: https://chromium-review.googlesource.com/538194
Commit-Queue: kylechar <kylechar@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#481541}
[modify] https://crrev.com/4952273aa8cf48001d7679babbc7d3fda1995d6e/components/BUILD.gn
[modify] https://crrev.com/4952273aa8cf48001d7679babbc7d3fda1995d6e/components/viz/host/BUILD.gn
[add] https://crrev.com/4952273aa8cf48001d7679babbc7d3fda1995d6e/components/viz/host/frame_sink_manager_host_unittests.cc
Project Member

Comment 43 by bugdroid1@chromium.org, Jun 27 2017 

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

commit 100bc68f6158ce4a6d9ad7c6136152f7ec8d5dcf
Author: kylechar <kylechar@chromium.org>
Date: Tue Jun 27 19:15:00 2017

Initialize HostFrameSinkManager in tests.

This CL ensures ui::InitializeContextFactoryForTests() initializes
HostFrameSinkManager. This uses Mojo so it requires that test targets
initialize mojo::edk.

Call mojo::edk::Init() from test main() functions where required. This
works for all test targets except interactive_ui_tests. Unfortunately,
there is a mix of content browser tests and unit tests in that target.
The browser tests each initialize mojo::edk, which works because each
test case runs in a new process. A similar individual initialization is
added to the appropriate non-browser test fixtures.

Bug: 657959
Change-Id: I984dc0fcc5d3dfb8ae6bd645e646660af99633e9
Reviewed-on: https://chromium-review.googlesource.com/549118
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#482704}
[modify] https://crrev.com/100bc68f6158ce4a6d9ad7c6136152f7ec8d5dcf/chrome/test/BUILD.gn
[modify] https://crrev.com/100bc68f6158ce4a6d9ad7c6136152f7ec8d5dcf/chrome/test/base/view_event_test_base.cc
[modify] https://crrev.com/100bc68f6158ce4a6d9ad7c6136152f7ec8d5dcf/content/browser/compositor/reflector_impl_unittest.cc
[modify] https://crrev.com/100bc68f6158ce4a6d9ad7c6136152f7ec8d5dcf/ui/app_list/BUILD.gn
[modify] https://crrev.com/100bc68f6158ce4a6d9ad7c6136152f7ec8d5dcf/ui/app_list/presenter/BUILD.gn
[modify] https://crrev.com/100bc68f6158ce4a6d9ad7c6136152f7ec8d5dcf/ui/app_list/presenter/test/DEPS
[modify] https://crrev.com/100bc68f6158ce4a6d9ad7c6136152f7ec8d5dcf/ui/app_list/presenter/test/run_all_unittests.cc
[modify] https://crrev.com/100bc68f6158ce4a6d9ad7c6136152f7ec8d5dcf/ui/app_list/test/DEPS
[modify] https://crrev.com/100bc68f6158ce4a6d9ad7c6136152f7ec8d5dcf/ui/app_list/test/run_all_unittests.cc
[modify] https://crrev.com/100bc68f6158ce4a6d9ad7c6136152f7ec8d5dcf/ui/chromeos/BUILD.gn
[modify] https://crrev.com/100bc68f6158ce4a6d9ad7c6136152f7ec8d5dcf/ui/chromeos/DEPS
[modify] https://crrev.com/100bc68f6158ce4a6d9ad7c6136152f7ec8d5dcf/ui/chromeos/run_all_unittests.cc
[modify] https://crrev.com/100bc68f6158ce4a6d9ad7c6136152f7ec8d5dcf/ui/compositor/BUILD.gn
[modify] https://crrev.com/100bc68f6158ce4a6d9ad7c6136152f7ec8d5dcf/ui/compositor/DEPS
[modify] https://crrev.com/100bc68f6158ce4a6d9ad7c6136152f7ec8d5dcf/ui/compositor/run_all_unittests.cc
[modify] https://crrev.com/100bc68f6158ce4a6d9ad7c6136152f7ec8d5dcf/ui/compositor/test/DEPS
[modify] https://crrev.com/100bc68f6158ce4a6d9ad7c6136152f7ec8d5dcf/ui/compositor/test/context_factories_for_test.cc
[modify] https://crrev.com/100bc68f6158ce4a6d9ad7c6136152f7ec8d5dcf/ui/keyboard/BUILD.gn
[add] https://crrev.com/100bc68f6158ce4a6d9ad7c6136152f7ec8d5dcf/ui/keyboard/test/DEPS
[modify] https://crrev.com/100bc68f6158ce4a6d9ad7c6136152f7ec8d5dcf/ui/keyboard/test/run_all_unittests.cc
[modify] https://crrev.com/100bc68f6158ce4a6d9ad7c6136152f7ec8d5dcf/ui/snapshot/BUILD.gn
[add] https://crrev.com/100bc68f6158ce4a6d9ad7c6136152f7ec8d5dcf/ui/snapshot/test/DEPS
[modify] https://crrev.com/100bc68f6158ce4a6d9ad7c6136152f7ec8d5dcf/ui/snapshot/test/run_all_unittests.cc
[modify] https://crrev.com/100bc68f6158ce4a6d9ad7c6136152f7ec8d5dcf/ui/views/BUILD.gn
[modify] https://crrev.com/100bc68f6158ce4a6d9ad7c6136152f7ec8d5dcf/ui/views/DEPS
[modify] https://crrev.com/100bc68f6158ce4a6d9ad7c6136152f7ec8d5dcf/ui/views/mus/BUILD.gn
[modify] https://crrev.com/100bc68f6158ce4a6d9ad7c6136152f7ec8d5dcf/ui/views/run_all_unittests_main.cc
[modify] https://crrev.com/100bc68f6158ce4a6d9ad7c6136152f7ec8d5dcf/ui/views/test/DEPS
[modify] https://crrev.com/100bc68f6158ce4a6d9ad7c6136152f7ec8d5dcf/ui/views/test/views_interactive_ui_test_base.cc
[add] https://crrev.com/100bc68f6158ce4a6d9ad7c6136152f7ec8d5dcf/ui/views/widget/DEPS
[modify] https://crrev.com/100bc68f6158ce4a6d9ad7c6136152f7ec8d5dcf/ui/views/widget/widget_interactive_uitest.cc
[modify] https://crrev.com/100bc68f6158ce4a6d9ad7c6136152f7ec8d5dcf/ui/wm/BUILD.gn
[modify] https://crrev.com/100bc68f6158ce4a6d9ad7c6136152f7ec8d5dcf/ui/wm/test/DEPS
[modify] https://crrev.com/100bc68f6158ce4a6d9ad7c6136152f7ec8d5dcf/ui/wm/test/run_all_unittests.cc
Project Member

Comment 44 by bugdroid1@chromium.org, Jun 27 2017 

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

commit 390e0b850ab633846734aee825f1ce927c863d46
Author: kylechar <kylechar@chromium.org>
Date: Tue Jun 27 20:16:05 2017

Move FrameSink hierarchy registration to FrameSinkManagerHost.

Change all calls to SurfaceManager::(Unr|R)registerFrameSinkHiearchy()
to be on FrameSinkManagerHost instead. This will have the calls happen
via Mojo asynchronously.

FrameSink hierarchy changes are infrequent so this isn't expected to
have performance implications. This will cause a change in ordering with
regards to FrameSink invalidation compared to hierarchy changes which is
already handled in cc::FrameSinkManager correctly.

Bug: 657959
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: I475e7952796cb93e1a89c1f168efee4a312d0e76
Reviewed-on: https://chromium-review.googlesource.com/524003
Commit-Queue: kylechar <kylechar@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Bo Liu <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#482724}
[modify] https://crrev.com/390e0b850ab633846734aee825f1ce927c863d46/content/browser/compositor/surface_utils.cc
[modify] https://crrev.com/390e0b850ab633846734aee825f1ce927c863d46/content/browser/frame_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/390e0b850ab633846734aee825f1ce927c863d46/content/browser/renderer_host/compositor_impl_android.cc
[modify] https://crrev.com/390e0b850ab633846734aee825f1ce927c863d46/ui/compositor/compositor.cc
Project Member

Comment 45 by bugdroid1@chromium.org, Jun 29 2017 

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

commit f9edf70a74e3f5027b2c38d980789a30726745dc
Author: kylechar <kylechar@chromium.org>
Date: Thu Jun 29 18:02:36 2017

Revert "Move FrameSink hierarchy registration to FrameSinkManagerHost."

This reverts commit 390e0b850ab633846734aee825f1ce927c863d46.

Reason for revert: This caused some large regressions in some time to first paint metrics. See https://chromeperf.appspot.com/group_report?bug_id=737974 and  http://crbug.com/737974 .

Original change's description:
> Move FrameSink hierarchy registration to FrameSinkManagerHost.
> 
> Change all calls to SurfaceManager::(Unr|R)registerFrameSinkHiearchy()
> to be on FrameSinkManagerHost instead. This will have the calls happen
> via Mojo asynchronously.
> 
> FrameSink hierarchy changes are infrequent so this isn't expected to
> have performance implications. This will cause a change in ordering with
> regards to FrameSink invalidation compared to hierarchy changes which is
> already handled in cc::FrameSinkManager correctly.
> 
> Bug: 657959
> Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
> Change-Id: I475e7952796cb93e1a89c1f168efee4a312d0e76
> Reviewed-on: https://chromium-review.googlesource.com/524003
> Commit-Queue: kylechar <kylechar@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Reviewed-by: Nasko Oskov <nasko@chromium.org>
> Reviewed-by: Ken Rockot <rockot@chromium.org>
> Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
> Reviewed-by: danakj <danakj@chromium.org>
> Reviewed-by: Bo Liu <boliu@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#482724}

TBR=sadrul@chromium.org,sky@chromium.org,danakj@chromium.org,nasko@chromium.org,boliu@chromium.org,rockot@chromium.org,kylechar@chromium.org

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

Bug:  737974 ,657959
Change-Id: Ieaa6e49e61c99c219acbcf63a5bdfe9da00fba17
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Reviewed-on: https://chromium-review.googlesource.com/555713
Commit-Queue: kylechar <kylechar@chromium.org>
Reviewed-by: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#483420}
[modify] https://crrev.com/f9edf70a74e3f5027b2c38d980789a30726745dc/content/browser/compositor/surface_utils.cc
[modify] https://crrev.com/f9edf70a74e3f5027b2c38d980789a30726745dc/content/browser/frame_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/f9edf70a74e3f5027b2c38d980789a30726745dc/content/browser/renderer_host/compositor_impl_android.cc
[modify] https://crrev.com/f9edf70a74e3f5027b2c38d980789a30726745dc/ui/compositor/compositor.cc
Project Member

Comment 46 by bugdroid1@chromium.org, Jul 12 2017 

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

commit a1037c1cd47988ecaf7c740aaf87354f4d8ff7e9
Author: kylechar <kylechar@chromium.org>
Date: Wed Jul 12 20:12:54 2017

Change FrameSinkManager task runner.

On Mac there is a special task runner that runs when the main task
runner is blocked on resize. The main task runner blocks until a
CompositorFrame arrives, and we need HostFrameSinkManager and
FrameSinkManagerImpl to continue processing messages during this time.

Bug: 657959
Change-Id: I3d8bb8bb22b5a4d36d1fe83e1144d11bbe1fd59d
Reviewed-on: https://chromium-review.googlesource.com/563860
Commit-Queue: kylechar <kylechar@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486070}
[modify] https://crrev.com/a1037c1cd47988ecaf7c740aaf87354f4d8ff7e9/components/viz/host/host_frame_sink_manager.cc
[modify] https://crrev.com/a1037c1cd47988ecaf7c740aaf87354f4d8ff7e9/components/viz/host/host_frame_sink_manager.h
[modify] https://crrev.com/a1037c1cd47988ecaf7c740aaf87354f4d8ff7e9/components/viz/host/host_frame_sink_manager_unittests.cc
[modify] https://crrev.com/a1037c1cd47988ecaf7c740aaf87354f4d8ff7e9/components/viz/service/frame_sinks/frame_sink_manager_impl.cc
[modify] https://crrev.com/a1037c1cd47988ecaf7c740aaf87354f4d8ff7e9/components/viz/service/frame_sinks/frame_sink_manager_impl.h
[modify] https://crrev.com/a1037c1cd47988ecaf7c740aaf87354f4d8ff7e9/content/browser/browser_main_loop.cc
[modify] https://crrev.com/a1037c1cd47988ecaf7c740aaf87354f4d8ff7e9/content/browser/browser_main_loop.h
[modify] https://crrev.com/a1037c1cd47988ecaf7c740aaf87354f4d8ff7e9/content/browser/compositor/gpu_process_transport_factory.cc
[modify] https://crrev.com/a1037c1cd47988ecaf7c740aaf87354f4d8ff7e9/content/browser/compositor/gpu_process_transport_factory.h
[modify] https://crrev.com/a1037c1cd47988ecaf7c740aaf87354f4d8ff7e9/content/browser/compositor/image_transport_factory.cc
[modify] https://crrev.com/a1037c1cd47988ecaf7c740aaf87354f4d8ff7e9/content/browser/compositor/image_transport_factory.h
[modify] https://crrev.com/a1037c1cd47988ecaf7c740aaf87354f4d8ff7e9/content/browser/compositor/surface_utils.cc
[modify] https://crrev.com/a1037c1cd47988ecaf7c740aaf87354f4d8ff7e9/content/browser/compositor/surface_utils.h
[modify] https://crrev.com/a1037c1cd47988ecaf7c740aaf87354f4d8ff7e9/content/browser/compositor/test/no_transport_image_transport_factory.cc
[modify] https://crrev.com/a1037c1cd47988ecaf7c740aaf87354f4d8ff7e9/content/browser/renderer_host/compositor_impl_android.cc
[modify] https://crrev.com/a1037c1cd47988ecaf7c740aaf87354f4d8ff7e9/content/browser/renderer_host/offscreen_canvas_provider_impl_unittest.cc
[modify] https://crrev.com/a1037c1cd47988ecaf7c740aaf87354f4d8ff7e9/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/a1037c1cd47988ecaf7c740aaf87354f4d8ff7e9/services/ui/gpu/gpu_main.cc
[modify] https://crrev.com/a1037c1cd47988ecaf7c740aaf87354f4d8ff7e9/ui/compositor/test/context_factories_for_test.cc

Comment 47 by mark@chromium.org, Jul 12 2017 

a1037c1cd479 caused compile failures on these bots (so far and at least):

https://build.chromium.org/p/chromium.mac/builders/Mac%20Builder/builds/70936
https://build.chromium.org/p/chromium.linux/builders/Linux%20Builder/builds/87456

I’m reverting it. It’s suspicious that the tryserver didn’t catch this.
Project Member

Comment 48 by bugdroid1@chromium.org, Jul 12 2017 

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

commit 77cf3bbc98957d0f19da8b01afc47028c401cf79
Author: Mark Mentovai <mark@chromium.org>
Date: Wed Jul 12 21:08:46 2017

Revert "Change FrameSinkManager task runner."

This reverts commit a1037c1cd47988ecaf7c740aaf87354f4d8ff7e9.

Reason for revert: https://crbug.com/657959#c47

Original change's description:
> Change FrameSinkManager task runner.
> 
> On Mac there is a special task runner that runs when the main task
> runner is blocked on resize. The main task runner blocks until a
> CompositorFrame arrives, and we need HostFrameSinkManager and
> FrameSinkManagerImpl to continue processing messages during this time.
> 
> Bug: 657959
> Change-Id: I3d8bb8bb22b5a4d36d1fe83e1144d11bbe1fd59d
> Reviewed-on: https://chromium-review.googlesource.com/563860
> Commit-Queue: kylechar <kylechar@chromium.org>
> Reviewed-by: Saman Sami <samans@chromium.org>
> Reviewed-by: Fady Samuel <fsamuel@chromium.org>
> Reviewed-by: Antoine Labour <piman@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#486070}

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

Change-Id: I540b79a8fc55ec260268bf437903e05020dc398f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 657959
Reviewed-on: https://chromium-review.googlesource.com/568879
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486088}
[modify] https://crrev.com/77cf3bbc98957d0f19da8b01afc47028c401cf79/components/viz/host/host_frame_sink_manager.cc
[modify] https://crrev.com/77cf3bbc98957d0f19da8b01afc47028c401cf79/components/viz/host/host_frame_sink_manager.h
[modify] https://crrev.com/77cf3bbc98957d0f19da8b01afc47028c401cf79/components/viz/host/host_frame_sink_manager_unittests.cc
[modify] https://crrev.com/77cf3bbc98957d0f19da8b01afc47028c401cf79/components/viz/service/frame_sinks/frame_sink_manager_impl.cc
[modify] https://crrev.com/77cf3bbc98957d0f19da8b01afc47028c401cf79/components/viz/service/frame_sinks/frame_sink_manager_impl.h
[modify] https://crrev.com/77cf3bbc98957d0f19da8b01afc47028c401cf79/content/browser/browser_main_loop.cc
[modify] https://crrev.com/77cf3bbc98957d0f19da8b01afc47028c401cf79/content/browser/browser_main_loop.h
[modify] https://crrev.com/77cf3bbc98957d0f19da8b01afc47028c401cf79/content/browser/compositor/gpu_process_transport_factory.cc
[modify] https://crrev.com/77cf3bbc98957d0f19da8b01afc47028c401cf79/content/browser/compositor/gpu_process_transport_factory.h
[modify] https://crrev.com/77cf3bbc98957d0f19da8b01afc47028c401cf79/content/browser/compositor/image_transport_factory.cc
[modify] https://crrev.com/77cf3bbc98957d0f19da8b01afc47028c401cf79/content/browser/compositor/image_transport_factory.h
[modify] https://crrev.com/77cf3bbc98957d0f19da8b01afc47028c401cf79/content/browser/compositor/surface_utils.cc
[modify] https://crrev.com/77cf3bbc98957d0f19da8b01afc47028c401cf79/content/browser/compositor/surface_utils.h
[modify] https://crrev.com/77cf3bbc98957d0f19da8b01afc47028c401cf79/content/browser/compositor/test/no_transport_image_transport_factory.cc
[modify] https://crrev.com/77cf3bbc98957d0f19da8b01afc47028c401cf79/content/browser/renderer_host/compositor_impl_android.cc
[modify] https://crrev.com/77cf3bbc98957d0f19da8b01afc47028c401cf79/content/browser/renderer_host/offscreen_canvas_provider_impl_unittest.cc
[modify] https://crrev.com/77cf3bbc98957d0f19da8b01afc47028c401cf79/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/77cf3bbc98957d0f19da8b01afc47028c401cf79/services/ui/gpu/gpu_main.cc
[modify] https://crrev.com/77cf3bbc98957d0f19da8b01afc47028c401cf79/ui/compositor/test/context_factories_for_test.cc

Comment 49 by kylec...@chromium.org, Jul 12 2017 

Okay. It looks like two conflicting CLs landed at roughly the same time, which explains why the CQ didn't catch it.

https://chromium-review.googlesource.com/c/563860/
https://chromium-review.googlesource.com/c/562216/

Both CLs would have been fine on their own, but the second one moved |message_loop_|. Somehow there was no conflicting lines between the CLs.
Project Member

Comment 50 by bugdroid1@chromium.org, Jul 13 2017 

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

commit ee7cefd0ebfc371fdeb141cb044c2191a9968749
Author: kylechar <kylechar@chromium.org>
Date: Thu Jul 13 16:05:03 2017

Reland "Change FrameSinkManager task runner."

On Mac there is a special task runner that runs when the main task
runner is blocked on resize. The main task runner blocks until a
CompositorFrame arrives, and we need HostFrameSinkManager and
FrameSinkManagerImpl to continue processing messages during this time.

The original CL http://crrev.com/c/563860 conflicted with
http://crrev.com/c/562216 which moved the MessageLoop out of
HostFrameSinkManagerTest. Get the TaskRunner from helper function
instead of from MessageLoop directly.

Bug: 657959
Change-Id: I9488d5ca38827e9ba72431c144f9a8ea69f7b66c
TBR: piman@chromium.org,fsamuel@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/568999
Commit-Queue: kylechar <kylechar@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486401}
[modify] https://crrev.com/ee7cefd0ebfc371fdeb141cb044c2191a9968749/components/viz/host/host_frame_sink_manager.cc
[modify] https://crrev.com/ee7cefd0ebfc371fdeb141cb044c2191a9968749/components/viz/host/host_frame_sink_manager.h
[modify] https://crrev.com/ee7cefd0ebfc371fdeb141cb044c2191a9968749/components/viz/host/host_frame_sink_manager_unittests.cc
[modify] https://crrev.com/ee7cefd0ebfc371fdeb141cb044c2191a9968749/components/viz/service/frame_sinks/frame_sink_manager_impl.cc
[modify] https://crrev.com/ee7cefd0ebfc371fdeb141cb044c2191a9968749/components/viz/service/frame_sinks/frame_sink_manager_impl.h
[modify] https://crrev.com/ee7cefd0ebfc371fdeb141cb044c2191a9968749/content/browser/browser_main_loop.cc
[modify] https://crrev.com/ee7cefd0ebfc371fdeb141cb044c2191a9968749/content/browser/browser_main_loop.h
[modify] https://crrev.com/ee7cefd0ebfc371fdeb141cb044c2191a9968749/content/browser/compositor/gpu_process_transport_factory.cc
[modify] https://crrev.com/ee7cefd0ebfc371fdeb141cb044c2191a9968749/content/browser/compositor/gpu_process_transport_factory.h
[modify] https://crrev.com/ee7cefd0ebfc371fdeb141cb044c2191a9968749/content/browser/compositor/image_transport_factory.cc
[modify] https://crrev.com/ee7cefd0ebfc371fdeb141cb044c2191a9968749/content/browser/compositor/image_transport_factory.h
[modify] https://crrev.com/ee7cefd0ebfc371fdeb141cb044c2191a9968749/content/browser/compositor/surface_utils.cc
[modify] https://crrev.com/ee7cefd0ebfc371fdeb141cb044c2191a9968749/content/browser/compositor/surface_utils.h
[modify] https://crrev.com/ee7cefd0ebfc371fdeb141cb044c2191a9968749/content/browser/compositor/test/no_transport_image_transport_factory.cc
[modify] https://crrev.com/ee7cefd0ebfc371fdeb141cb044c2191a9968749/content/browser/renderer_host/compositor_impl_android.cc
[modify] https://crrev.com/ee7cefd0ebfc371fdeb141cb044c2191a9968749/content/browser/renderer_host/offscreen_canvas_provider_impl_unittest.cc
[modify] https://crrev.com/ee7cefd0ebfc371fdeb141cb044c2191a9968749/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/ee7cefd0ebfc371fdeb141cb044c2191a9968749/services/ui/gpu/gpu_main.cc
[modify] https://crrev.com/ee7cefd0ebfc371fdeb141cb044c2191a9968749/ui/compositor/test/context_factories_for_test.cc
Project Member

Comment 51 by bugdroid1@chromium.org, Jul 26 2017 

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

commit 12bad2c5f5047a44e505d85f83f16da5b1fe043d
Author: kylechar <kylechar@chromium.org>
Date: Wed Jul 26 19:30:01 2017

Remove mojom::CompositorFrameSinkManagerPrivate.

This interface was added to take advantage of the queuing data on the
partially bound InterfacePtr<> while waiting for the client to connect.
This is problematic, since we have no guarantees the client will connect
and can't recover the queued messages if the client, for example,
crashes before connecting.

Remove the interface entirely and move the same functionality over to
mojom::FrameSinkManager. There were three events that private interface
provided: connecting which is replaced by RegisterFrameSinkId(),
ClaimTemporaryReference() which is replaced by
AssignTemporaryReference() and disconnecting which is replaced by
InvalidateFrameSinkId().

Since we can register a FrameSinkId as soon as we know it will exist we
no longer need to queue temporary reference assignment either, which
simplifies the code further.

Bug: 657959
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I3442c9f0459ed4ddc9219425036fa9feef0aa8bc
Reviewed-on: https://chromium-review.googlesource.com/585632
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489723}
[modify] https://crrev.com/12bad2c5f5047a44e505d85f83f16da5b1fe043d/cc/ipc/compositor_frame_sink.mojom
[modify] https://crrev.com/12bad2c5f5047a44e505d85f83f16da5b1fe043d/cc/ipc/frame_sink_manager.mojom
[modify] https://crrev.com/12bad2c5f5047a44e505d85f83f16da5b1fe043d/components/viz/host/host_frame_sink_manager.cc
[modify] https://crrev.com/12bad2c5f5047a44e505d85f83f16da5b1fe043d/components/viz/host/host_frame_sink_manager.h
[modify] https://crrev.com/12bad2c5f5047a44e505d85f83f16da5b1fe043d/components/viz/host/host_frame_sink_manager_unittests.cc
[modify] https://crrev.com/12bad2c5f5047a44e505d85f83f16da5b1fe043d/components/viz/service/frame_sinks/compositor_frame_sink_support.cc
[modify] https://crrev.com/12bad2c5f5047a44e505d85f83f16da5b1fe043d/components/viz/service/frame_sinks/compositor_frame_sink_support.h
[modify] https://crrev.com/12bad2c5f5047a44e505d85f83f16da5b1fe043d/components/viz/service/frame_sinks/frame_sink_manager_impl.cc
[modify] https://crrev.com/12bad2c5f5047a44e505d85f83f16da5b1fe043d/components/viz/service/frame_sinks/frame_sink_manager_impl.h
[modify] https://crrev.com/12bad2c5f5047a44e505d85f83f16da5b1fe043d/components/viz/service/frame_sinks/gpu_compositor_frame_sink.cc
[modify] https://crrev.com/12bad2c5f5047a44e505d85f83f16da5b1fe043d/components/viz/service/frame_sinks/gpu_compositor_frame_sink.h
[modify] https://crrev.com/12bad2c5f5047a44e505d85f83f16da5b1fe043d/components/viz/service/frame_sinks/gpu_root_compositor_frame_sink.cc
[modify] https://crrev.com/12bad2c5f5047a44e505d85f83f16da5b1fe043d/components/viz/service/frame_sinks/gpu_root_compositor_frame_sink.h
[modify] https://crrev.com/12bad2c5f5047a44e505d85f83f16da5b1fe043d/services/ui/ws/frame_sink_manager_client_binding.cc
[modify] https://crrev.com/12bad2c5f5047a44e505d85f83f16da5b1fe043d/services/ui/ws/frame_sink_manager_client_binding.h
[modify] https://crrev.com/12bad2c5f5047a44e505d85f83f16da5b1fe043d/services/ui/ws/server_window.cc
[modify] https://crrev.com/12bad2c5f5047a44e505d85f83f16da5b1fe043d/services/ui/ws/server_window_compositor_frame_sink_manager.cc
[modify] https://crrev.com/12bad2c5f5047a44e505d85f83f16da5b1fe043d/services/ui/ws/server_window_compositor_frame_sink_manager.h
Project Member

Comment 52 by bugdroid1@chromium.org, Jul 26 2017 

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

commit 5aded5a46e516b5bc2f8a0daea3c3fbbd0bee066
Author: Mike Bjorge <mbjorge@chromium.org>
Date: Wed Jul 26 23:13:09 2017

[Chromecast] Init Mojo in cast_graphics_unittests

https://chromium-review.googlesource.com/c/549118/ added a requirement
that test targets using ui::InitializeContextFactoryForTests()
initialize the mojo::edk.

Bug: 657959,  749144 
Test: cast_graphics_unittests
Change-Id: Ibdd7ff732eed4c74a9e5f2160b1b42b34cae75d6
Reviewed-on: https://chromium-review.googlesource.com/587127
Reviewed-by: Alok Priyadarshi <alokp@chromium.org>
Reviewed-by: Yuzhu Shen <yzshen@chromium.org>
Reviewed-by: kylechar <kylechar@chromium.org>
Reviewed-by: Luke Halliwell <halliwell@chromium.org>
Commit-Queue: Mike Bjorge <mbjorge@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489792}
[modify] https://crrev.com/5aded5a46e516b5bc2f8a0daea3c3fbbd0bee066/chromecast/graphics/BUILD.gn
[modify] https://crrev.com/5aded5a46e516b5bc2f8a0daea3c3fbbd0bee066/chromecast/graphics/DEPS
[modify] https://crrev.com/5aded5a46e516b5bc2f8a0daea3c3fbbd0bee066/chromecast/graphics/run_all_unittests.cc

Comment 53 by kylec...@chromium.org, Jul 27 2017

Summary: Eliminate Direct Access To FrameSinkManagerImpl (from chrome UI and renderer) (was: Eliminate Direct Access To SurfaceManager (from chrome UI and renderer))
Project Member

Comment 54 by bugdroid1@chromium.org, Aug 2 2017 

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

commit bac0f1afc84595936d4fdf3e1061c357a174b673
Author: Fady Samuel <fsamuel@chromium.org>
Date: Wed Aug 02 15:54:02 2017

viz: Introduce RegisterFrameSinkId and InvalidateFrameSinkId Host API

Registering a FrameSinkId delimits the beginning of the lifetime of a
FrameSinkId. From Viz's perspective that ID is valid until
InvalidateFrameSinkId is called. RegisterFrameSinkId also bundles
a HostFrameSinkClient (was FrameSinkObserver) that listens for new surfaces
submitted to the registered FrameSinkId.

This CL allows us to replace multiple call sites to RegisterFrameSink /
InvalidateFrameSinkId in content that go directly to FrameSinkManagerImpl
with calls to HostFrameSinkManager, taking us closer to a point where
content does not directly use FrameSinkManagerImpl.

This CL is also a prerequisite for using HostFrameSinkManager in the
window server because it introduces a mechanism for listening for events
pertaining to a particular FrameSinkId which is a requirement for the
window server. This interface will be used for events as well in the
future.

Bug: 657959
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: I3e1343ea6cfbe6b3d39ba2fcd23d193dd2c3e4a0
Reviewed-on: https://chromium-review.googlesource.com/595147
Reviewed-by: Bo Liu <boliu@chromium.org>
Reviewed-by: Justin Novosad <junov@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
Reviewed-by: Ria Jiang <riajiang@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491386}
[modify] https://crrev.com/bac0f1afc84595936d4fdf3e1061c357a174b673/components/viz/host/BUILD.gn
[rename] https://crrev.com/bac0f1afc84595936d4fdf3e1061c357a174b673/components/viz/host/host_frame_sink_client.h
[modify] https://crrev.com/bac0f1afc84595936d4fdf3e1061c357a174b673/components/viz/host/host_frame_sink_manager.cc
[modify] https://crrev.com/bac0f1afc84595936d4fdf3e1061c357a174b673/components/viz/host/host_frame_sink_manager.h
[modify] https://crrev.com/bac0f1afc84595936d4fdf3e1061c357a174b673/components/viz/host/host_frame_sink_manager_unittests.cc
[modify] https://crrev.com/bac0f1afc84595936d4fdf3e1061c357a174b673/content/browser/frame_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/bac0f1afc84595936d4fdf3e1061c357a174b673/content/browser/frame_host/render_widget_host_view_child_frame.h
[modify] https://crrev.com/bac0f1afc84595936d4fdf3e1061c357a174b673/content/browser/renderer_host/compositor_impl_android.cc
[modify] https://crrev.com/bac0f1afc84595936d4fdf3e1061c357a174b673/content/browser/renderer_host/compositor_impl_android.h
[modify] https://crrev.com/bac0f1afc84595936d4fdf3e1061c357a174b673/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/bac0f1afc84595936d4fdf3e1061c357a174b673/content/browser/renderer_host/delegated_frame_host.h
[modify] https://crrev.com/bac0f1afc84595936d4fdf3e1061c357a174b673/content/browser/renderer_host/offscreen_canvas_surface_impl.cc
[modify] https://crrev.com/bac0f1afc84595936d4fdf3e1061c357a174b673/content/browser/renderer_host/offscreen_canvas_surface_impl.h
[modify] https://crrev.com/bac0f1afc84595936d4fdf3e1061c357a174b673/content/test/BUILD.gn
[modify] https://crrev.com/bac0f1afc84595936d4fdf3e1061c357a174b673/content/test/DEPS
[modify] https://crrev.com/bac0f1afc84595936d4fdf3e1061c357a174b673/content/test/test_render_view_host.cc
[modify] https://crrev.com/bac0f1afc84595936d4fdf3e1061c357a174b673/content/test/test_render_view_host.h
[modify] https://crrev.com/bac0f1afc84595936d4fdf3e1061c357a174b673/ui/android/delegated_frame_host_android.cc
[modify] https://crrev.com/bac0f1afc84595936d4fdf3e1061c357a174b673/ui/android/delegated_frame_host_android.h
[modify] https://crrev.com/bac0f1afc84595936d4fdf3e1061c357a174b673/ui/compositor/compositor.cc
[modify] https://crrev.com/bac0f1afc84595936d4fdf3e1061c357a174b673/ui/compositor/compositor.h
Project Member

Comment 55 by bugdroid1@chromium.org, Aug 2 2017 

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

commit 95acc1b23e9bf10e784c8d08c9d97039c7d210a7
Author: Fady Samuel <fsamuel@chromium.org>
Date: Wed Aug 02 19:58:39 2017

viz: Remove handles_frame_sink_id_invalidation bit

The handles_frame_sink_id_invalidation bit was introduced because in
the OOP display compositor case, each CompositorFrameSinkSupport handled
its own FrameSinkId invalidation. Now that HostFrameSinkManager has introduced
explicit RegisterFrameSinkId and InvalidFrameSinkId APIs, this bit is no longer
necessary. This CL removes it and updates unit tests accordingly.

Bug: 657959
TBR: jam@chromium.org for content stamp: already reviewed by boliu@ and kenrb@
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: I901614988ada0134cb029a7bbb98f58d94ada337
Reviewed-on: https://chromium-review.googlesource.com/598529
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Bo Liu <boliu@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491475}
[modify] https://crrev.com/95acc1b23e9bf10e784c8d08c9d97039c7d210a7/android_webview/browser/hardware_renderer.cc
[modify] https://crrev.com/95acc1b23e9bf10e784c8d08c9d97039c7d210a7/android_webview/browser/surfaces_instance.cc
[modify] https://crrev.com/95acc1b23e9bf10e784c8d08c9d97039c7d210a7/components/viz/host/host_frame_sink_manager.cc
[modify] https://crrev.com/95acc1b23e9bf10e784c8d08c9d97039c7d210a7/components/viz/host/host_frame_sink_manager.h
[modify] https://crrev.com/95acc1b23e9bf10e784c8d08c9d97039c7d210a7/components/viz/host/host_frame_sink_manager_unittests.cc
[modify] https://crrev.com/95acc1b23e9bf10e784c8d08c9d97039c7d210a7/components/viz/service/display/display_unittest.cc
[modify] https://crrev.com/95acc1b23e9bf10e784c8d08c9d97039c7d210a7/components/viz/service/display/surface_aggregator_perftest.cc
[modify] https://crrev.com/95acc1b23e9bf10e784c8d08c9d97039c7d210a7/components/viz/service/display/surface_aggregator_pixeltest.cc
[modify] https://crrev.com/95acc1b23e9bf10e784c8d08c9d97039c7d210a7/components/viz/service/display/surface_aggregator_unittest.cc
[modify] https://crrev.com/95acc1b23e9bf10e784c8d08c9d97039c7d210a7/components/viz/service/frame_sinks/compositor_frame_sink_support.cc
[modify] https://crrev.com/95acc1b23e9bf10e784c8d08c9d97039c7d210a7/components/viz/service/frame_sinks/compositor_frame_sink_support.h
[modify] https://crrev.com/95acc1b23e9bf10e784c8d08c9d97039c7d210a7/components/viz/service/frame_sinks/compositor_frame_sink_support_manager.h
[modify] https://crrev.com/95acc1b23e9bf10e784c8d08c9d97039c7d210a7/components/viz/service/frame_sinks/compositor_frame_sink_support_unittest.cc
[modify] https://crrev.com/95acc1b23e9bf10e784c8d08c9d97039c7d210a7/components/viz/service/frame_sinks/direct_layer_tree_frame_sink.cc
[modify] https://crrev.com/95acc1b23e9bf10e784c8d08c9d97039c7d210a7/components/viz/service/frame_sinks/direct_layer_tree_frame_sink_unittest.cc
[modify] https://crrev.com/95acc1b23e9bf10e784c8d08c9d97039c7d210a7/components/viz/service/frame_sinks/gpu_compositor_frame_sink.cc
[modify] https://crrev.com/95acc1b23e9bf10e784c8d08c9d97039c7d210a7/components/viz/service/frame_sinks/gpu_root_compositor_frame_sink.cc
[modify] https://crrev.com/95acc1b23e9bf10e784c8d08c9d97039c7d210a7/components/viz/service/frame_sinks/surface_references_unittest.cc
[modify] https://crrev.com/95acc1b23e9bf10e784c8d08c9d97039c7d210a7/components/viz/service/frame_sinks/surface_synchronization_unittest.cc
[modify] https://crrev.com/95acc1b23e9bf10e784c8d08c9d97039c7d210a7/components/viz/service/surfaces/surface_hittest_unittest.cc
[modify] https://crrev.com/95acc1b23e9bf10e784c8d08c9d97039c7d210a7/components/viz/service/surfaces/surface_unittest.cc
[modify] https://crrev.com/95acc1b23e9bf10e784c8d08c9d97039c7d210a7/components/viz/test/test_layer_tree_frame_sink.cc
[modify] https://crrev.com/95acc1b23e9bf10e784c8d08c9d97039c7d210a7/content/browser/frame_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/95acc1b23e9bf10e784c8d08c9d97039c7d210a7/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/95acc1b23e9bf10e784c8d08c9d97039c7d210a7/content/renderer/android/synchronous_layer_tree_frame_sink.cc
[modify] https://crrev.com/95acc1b23e9bf10e784c8d08c9d97039c7d210a7/ui/android/delegated_frame_host_android.cc
[modify] https://crrev.com/95acc1b23e9bf10e784c8d08c9d97039c7d210a7/ui/aura/local/layer_tree_frame_sink_local.cc
Project Member

Comment 56 by bugdroid1@chromium.org, Aug 3 2017 

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

commit 2c36a6bc6d451b4d25c19e9ed5b2efc69a1d4fec
Author: Fady Samuel <fsamuel@chromium.org>
Date: Thu Aug 03 02:12:14 2017

viz: Drop temporary reference to discarded FrameSink

This CL fixes a bug where HostFrameSinkManager will not drop the temporary
reference to a FrameSink that has been invalidated from HostFrameSinkManager.

This verifies that HostFrameSinkManager knows about the FrameSinkId of
the Surface created in OnSurfaceCreated. If it doesn't then HostFrameSinkManager
asks FrameSinkManager to drop the temporary reference.

Bug: 657959
Change-Id: Ia9b8eff0885fc0b56e9d866acd41860b11a0b3ba
Reviewed-on: https://chromium-review.googlesource.com/599001
Reviewed-by: Ria Jiang <riajiang@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491612}
[modify] https://crrev.com/2c36a6bc6d451b4d25c19e9ed5b2efc69a1d4fec/components/viz/host/host_frame_sink_manager.cc
[modify] https://crrev.com/2c36a6bc6d451b4d25c19e9ed5b2efc69a1d4fec/components/viz/host/host_frame_sink_manager_unittests.cc
[modify] https://crrev.com/2c36a6bc6d451b4d25c19e9ed5b2efc69a1d4fec/components/viz/service/surfaces/surface_manager.cc
Project Member

Comment 57 by bugdroid1@chromium.org, Aug 9 2017 

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

commit 78993070f9b60d14eb0baeb5c46fae90060bc647
Author: kylechar <kylechar@chromium.org>
Date: Wed Aug 09 05:11:03 2017

viz: Fix HostFrameSinkManager API.

Fix a few parts of the HostFrameSinkManager API.
1. Add a CreateRootCompositorFrameSink() so we can use
   HostFrameSinkManager in mus-ws.
2. Track the multiple parent case for frame sink hierarchy.
3. Don't throw out frame sink hierarchy information on invalidate, since
   we still want to verify when unregistering hierarchy.

Bug: 657959
Change-Id: I2764ca19658fba0905316f94a440191f1c70ce5f
Reviewed-on: https://chromium-review.googlesource.com/602375
Commit-Queue: kylechar <kylechar@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492831}
[modify] https://crrev.com/78993070f9b60d14eb0baeb5c46fae90060bc647/components/viz/host/host_frame_sink_manager.cc
[modify] https://crrev.com/78993070f9b60d14eb0baeb5c46fae90060bc647/components/viz/host/host_frame_sink_manager.h
[modify] https://crrev.com/78993070f9b60d14eb0baeb5c46fae90060bc647/components/viz/host/host_frame_sink_manager_unittests.cc
Project Member

Comment 58 by bugdroid1@chromium.org, Aug 18 2017 

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

commit 8c0beaa5b1f50625bd29f01716254912524a15a1
Author: kylechar <kylechar@chromium.org>
Date: Fri Aug 18 03:42:17 2017

Check FrameSinkId registration.

Add DCHECK in HostFrameSinkManager that FrameSinkId is registered before
CompositorFrameSink or CompositorFrameSinkSupport is created for it.
Also cleanup HostFrameSinkManagerTests and make sure they all pass with
the new DCHECK.

Bug: 657959
Change-Id: I0f08c2abfee9b1c9bb25bf473ebeeb6c066f81fe
Reviewed-on: https://chromium-review.googlesource.com/619547
Reviewed-by: Ria Jiang <riajiang@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495452}
[modify] https://crrev.com/8c0beaa5b1f50625bd29f01716254912524a15a1/components/viz/host/host_frame_sink_manager.cc
[modify] https://crrev.com/8c0beaa5b1f50625bd29f01716254912524a15a1/components/viz/host/host_frame_sink_manager_unittests.cc

Comment 59 by sadrul@chromium.org, Sep 28 2017

Blocking: -730193

Comment 60 by sadrul@chromium.org, Sep 29 2017

Blocking: 732555
Project Member

Comment 61 by bugdroid1@chromium.org, Oct 30 2017 

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

commit f483b30be7f2be750117bc6e6eb30d9485d1cbe9
Author: kylechar <kylechar@chromium.org>
Date: Mon Oct 30 16:14:28 2017

Verify FrameSinkIds are registered.

Add a DCHECK to enforce that FrameSinkIds are registered before adding
them to hierarchy. This is already the case for all usage, except one
test which I fixed, but this will enforce it doesn't get broken.

Also remove ImageTransportFactory initialization from
OffscreenCanvasProviderImplTests. It's not actually used.

Bug: 657959
Change-Id: Ie197b655ecb1b1eabb5da56ca92fd590e2dfa450
Reviewed-on: https://chromium-review.googlesource.com/738284
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512508}
[modify] https://crrev.com/f483b30be7f2be750117bc6e6eb30d9485d1cbe9/components/viz/host/host_frame_sink_manager.cc
[modify] https://crrev.com/f483b30be7f2be750117bc6e6eb30d9485d1cbe9/components/viz/host/host_frame_sink_manager.h
[modify] https://crrev.com/f483b30be7f2be750117bc6e6eb30d9485d1cbe9/components/viz/host/host_frame_sink_manager_unittest.cc
[modify] https://crrev.com/f483b30be7f2be750117bc6e6eb30d9485d1cbe9/components/viz/test/BUILD.gn
[add] https://crrev.com/f483b30be7f2be750117bc6e6eb30d9485d1cbe9/components/viz/test/fake_host_frame_sink_client.cc
[add] https://crrev.com/f483b30be7f2be750117bc6e6eb30d9485d1cbe9/components/viz/test/fake_host_frame_sink_client.h
[modify] https://crrev.com/f483b30be7f2be750117bc6e6eb30d9485d1cbe9/content/browser/renderer_host/offscreen_canvas_provider_impl_unittest.cc

Comment 62 by fsam...@chromium.org, Nov 7 2017

Blocking: -732555
This can be closed once SurfaceSequences are gone and we use the new hit test code path.

Comment 63 by kylec...@chromium.org, Feb 1 2018 

SurfaceSequence is gone. The only thing left that depends on FrameSinkManagerImpl is the surface hit test code I think.

Comment 64 by sadrul@chromium.org, Feb 1 2018

Cc: riajiang@chromium.org
Do you know where that code is?

/cc+ riajiang@

Comment 65 by varkha@chromium.org, Feb 1 2018

Cc: -varkha@chromium.org

Comment 66 by riajiang@chromium.org, Feb 1 2018 

The only direct dependency on FrameSinkManagerImpl I found in the surface hit-test code is in surface_hittest_unittest.cc [1]. Other places seem to be only using FrameSinkManagerImpl to get its SurfaceManager when creating SurfaceHittest (e.g. [2]).

[1] https://cs.chromium.org/chromium/src/components/viz/service/surfaces/surface_hittest_unittest.cc?dr=C&l=99

[2] https://cs.chromium.org/chromium/src/content/browser/frame_host/cross_process_frame_connector.cc?dr=C&l=155

Comment 67 by kylec...@chromium.org, Feb 1 2018 

It's all callers of content::GetFrameSinkManager(). They use it to get SurfaceManager which is an implementation detail of FrameSinkManagerImpl and isn't allowed.

https://cs.chromium.org/chromium/src/content/browser/compositor/surface_utils.h?l=26&rcl=c8f1338f54e530e7865ec0d2f015725c48605de1

Comment 68 by lafo...@chromium.org, Feb 26 2018

Components: -MUS Internals>Services>WindowService

Comment 69 by jamescook@chromium.org, Apr 24 2018

Labels: -Proj-Mustash-Mus
Migrating Proj-Mustash-Mus to components Internals>Services>WindowService and Internals>Services>Ash

Comment 70 by kylec...@chromium.org, Apr 25 2018

Components: -Internals>Compositing -Internals>Services>WindowService Internals>Services>Viz
Labels: -displaycompositor -Proj-Mustash -Proj-Mustash-Mus-GPU
►

Sign in to add a comment