New issue
Advanced search Search tips

Issue 780635 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner: ----
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

some Embed code in mus-ws is only used in tests

Project Member Reported by riajiang@chromium.org, Nov 1 2017

Issue description

Currently, there are two ways for embedding:

1) embed client in window manager: embedded client creates a WindowPort for its top level window and asks its WindowTree in NewTopLevelWindow. Embedded WT then asks embedder (window manager) to create that top level window and after a series of function calls, embedded client gets notified of that window.

2) embed renderer in browser: embedded client only has WindowTreeClient but not a WindowTree yet. Embedder creates the aura::Window (the embed window) and asks its WindowTreeClient::Embed -> WindowTree::Embed -> WindowServer::EmbedAtWindow to create a WT for that client with that corresponding WTC and embed it at the embed window. Embedded client will get notified in OnEmbed with that new tree and the embed window (and other information).

Since we now only use the second path for renderers, some code in mus-ws is only used in tests:
We don't use WindowTreeHostFactory::CreateWindowTreeHost[1] to create ws::Display anymore; as a result, ws::Display::binding_ is always null so we never take the DisplayBindingImpl::CreateWindowTree[2] (instead we use Display::CreateWindowManagerDisplayRootFromFactory [3]) -> WindowServer::EmbedAtWindow[4] -> WindowTreeClient::OnEmbed[5] path.
(WindowServer::EmbedAtWindow and RendererWindowTreeClient::OnEmbed are used for renderers.)

Are we going to use these functions again in the future? (I am also curious about the history behind this...)
If they are only used in tests, should we just delete them? However, they are used in WindowTreeTest, WindowManagerStateTest, WindowServerTest and WindowManagerServiceTest.OpenWindow so we'll have to update all the tests.

[1]https://cs.chromium.org/chromium/src/services/ui/ws/window_tree_host_factory.cc?type=cs&q=WindowTreeHostFactory::CreateWindowTreeHost&l=26
[2]https://cs.chromium.org/chromium/src/services/ui/ws/display_binding.cc?gsn=CreateWindowTree&l=29
[3]https://cs.chromium.org/chromium/src/services/ui/ws/display.cc?type=cs&l=242
[4]https://cs.chromium.org/chromium/src/services/ui/ws/window_server.cc?type=cs&q=embedatwindow&l=129
[5]https://cs.chromium.org/chromium/src/ui/aura/mus/window_tree_client.cc?type=cs&q=window_tree_client.cc&l=1117
 

Comment 1 by sky@chromium.org, Nov 2 2017

Status: WontFix (was: Untriaged)
I'm going to revive this code content_browsertests with --mus. The thinking is other platforms would use the embed apis you mention (say windows).
Components: -Internals>MUS Internals>Services>WindowService

Sign in to add a comment