Commit a5f21099 authored by Ria Jiang's avatar Ria Jiang Committed by Commit Bot

Change FrameSinkId for embed windows to be in embedded tree's id space.

When a client submits hit-test data for its own children, the client_id
part of the frame_sink_id of its children is 0 and HitTestManager is
supposed to fill in the correct client_id. However, if the root window
of this client (e.g. browser, quick-launch, renderer; WindowManager is
an exception) is being embedded, then the root window is really created
by its embedder and has the client_id of its embedder. In these cases,
we are filling in the wrong client_id if we just get the client_id part
in CompositorFrameSinkSupport::frame_sink_id_. Essentially, in mus/mash,
we are submitting compositor frames and hit-test data using parent’s
client_id.

This CL changes FrameSinkId for embed windows to be in embedded tree's
id space. Currently, there are two ways for embedding:
1) An embedded client creates a top level window by asking its WindowTree
in NewTopLevelWindow(). The window_id provided in this call by the client
is the same window_id used in its WindowTree. However, the associated
ServerWindow is created by the WindowManager(embedder), and so its
FrameSinkId used to include the WindowManager's client id. This CL
changes that, so that the FrameSinkId of the ServerWindow is generated
using the client_id (and window_id) of the embedded client.
2) The embedder creates the aura::Window (the embed window) and asks its
WindowTreeClient -> WindoeTree -> WindowServer to create a new WT for the
embed client and embed it at the embed window. That embedded client will
get notified in OnEmbed with that new tree and the embed window (and
other information).
For 1), this CL plumbs the client_window_id for the embed window from the
embedded tree to the embedder tree and use that as the new FrameSinkId to
make sure we don't use that FrameSinkId in the embedded tree again.
For 2), right now it's only used by renderers (RendererWindowTreeClient)
and we don't provide a window id to use for the embed window like case 1.
This CL changes the new FrameSinkId for the embed window to be
(embedded tree's id, 0) so that next_window_id_ on the embedded client
side can remain unchanged.

Bug: 712302
Test: covered by tests
Change-Id: Iaa6c5e477784f9887db696f74fa2f7bd938ab67c
Reviewed-on: https://chromium-review.googlesource.com/748442
Commit-Queue: Ria Jiang <riajiang@chromium.org>
Reviewed-by: 's avatarSadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: 's avatarTom Sepez <tsepez@chromium.org>
Reviewed-by: 's avatarScott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516142}
parent 80520100
......@@ -188,10 +188,10 @@ interface WindowManager {
WmSetCanFocus(uint32 window_id, bool can_focus);
// Asks the WindowManager to create a new window.
// |requesting_client_id| is the id of the client issuing the request. This
// allows the window manager to track top level windows by client.
// |frame_sink_id| is the FrameSinkId for the new window. Window manager
// tracks top level windows by client with |frame_sink_id|.client_id().
WmCreateTopLevelWindow(uint32 change_id,
uint16 requesting_client_id,
viz.mojom.FrameSinkId frame_sink_id,
map<string, array<uint8>> properties);
// A WindowTreeClient is considered "janky" by Mus when it stops ACK'ing input
......
......@@ -32,7 +32,6 @@ ServerWindow::ServerWindow(ServerWindowDelegate* delegate,
const Properties& properties)
: delegate_(delegate),
id_(id),
frame_sink_id_(frame_sink_id),
parent_(nullptr),
stacking_target_(nullptr),
transient_parent_(nullptr),
......@@ -51,15 +50,7 @@ ServerWindow::ServerWindow(ServerWindowDelegate* delegate,
observers_(
base::ObserverList<ServerWindowObserver>::NOTIFY_EXISTING_ONLY) {
DCHECK(delegate); // Must provide a delegate.
// TODO(kylechar): Add method to reregister |frame_sink_id_| when viz service
// has crashed.
DCHECK(frame_sink_id_.is_valid());
auto* host_frame_sink_manager = delegate_->GetHostFrameSinkManager();
DCHECK(host_frame_sink_manager);
host_frame_sink_manager->RegisterFrameSinkId(frame_sink_id_, this);
#if DCHECK_IS_ON()
host_frame_sink_manager->SetFrameSinkDebugLabel(frame_sink_id_, GetName());
#endif
UpdateFrameSinkId(frame_sink_id);
}
ServerWindow::~ServerWindow() {
......@@ -127,6 +118,26 @@ void ServerWindow::CreateCompositorFrameSink(
frame_sink_id_, std::move(request), std::move(client));
}
void ServerWindow::UpdateFrameSinkId(const viz::FrameSinkId& frame_sink_id) {
DCHECK(frame_sink_id.is_valid());
auto* host_frame_sink_manager = delegate_->GetHostFrameSinkManager();
DCHECK(host_frame_sink_manager);
host_frame_sink_manager->RegisterFrameSinkId(frame_sink_id, this);
#if DCHECK_IS_ON()
host_frame_sink_manager->SetFrameSinkDebugLabel(frame_sink_id, GetName());
#endif
if (frame_sink_id_.is_valid()) {
if (parent()) {
host_frame_sink_manager->UnregisterFrameSinkHierarchy(
parent()->frame_sink_id(), frame_sink_id_);
host_frame_sink_manager->RegisterFrameSinkHierarchy(
parent()->frame_sink_id(), frame_sink_id);
}
host_frame_sink_manager->InvalidateFrameSinkId(frame_sink_id_);
}
frame_sink_id_ = frame_sink_id;
}
void ServerWindow::Add(ServerWindow* child) {
// We assume validation checks happened already.
DCHECK(child);
......
......@@ -80,6 +80,7 @@ class ServerWindow : public viz::HostFrameSinkClient {
const WindowId& id() const { return id_; }
const viz::FrameSinkId& frame_sink_id() const { return frame_sink_id_; }
void UpdateFrameSinkId(const viz::FrameSinkId& frame_sink_id);
const base::Optional<viz::LocalSurfaceId>& current_local_surface_id() const {
return current_local_surface_id_;
......@@ -249,7 +250,8 @@ class ServerWindow : public viz::HostFrameSinkClient {
ServerWindowDelegate* const delegate_;
const WindowId id_;
const viz::FrameSinkId frame_sink_id_;
// This may change for embed windows.
viz::FrameSinkId frame_sink_id_;
base::Optional<viz::LocalSurfaceId> current_local_surface_id_;
ServerWindow* parent_;
......
......@@ -32,7 +32,7 @@ namespace test {
namespace {
ClientWindowId NextUnusedClientWindowId(WindowTree* tree) {
for (ClientSpecificId id = 1;; ++id) {
for (ClientSpecificId id = kEmbedTreeWindowId;; ++id) {
// Used the id of the client in the upper bits to simplify things.
const ClientWindowId client_id = ClientWindowId(tree->id(), id);
if (!tree->GetWindowByClientId(client_id))
......@@ -236,7 +236,7 @@ void TestWindowManager::WmSetModalType(uint32_t window_id, ui::ModalType type) {
void TestWindowManager::WmCreateTopLevelWindow(
uint32_t change_id,
ClientSpecificId requesting_client_id,
const viz::FrameSinkId& frame_sink_id,
const std::unordered_map<std::string, std::vector<uint8_t>>& properties) {
got_create_top_level_window_ = true;
change_id_ = change_id;
......@@ -638,7 +638,7 @@ void WindowEventTargetingHelper::CreateSecondaryTree(
ServerWindow** window) {
WindowTree* tree1 = window_server()->GetTreeWithRoot(embed_window);
ASSERT_TRUE(tree1 != nullptr);
const ClientWindowId child1_id(tree1->id(), 1);
const ClientWindowId child1_id(tree1->id(), kEmbedTreeWindowId);
ASSERT_TRUE(tree1->NewWindow(child1_id, ServerWindow::Properties()));
ServerWindow* child1 = tree1->GetWindowByClientId(child1_id);
ASSERT_TRUE(child1);
......
......@@ -51,6 +51,7 @@ namespace test {
const ClientSpecificId kWindowManagerClientId = kWindowServerClientId + 1;
const std::string kWindowManagerClientIdString =
std::to_string(kWindowManagerClientId);
const ClientSpecificId kEmbedTreeWindowId = 1;
// Collection of utilities useful in creating mus tests.
......@@ -411,7 +412,7 @@ class TestWindowManager : public mojom::WindowManager {
void WmSetCanFocus(uint32_t window_id, bool can_focus) override {}
void WmCreateTopLevelWindow(
uint32_t change_id,
ClientSpecificId requesting_client_id,
const viz::FrameSinkId& frame_sink_id,
const std::unordered_map<std::string, std::vector<uint8_t>>& properties)
override;
void WmClientJankinessChanged(ClientSpecificId client_id,
......@@ -753,7 +754,7 @@ class WindowEventTargetingHelper {
TestDisplayBinding* display_binding_ = nullptr;
// Owned by WindowServer's DisplayManager.
Display* display_ = nullptr;
ClientSpecificId next_primary_tree_window_id_ = 1;
ClientSpecificId next_primary_tree_window_id_ = kEmbedTreeWindowId;
DISALLOW_COPY_AND_ASSIGN(WindowEventTargetingHelper);
};
......
......@@ -511,7 +511,8 @@ TEST_F(WindowManagerStateTest, DeleteNonRootTree) {
ASSERT_EQ(1u, tracker->changes()->size());
// clients that created this window is receiving the event, so client_id part
// would be reset to 0 before sending back to clients.
EXPECT_EQ("InputEvent window=0,1 event_action=7",
EXPECT_EQ("InputEvent window=0," + std::to_string(kEmbedTreeWindowId) +
" event_action=7",
ChangesToDescription1(*tracker->changes())[0]);
EXPECT_TRUE(wm_client()->tracker()->changes()->empty());
......@@ -616,6 +617,8 @@ TEST_F(WindowManagerStateTest, InterceptingEmbedderReceivesEvents) {
const uint32_t embed_flags = mojom::kEmbedFlagEmbedderInterceptsEvents;
WindowTree* embed_tree = nullptr;
TestWindowTreeClient* embed_client_proxy = nullptr;
const ClientWindowId embed_client_window_id =
embedder_window->frame_sink_id();
EmbedAt(embedder_tree, embed_window_id, embed_flags, &embed_tree,
&embed_client_proxy);
ASSERT_TRUE(embed_client_proxy);
......@@ -641,7 +644,8 @@ TEST_F(WindowManagerStateTest, InterceptingEmbedderReceivesEvents) {
// Embed another tree in the embedded tree.
const ClientWindowId nested_embed_window_id(embed_tree->id(), 23);
embed_tree->NewWindow(nested_embed_window_id, ServerWindow::Properties());
ASSERT_TRUE(embed_tree->AddWindow(embed_window_id, nested_embed_window_id));
ASSERT_TRUE(
embed_tree->AddWindow(embed_client_window_id, nested_embed_window_id));
WindowTree* nested_embed_tree = nullptr;
TestWindowTreeClient* nested_embed_client_proxy = nullptr;
......
......@@ -155,6 +155,7 @@ WindowTree* WindowServer::EmbedAtWindow(
AddTree(std::move(tree_ptr), std::move(binding), std::move(window_tree_ptr));
OnTreeMessagedClient(tree->id());
root->UpdateFrameSinkId(ClientWindowId(tree->id(), 0));
return tree;
}
......@@ -352,7 +353,7 @@ void WindowServer::WindowManagerChangeCompleted(
void WindowServer::WindowManagerCreatedTopLevelWindow(
WindowTree* wm_tree,
uint32_t window_manager_change_id,
const ServerWindow* window) {
ServerWindow* window) {
InFlightWindowManagerChange change;
if (!GetAndClearInFlightWindowManagerChange(window_manager_change_id,
&change)) {
......@@ -383,8 +384,11 @@ void WindowServer::WindowManagerCreatedTopLevelWindow(
return;
}
tree->OnWindowManagerCreatedTopLevelWindow(window_manager_change_id,
change.client_change_id, window);
viz::FrameSinkId updated_frame_sink_id =
tree->OnWindowManagerCreatedTopLevelWindow(
window_manager_change_id, change.client_change_id, window);
if (window)
window->UpdateFrameSinkId(updated_frame_sink_id);
}
void WindowServer::ProcessWindowBoundsChanged(
......
......@@ -174,7 +174,7 @@ class WindowServer : public ServerWindowDelegate,
bool success);
void WindowManagerCreatedTopLevelWindow(WindowTree* wm_tree,
uint32_t window_manager_change_id,
const ServerWindow* window);
ServerWindow* window);
// Called when we get an unexpected message from the WindowManager.
// TODO(sky): decide what we want to do here.
......
......@@ -736,7 +736,7 @@ bool WindowTree::IsWaitingForNewTopLevelWindow(uint32_t wm_change_id) {
waiting_for_top_level_window_info_->wm_change_id == wm_change_id;
}
void WindowTree::OnWindowManagerCreatedTopLevelWindow(
viz::FrameSinkId WindowTree::OnWindowManagerCreatedTopLevelWindow(
uint32_t wm_change_id,
uint32_t client_change_id,
const ServerWindow* window) {
......@@ -750,7 +750,7 @@ void WindowTree::OnWindowManagerCreatedTopLevelWindow(
waiting_for_top_level_window_info->client_window_id));
if (!window) {
client()->OnChangeCompleted(client_change_id, false);
return;
return viz::FrameSinkId();
}
client_id_to_window_id_map_[waiting_for_top_level_window_info
->client_window_id] = window->id();
......@@ -763,6 +763,7 @@ void WindowTree::OnWindowManagerCreatedTopLevelWindow(
client()->OnTopLevelCreated(client_change_id, WindowToWindowData(window),
display_id, drawn,
window->current_local_surface_id());
return waiting_for_top_level_window_info->client_window_id;
}
void WindowTree::AddActivationParent(const ClientWindowId& window_id) {
......@@ -1598,8 +1599,8 @@ void WindowTree::NewTopLevelWindow(
display_root->window_manager_state()
->window_tree()
->window_manager_internal_->WmCreateTopLevelWindow(wm_change_id, id_,
transport_properties);
->window_manager_internal_->WmCreateTopLevelWindow(
wm_change_id, client_window_id, transport_properties);
}
void WindowTree::DeleteWindow(uint32_t change_id, Id transport_window_id) {
......@@ -2639,10 +2640,6 @@ void WindowTree::OnWmCreatedTopLevelWindow(uint32_t change_id,
window_server_->WindowManagerSentBogusMessage();
window = nullptr;
}
if (window) {
client()->OnFrameSinkIdAllocated(transport_window_id,
window->frame_sink_id());
}
window_server_->WindowManagerCreatedTopLevelWindow(this, change_id, window);
}
......
......@@ -211,9 +211,10 @@ class WindowTree : public mojom::WindowTree,
DispatchEventCallback callback);
bool IsWaitingForNewTopLevelWindow(uint32_t wm_change_id);
void OnWindowManagerCreatedTopLevelWindow(uint32_t wm_change_id,
uint32_t client_change_id,
const ServerWindow* window);
viz::FrameSinkId OnWindowManagerCreatedTopLevelWindow(
uint32_t wm_change_id,
uint32_t client_change_id,
const ServerWindow* window);
void AddActivationParent(const ClientWindowId& window_id);
// Calls through to the client.
......
......@@ -232,11 +232,9 @@ class TestWindowTreeClient : public mojom::WindowTreeClient,
}
// Waits for all messages to be received by |ws|. This is done by attempting
// to create a bogus window. When we get the response we know all messages
// have been processed.
bool WaitForAllMessages() {
return NewWindowWithCompleteId(kInvalidClientId) == 0;
}
// to set opacity on an embed/invalid window. 1.0f is the default opacity
// value. When we get the response we know all messages have been processed.
bool WaitForAllMessages() { return !SetWindowOpacity(0, 1.0f); }
Id NewWindow(ClientSpecificId window_id) {
return NewWindowWithCompleteId(window_id);
......@@ -504,7 +502,7 @@ class TestWindowTreeClient : public mojom::WindowTreeClient,
void WmSetCanFocus(uint32_t window_id, bool can_focus) override {}
void WmCreateTopLevelWindow(
uint32_t change_id,
ClientSpecificId requesting_client_id,
const viz::FrameSinkId& frame_sink_id,
const std::unordered_map<std::string, std::vector<uint8_t>>& properties)
override {
NOTIMPLEMENTED();
......@@ -1580,7 +1578,10 @@ TEST_F(WindowTreeClientTest, EmbedWithSameWindowId2) {
// Create a window in the third client and parent it to the root.
Id window_3_1 = wt_client3()->NewWindow(1);
ASSERT_TRUE(window_3_1);
ASSERT_TRUE(wt_client3()->AddWindow(window_1_1, window_3_1));
// After EstablishThirdClient, window_1_1 should have a ClientWindowId of
// (client_id_2, 0).
Id embedded_window_1_1_wt3 = BuildWindowId(client_id_2(), 0);
ASSERT_TRUE(wt_client3()->AddWindow(embedded_window_1_1_wt3, window_3_1));
// wt1 created window_1_1 but not window_3_1.
Id window11_in_wt1 = LoWord(window_1_1);
......@@ -1601,15 +1602,18 @@ TEST_F(WindowTreeClientTest, EmbedWithSameWindowId2) {
// We should get a new client for the new embedding.
std::unique_ptr<TestWindowTreeClient> client4(
EstablishClientViaEmbed(wt1(), window_1_1));
Id embedded_window_1_1_wt4 = BuildWindowId(client_id_3(), 0);
ASSERT_TRUE(client4.get());
EXPECT_EQ("[" + WindowParentToString(window_1_1, kNullParentId) + "]",
EXPECT_EQ("[" +
WindowParentToString(embedded_window_1_1_wt4, kNullParentId) +
"]",
ChangeWindowDescription(*client4->tracker()->changes()));
// And 3 should get an unembed and delete.
wt_client3_->WaitForChangeCount(2);
EXPECT_EQ("OnUnembed window=" + IdToString(window_1_1),
EXPECT_EQ("OnUnembed window=" + IdToString(embedded_window_1_1_wt3),
ChangesToDescription1(*changes3())[0]);
EXPECT_EQ("WindowDeleted window=" + IdToString(window_1_1),
EXPECT_EQ("WindowDeleted window=" + IdToString(embedded_window_1_1_wt3),
ChangesToDescription1(*changes3())[1]);
}
......@@ -2397,8 +2401,8 @@ TEST_F(WindowTreeClientTest, SurfaceIdPropagation) {
viz::FrameSinkId frame_sink_id =
changes1()->back().surface_id.frame_sink_id();
// FrameSinkId is based on window's ClientWindowId.
EXPECT_NE(0u, frame_sink_id.client_id());
EXPECT_EQ(LoWord(window_1_100), frame_sink_id.sink_id());
EXPECT_EQ(static_cast<size_t>(client_id_2()), frame_sink_id.client_id());
EXPECT_EQ(0u, frame_sink_id.sink_id());
changes1()->clear();
// The first window created in the second client gets a server id of
......
This diff is collapsed.
......@@ -1798,9 +1798,10 @@ void WindowTreeClient::WmSetCanFocus(Id window_id, bool can_focus) {
void WindowTreeClient::WmCreateTopLevelWindow(
uint32_t change_id,
ClientSpecificId requesting_client_id,
const viz::FrameSinkId& frame_sink_id,
const std::unordered_map<std::string, std::vector<uint8_t>>&
transport_properties) {
DCHECK(frame_sink_id.is_valid());
std::map<std::string, std::vector<uint8_t>> properties =
mojo::UnorderedMapToMap(transport_properties);
ui::mojom::WindowType window_type = ui::mojom::WindowType::UNKNOWN;
......@@ -1818,10 +1819,13 @@ void WindowTreeClient::WmCreateTopLevelWindow(
kInvalidServerId);
return;
}
embedded_windows_[requesting_client_id].insert(window);
embedded_windows_[base::checked_cast<ClientSpecificId>(
frame_sink_id.client_id())]
.insert(window);
if (window_manager_client_) {
window_manager_client_->OnWmCreatedTopLevelWindow(
change_id, WindowMus::Get(window)->server_id());
OnFrameSinkIdAllocated(WindowMus::Get(window)->server_id(), frame_sink_id);
}
}
......
......@@ -473,7 +473,7 @@ class AURA_EXPORT WindowTreeClient
void WmSetCanFocus(Id window_id, bool can_focus) override;
void WmCreateTopLevelWindow(
uint32_t change_id,
ClientSpecificId requesting_client_id,
const viz::FrameSinkId& frame_sink_id,
const std::unordered_map<std::string, std::vector<uint8_t>>&
transport_properties) override;
void WmClientJankinessChanged(ClientSpecificId client_id,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment