Skip to content

Commit

Permalink
Ensure compositor initialized before WindowTreeHost is Shown.
Browse files Browse the repository at this point in the history
This is merely a sanity check. Technically you could show the host
without initializing it, but chances are you didn't meant to do that (or
you can easily change your code to Init before Show).

BUG=NONE

Review-Url: https://codereview.chromium.org/2552003002
Cr-Commit-Position: refs/heads/master@{#437285}
  • Loading branch information
mfomitchev authored and Commit bot committed Dec 8, 2016
1 parent 6241c26 commit 2212ddb
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 7 deletions.
1 change: 1 addition & 0 deletions ui/aura/mus/window_tree_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,7 @@ std::unique_ptr<WindowTreeHostMus> WindowTreeClient::CreateWindowTreeHost(
std::unique_ptr<WindowTreeHostMus> window_tree_host =
base::MakeUnique<WindowTreeHostMus>(std::move(window_port), this,
display_id);
window_tree_host->InitHost();
if (!window_data.is_null()) {
SetLocalPropertiesFromServerProperties(
WindowMus::Get(window_tree_host->window()), window_data);
Expand Down
21 changes: 16 additions & 5 deletions ui/aura/mus/window_tree_client_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,7 @@ TEST_F(WindowTreeClientClientTest, InputEventBasic) {
Window* top_level = window_tree_host.window();
const gfx::Rect bounds(0, 0, 100, 100);
window_tree_host.SetBoundsInPixels(bounds);
window_tree_host.InitHost();
window_tree_host.Show();
EXPECT_EQ(bounds, top_level->bounds());
EXPECT_EQ(bounds, window_tree_host.GetBoundsInPixels());
Expand Down Expand Up @@ -713,6 +714,7 @@ TEST_F(WindowTreeClientClientTest, NewTopLevelWindow) {
window_tree_client_impl()->GetRoots().size();
std::unique_ptr<WindowTreeHostMus> window_tree_host =
base::MakeUnique<WindowTreeHostMus>(window_tree_client_impl());
window_tree_host->InitHost();
aura::Window* top_level = window_tree_host->window();
// TODO: need to check WindowTreeHost visibility.
// EXPECT_TRUE(WindowPrivate(root2).parent_drawn());
Expand Down Expand Up @@ -757,6 +759,10 @@ TEST_F(WindowTreeClientClientTest, NewTopLevelWindowGetsPropertiesFromData) {
EXPECT_FALSE(IsWindowHostVisible(top_level));
EXPECT_FALSE(top_level->TargetVisibility());

// TODO(mfomitchev): crbug.com/672150 InitHost() currently makes the host
// visible, which shouldn't be the case.
window_tree_host.InitHost();

// Ack the request to the windowtree to create the new window.
EXPECT_EQ(window_tree()->window_id(), server_id(top_level));

Expand All @@ -770,13 +776,15 @@ TEST_F(WindowTreeClientClientTest, NewTopLevelWindowGetsPropertiesFromData) {
WindowTreeChangeType::NEW_TOP_LEVEL, &change_id));
window_tree_client()->OnTopLevelCreated(change_id, std::move(data),
display_id, true);
EXPECT_EQ(
0u, window_tree()->GetChangeCountForType(WindowTreeChangeType::VISIBLE));
// TODO(mfomitchev): Uncomment while crbug.com/crbug.com/672150 is fixed
// EXPECT_EQ(
// 0u,
// window_tree()->GetChangeCountForType(WindowTreeChangeType::VISIBLE));

// Make sure all the properties took.
EXPECT_TRUE(IsWindowHostVisible(top_level));
// TODO(mfomitchev): Uncomment while crbug.com/crbug.com/672150 is fixed
// EXPECT_TRUE(IsWindowHostVisible(top_level));
EXPECT_TRUE(top_level->TargetVisibility());
// TODO: check display_id.
EXPECT_EQ(display_id, window_tree_host.display_id());
EXPECT_EQ(gfx::Rect(0, 0, 3, 4), top_level->bounds());
EXPECT_EQ(gfx::Rect(1, 2, 3, 4), top_level->GetHost()->GetBoundsInPixels());
Expand All @@ -787,9 +795,10 @@ TEST_F(WindowTreeClientClientTest, NewWindowGetsAllChangesInFlight) {

WindowTreeHostMus window_tree_host(window_tree_client_impl());
Window* top_level = window_tree_host.window();

EXPECT_FALSE(top_level->TargetVisibility());

window_tree_host.InitHost();

// Make visibility go from false->true->false. Don't ack immediately.
top_level->Show();
top_level->Hide();
Expand Down Expand Up @@ -1024,6 +1033,7 @@ TEST_F(WindowTreeClientClientTest,
window_tree_client_impl()->GetRoots().size();
std::unique_ptr<WindowTreeHostMus> window_tree_host =
base::MakeUnique<WindowTreeHostMus>(window_tree_client_impl());
window_tree_host->InitHost();
EXPECT_EQ(initial_root_count + 1,
window_tree_client_impl()->GetRoots().size());

Expand Down Expand Up @@ -1060,6 +1070,7 @@ TEST_F(WindowTreeClientClientTest, NewTopLevelWindowGetsProperties) {
std::unique_ptr<WindowTreeHostMus> window_tree_host =
base::MakeUnique<WindowTreeHostMus>(window_tree_client_impl(),
&properties);
window_tree_host->InitHost();
// Verify the property made it to the window.
EXPECT_EQ(property_value,
window_tree_host->window()->GetProperty(kTestPropertyKey1));
Expand Down
7 changes: 5 additions & 2 deletions ui/aura/window_tree_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,11 @@ ui::EventDispatchDetails WindowTreeHost::DispatchKeyEventPostIME(
}

void WindowTreeHost::Show() {
if (compositor())
compositor()->SetVisible(true);
// Ensure that compositor has been properly initialized, see InitCompositor()
// and InitHost().
DCHECK(compositor());
DCHECK_EQ(compositor()->root_layer(), window()->layer());
compositor()->SetVisible(true);
ShowImpl();
}

Expand Down

0 comments on commit 2212ddb

Please sign in to comment.