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

Issue 2764433003: mus-ws: Plumb FrameSinkId to Children (Closed)

Created:
3 years, 9 months ago by Fady Samuel
Modified:
3 years, 9 months ago
Reviewers:
Tom Sepez, sadrul, sky, piman
CC:
chromium-reviews, rjkroege, mlamouri+watch-content_chromium.org, sadrul, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, tfarina, jam, yzshen+watch_chromium.org, abarth-chromium, jbauman+watch_chromium.org, Aaron Boodman, darin-cc_chromium.org, piman+watch_chromium.org, kalyank, danakj+watch_chromium.org, darin (slow to review)
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

mus-ws: Plumb FrameSinkId to Children This CL takes us a step closer to surface synchronization by plumbing the correct FrameSinkId to children and delaying CompositorFrameSink creation until the FrameSinkId is propagated to the child. FrameSinkIds are plumbed to children via: 1. WindowTreeClient::OnTopLevelCreated -- for top level windows like Chrome 2. WindowTreeClient::OnEmbed -- for embedded apps and OOPIFs 3. WindowManager::WmNewDisplayAdded -- for the window manager's CompositorFrameSink Requesting a CompositorFrameSink from WindowPortMus is now async, and completes when the FrameSinkId is available. BUG=672962 TBR=piman@chromium.org Review-Url: https://codereview.chromium.org/2764433003 Cr-Commit-Position: refs/heads/master@{#458703} Committed: https://chromium.googlesource.com/chromium/src/+/95c974eb51c3626888d8cc74d0e47624da956210

Patch Set 1 #

Patch Set 2 : Renders #

Patch Set 3 : Use the correct FrameSinkId from the renderer #

Patch Set 4 : Rebase and update unit tests #

Patch Set 5 : Rebased #

Patch Set 6 : Cleanup #

Total comments: 4

Patch Set 7 : Addressed Scott's comments #

Patch Set 8 : Fixed unit test #

Total comments: 2

Patch Set 9 : Addressed Antoine's comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+305 lines, -148 lines) Patch
M ash/mus/window_manager_unittest.cc View 1 2 3 4 5 6 7 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/ui/views/frame/browser_frame_mus.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M content/renderer/mus/renderer_window_tree_client.h View 1 2 3 4 5 6 7 8 5 chunks +18 lines, -10 lines 0 comments Download
M content/renderer/mus/renderer_window_tree_client.cc View 1 2 3 4 5 6 7 8 4 chunks +45 lines, -28 lines 0 comments Download
M content/renderer/render_thread_impl.cc View 1 2 3 4 1 chunk +4 lines, -5 lines 0 comments Download
M services/ui/public/interfaces/window_manager.mojom View 1 2 2 chunks +3 lines, -1 line 0 comments Download
M services/ui/public/interfaces/window_tree.mojom View 3 chunks +5 lines, -2 lines 0 comments Download
M services/ui/ws/test_change_tracker.h View 1 2 3 3 chunks +5 lines, -2 lines 0 comments Download
M services/ui/ws/test_change_tracker.cc View 1 2 3 4 chunks +16 lines, -9 lines 0 comments Download
M services/ui/ws/test_utils.h View 1 2 3 4 3 chunks +6 lines, -3 lines 0 comments Download
M services/ui/ws/test_utils.cc View 1 2 3 4 2 chunks +10 lines, -7 lines 0 comments Download
M services/ui/ws/window_manager_client_unittest.cc View 2 chunks +4 lines, -2 lines 0 comments Download
M services/ui/ws/window_tree.cc View 1 2 3 chunks +6 lines, -5 lines 0 comments Download
M services/ui/ws/window_tree_client_unittest.cc View 1 2 3 6 chunks +21 lines, -7 lines 0 comments Download
M services/ui/ws/window_tree_unittest.cc View 1 2 3 1 chunk +14 lines, -8 lines 0 comments Download
M ui/aura/mus/mus_context_factory.h View 1 chunk +4 lines, -0 lines 0 comments Download
M ui/aura/mus/mus_context_factory.cc View 1 chunk +11 lines, -2 lines 0 comments Download
M ui/aura/mus/window_mus.h View 1 chunk +2 lines, -0 lines 0 comments Download
M ui/aura/mus/window_port_mus.h View 3 chunks +14 lines, -2 lines 0 comments Download
M ui/aura/mus/window_port_mus.cc View 1 2 3 4 5 2 chunks +33 lines, -5 lines 0 comments Download
M ui/aura/mus/window_tree_client.h View 1 2 3 4 5 6 5 chunks +15 lines, -8 lines 0 comments Download
M ui/aura/mus/window_tree_client.cc View 1 2 3 4 5 6 8 chunks +22 lines, -13 lines 0 comments Download
M ui/aura/mus/window_tree_client_unittest.cc View 1 2 3 4 9 chunks +17 lines, -14 lines 0 comments Download
M ui/aura/mus/window_tree_host_mus.h View 1 2 3 4 5 6 2 chunks +2 lines, -0 lines 0 comments Download
M ui/aura/mus/window_tree_host_mus.cc View 1 4 chunks +10 lines, -5 lines 0 comments Download
M ui/aura/mus/window_tree_host_mus_unittest.cc View 1 2 3 4 5 6 2 chunks +4 lines, -2 lines 0 comments Download
M ui/aura/test/mus/window_tree_client_private.cc View 2 chunks +4 lines, -3 lines 0 comments Download
M ui/views/mus/desktop_window_tree_host_mus.h View 1 chunk +1 line, -0 lines 0 comments Download
M ui/views/mus/desktop_window_tree_host_mus.cc View 1 chunk +2 lines, -0 lines 0 comments Download
M ui/views/mus/mus_client.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download
M ui/views/test/native_widget_factory_aura_mus.cc View 1 chunk +2 lines, -1 line 0 comments Download

Messages

Total messages: 57 (37 generated)
Fady Samuel
Hi Scott, As discussed on Friday, I'm plumbing FrameSinkId to the child in a separate ...
3 years, 9 months ago (2017-03-21 19:03:07 UTC) #11
Fady Samuel
+sadrul@ FYI!
3 years, 9 months ago (2017-03-21 19:06:26 UTC) #15
sky
https://codereview.chromium.org/2764433003/diff/100001/ui/aura/mus/window_tree_client.cc File ui/aura/mus/window_tree_client.cc (right): https://codereview.chromium.org/2764433003/diff/100001/ui/aura/mus/window_tree_client.cc#newcode460 ui/aura/mus/window_tree_client.cc:460: WindowMus::Get(window_tree_host->window()) How come you call this here and not ...
3 years, 9 months ago (2017-03-21 21:12:03 UTC) #18
Fady Samuel
PTAL Scott! https://codereview.chromium.org/2764433003/diff/100001/ui/aura/mus/window_tree_client.cc File ui/aura/mus/window_tree_client.cc (right): https://codereview.chromium.org/2764433003/diff/100001/ui/aura/mus/window_tree_client.cc#newcode460 ui/aura/mus/window_tree_client.cc:460: WindowMus::Get(window_tree_host->window()) On 2017/03/21 21:12:03, sky wrote: > ...
3 years, 9 months ago (2017-03-21 22:03:29 UTC) #19
Fady Samuel
+tsepez@ for mojom.
3 years, 9 months ago (2017-03-21 22:10:39 UTC) #23
Fady Samuel
PTAL. Accidentally broke ash_unittests. Fixed.
3 years, 9 months ago (2017-03-21 22:30:44 UTC) #26
Tom Sepez
mojom LGTM
3 years, 9 months ago (2017-03-21 22:58:21 UTC) #27
sky
LGTM
3 years, 9 months ago (2017-03-22 00:02:39 UTC) #30
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/2764433003/140001
3 years, 9 months ago (2017-03-22 00:07:45 UTC) #32
Fady Samuel
Ooops, I need a content/ OWNER stamp for content/renderer/render_thread_impl.cc mus specific code. +piman@. I'm TBR'ing ...
3 years, 9 months ago (2017-03-22 00:16:51 UTC) #34
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/2764433003/140001
3 years, 9 months ago (2017-03-22 00:20:19 UTC) #39
piman
https://codereview.chromium.org/2764433003/diff/140001/content/renderer/mus/renderer_window_tree_client.cc File content/renderer/mus/renderer_window_tree_client.cc (right): https://codereview.chromium.org/2764433003/diff/140001/content/renderer/mus/renderer_window_tree_client.cc#newcode61 content/renderer/mus/renderer_window_tree_client.cc:61: base::Bind(&RendererWindowTreeClient::RequestCompositorFrameSinkInternal, Why use a callback for this? Couldn't we ...
3 years, 9 months ago (2017-03-22 00:38:22 UTC) #40
Fady Samuel
Ohh, noticed you just replied. Un-CQ'ing. Do you feel strongly about whether I use a ...
3 years, 9 months ago (2017-03-22 00:40:40 UTC) #42
piman
On 2017/03/22 00:40:40, Fady Samuel wrote: > Ohh, noticed you just replied. Un-CQ'ing. Do you ...
3 years, 9 months ago (2017-03-22 00:45:00 UTC) #43
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/2764433003/160001
3 years, 9 months ago (2017-03-22 01:09:44 UTC) #46
commit-bot: I haz the power
Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_TIMED_OUT, build hasn't started yet, builder ...
3 years, 9 months ago (2017-03-22 03:11:11 UTC) #48
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/2764433003/160001
3 years, 9 months ago (2017-03-22 03:15:45 UTC) #50
commit-bot: I haz the power
Exceeded global retry quota
3 years, 9 months ago (2017-03-22 05:17:33 UTC) #52
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/2764433003/160001
3 years, 9 months ago (2017-03-22 09:55:54 UTC) #54
commit-bot: I haz the power
3 years, 9 months ago (2017-03-22 10:01:20 UTC) #57
Message was sent while issue was closed.
Committed patchset #9 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/95c974eb51c3626888d8cc74d0e4...

Powered by Google App Engine
This is Rietveld 408576698