Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(132)

Issue 2830143002: Add FrameSinkManagerHost to NoTransportImageTransportFactory. (Closed)

Created:
3 years, 8 months ago by kylechar
Modified:
3 years, 8 months ago
CC:
Aaron Boodman, abarth-chromium, chromium-reviews, danakj+watch_chromium.org, darin (slow to review), darin-cc_chromium.org, jbauman+watch_chromium.org, kalyank, piman+watch_chromium.org, qsr+mojo_chromium.org, rjkroege, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

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} Committed: https://chromium.googlesource.com/chromium/src/+/bfc507f8797d610a272dd3dbcf264825b1cc5384

Patch Set 1 #

Patch Set 2 : Also android. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+48 lines, -23 lines) Patch
M components/viz/frame_sinks/mojo_frame_sink_manager.h View 1 chunk +5 lines, -3 lines 0 comments Download
M components/viz/frame_sinks/mojo_frame_sink_manager.cc View 2 chunks +11 lines, -7 lines 0 comments Download
M content/browser/compositor/frame_sink_manager_host.h View 2 chunks +8 lines, -2 lines 0 comments Download
M content/browser/compositor/frame_sink_manager_host.cc View 4 chunks +10 lines, -3 lines 0 comments Download
M content/browser/compositor/gpu_process_transport_factory.cc View 1 chunk +1 line, -0 lines 0 comments Download
M content/browser/compositor/test/no_transport_image_transport_factory.h View 2 chunks +2 lines, -1 line 0 comments Download
M content/browser/compositor/test/no_transport_image_transport_factory.cc View 3 chunks +6 lines, -5 lines 0 comments Download
M content/browser/renderer_host/compositor_impl_android.cc View 1 1 chunk +3 lines, -1 line 0 comments Download
M services/ui/gpu/gpu_main.cc View 1 chunk +2 lines, -1 line 0 comments Download

Dependent Patchsets:

Messages

Total messages: 39 (23 generated)
kylechar
fsamuel for components/viz/*
3 years, 8 months ago (2017-04-21 13:13:18 UTC) #6
kylechar
I guess also services/ui/gpu/* and just general review.
3 years, 8 months ago (2017-04-21 13:14:12 UTC) #7
Fady Samuel
This looks reasonable but it looks like you're failing on Android because you're not connecting? ...
3 years, 8 months ago (2017-04-21 13:16:31 UTC) #8
kylechar
On 2017/04/21 13:16:31, Fady Samuel wrote: > This looks reasonable but it looks like you're ...
3 years, 8 months ago (2017-04-21 13:27:56 UTC) #9
kylechar
On 2017/04/21 13:16:31, Fady Samuel wrote: > This looks reasonable but it looks like you're ...
3 years, 8 months ago (2017-04-21 13:27:56 UTC) #10
Fady Samuel
lgtm
3 years, 8 months ago (2017-04-21 13:37:54 UTC) #11
kylechar
+jbauman for content/browser/compositor/* +aelias for content/browser/renderer_host/compositor_impl_android.cc
3 years, 8 months ago (2017-04-21 14:18:46 UTC) #13
aelias_OOO_until_Jul13
lgtm
3 years, 8 months ago (2017-04-21 18:08:55 UTC) #14
Fady Samuel
You might want to add a content/ reviewer like jam@ because both danakj@ and jbauman@ ...
3 years, 8 months ago (2017-04-22 23:09:16 UTC) #17
kylechar
-jbauman who's OOO +jam for content/browser/compositor/*
3 years, 8 months ago (2017-04-24 20:51:41 UTC) #21
jam
On 2017/04/24 20:51:41, kylechar wrote: > -jbauman who's OOO > +jam for content/browser/compositor/* please pick ...
3 years, 8 months ago (2017-04-24 22:12:12 UTC) #22
kylechar
-jam, +jbauman again.
3 years, 8 months ago (2017-04-25 13:18:06 UTC) #24
jbauman
lgtm
3 years, 8 months ago (2017-04-25 20:39:11 UTC) #25
kylechar
Thanks!
3 years, 8 months ago (2017-04-25 20:45:08 UTC) #27
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2830143002/20001
3 years, 8 months ago (2017-04-26 00:06:29 UTC) #36
commit-bot: I haz the power
3 years, 8 months ago (2017-04-26 00:13:50 UTC) #39
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/bfc507f8797d610a272dd3dbcf26...

Powered by Google App Engine
This is Rietveld 408576698