Makes DeleteWindow() take a change id

The client crashes if the deletion fails

BUG=none
TEST=none
R=ben@chromium.org

Review URL: https://codereview.chromium.org/1485713003

Cr-Commit-Position: refs/heads/master@{#362420}

10 files changed, 24 insertions, 28 deletions

diff --git a/components/mus/public/cpp/lib/in_flight_change.h b/components/mus/public/cpp/lib/in_flight_change.h
index 80c6780..3dc42a7 100644
--- a/components/mus/public/cpp/lib/in_flight_change.h
+++ b/components/mus/public/cpp/lib/in_flight_change.h
@@ -20,8 +20,9 @@ class Window;
 enum class ChangeType {
   ADD_TRANSIENT_WINDOW,
   BOUNDS,
-  PROPERTY,
+  DELETE_WINDOW,
   NEW_WINDOW,
+  PROPERTY,
   REMOVE_TRANSIENT_WINDOW_FROM_PARENT
 };
 
diff --git a/components/mus/public/cpp/lib/window.cc b/components/mus/public/cpp/lib/window.cc
index 5284785..8dffc7f 100644
--- a/components/mus/public/cpp/lib/window.cc
+++ b/components/mus/public/cpp/lib/window.cc
@@ -167,7 +167,7 @@ void Window::Destroy() {
     return;
 
   if (connection_)
-    tree_client()->DestroyWindow(id_);
+    tree_client()->DestroyWindow(this);
   while (!children_.empty()) {
     Window* child = children_.front();
     if (!OwnsWindow(connection_, child)) {
diff --git a/components/mus/public/cpp/lib/window_tree_client_impl.cc b/components/mus/public/cpp/lib/window_tree_client_impl.cc
index dee1d57..163f1a3 100644
--- a/components/mus/public/cpp/lib/window_tree_client_impl.cc
+++ b/components/mus/public/cpp/lib/window_tree_client_impl.cc
@@ -150,9 +150,11 @@ void WindowTreeClientImpl::WaitForEmbed() {
   // TODO(sky): deal with pipe being closed before we get OnEmbed().
 }
 
-void WindowTreeClientImpl::DestroyWindow(Id window_id) {
+void WindowTreeClientImpl::DestroyWindow(Window* window) {
   DCHECK(tree_);
-  tree_->DeleteWindow(window_id, ActionCompletedCallback());
+  const uint32_t change_id = ScheduleInFlightChange(make_scoped_ptr(
+      new CrashInFlightChange(window, ChangeType::DELETE_WINDOW)));
+  tree_->DeleteWindow(change_id, window->id());
 }
 
 void WindowTreeClientImpl::AddChild(Id child_id, Id parent_id) {
diff --git a/components/mus/public/cpp/lib/window_tree_client_impl.h b/components/mus/public/cpp/lib/window_tree_client_impl.h
index baa116e..16caa4c 100644
--- a/components/mus/public/cpp/lib/window_tree_client_impl.h
+++ b/components/mus/public/cpp/lib/window_tree_client_impl.h
@@ -43,7 +43,7 @@ class WindowTreeClientImpl : public WindowTreeConnection,
 
   // API exposed to the window implementations that pushes local changes to the
   // service.
-  void DestroyWindow(Id window_id);
+  void DestroyWindow(Window* window);
 
   // These methods take TransportIds. For windows owned by the current
   // connection, the connection id high word can be zero. In all cases, the
diff --git a/components/mus/public/cpp/tests/test_window_tree.cc b/components/mus/public/cpp/tests/test_window_tree.cc
index 031a3c2..46c6451 100644
--- a/components/mus/public/cpp/tests/test_window_tree.cc
+++ b/components/mus/public/cpp/tests/test_window_tree.cc
@@ -25,8 +25,7 @@ void TestWindowTree::NewWindow(
     uint32_t window_id,
     mojo::Map<mojo::String, mojo::Array<uint8_t>> properties) {}
 
-void TestWindowTree::DeleteWindow(uint32_t window_id,
-                                  const DeleteWindowCallback& callback) {}
+void TestWindowTree::DeleteWindow(uint32_t change_id, uint32_t window_id) {}
 
 void TestWindowTree::SetWindowBounds(uint32_t change_id,
                                      uint32_t window_id,
diff --git a/components/mus/public/cpp/tests/test_window_tree.h b/components/mus/public/cpp/tests/test_window_tree.h
index 56eace6..6db82a4 100644
--- a/components/mus/public/cpp/tests/test_window_tree.h
+++ b/components/mus/public/cpp/tests/test_window_tree.h
@@ -27,8 +27,7 @@ class TestWindowTree : public mojom::WindowTree {
       uint32_t change_id,
       uint32_t window_id,
       mojo::Map<mojo::String, mojo::Array<uint8_t>> properties) override;
-  void DeleteWindow(uint32_t window_id,
-                    const DeleteWindowCallback& callback) override;
+  void DeleteWindow(uint32_t change_id, uint32_t window_id) override;
   void SetWindowBounds(uint32_t change_id,
                        uint32_t window_id,
                        mojo::RectPtr bounds) override;
diff --git a/components/mus/public/interfaces/window_tree.mojom b/components/mus/public/interfaces/window_tree.mojom
index 1e70b2e..455929a 100644
--- a/components/mus/public/interfaces/window_tree.mojom
+++ b/components/mus/public/interfaces/window_tree.mojom
@@ -95,7 +95,7 @@ interface WindowTree {
   // Deletes a window. This does not recurse. No hierarchy change notifications
   // are sent as a result of this. Only the connection that created the window
   // can delete it.
-  DeleteWindow(uint32 window_id) => (bool success);
+  DeleteWindow(uint32 change_id, uint32 window_id);
 
   // Sets the specified bounds of the specified window.
   SetWindowBounds(uint32 change_id, uint32 window_id, mojo.Rect bounds);
diff --git a/components/mus/ws/window_tree_apptest.cc b/components/mus/ws/window_tree_apptest.cc
index 63ba178..70613fc 100644
--- a/components/mus/ws/window_tree_apptest.cc
+++ b/components/mus/ws/window_tree_apptest.cc
@@ -134,15 +134,6 @@ void GetWindowTree(WindowTree* ws,
   run_loop.Run();
 }
 
-bool DeleteWindow(WindowTree* ws, Id window_id) {
-  base::RunLoop run_loop;
-  bool result = false;
-  ws->DeleteWindow(window_id,
-                   base::Bind(&BoolResultCallback, &run_loop, &result));
-  run_loop.Run();
-  return result;
-}
-
 bool SetWindowVisibility(WindowTree* ws, Id window_id, bool visible) {
   base::RunLoop run_loop;
   bool result = false;
@@ -226,6 +217,12 @@ class TestWindowTreeClientImpl : public mojom::WindowTreeClient,
     return on_change_completed_result_;
   }
 
+  bool DeleteWindow(Id id) {
+    const uint32_t change_id = GetAndAdvanceChangeId();
+    tree()->DeleteWindow(change_id, id);
+    return WaitForChangeCompleted(change_id);
+  }
+
   // 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.
@@ -1012,7 +1009,7 @@ TEST_F(WindowTreeAppTest, DeleteWindow) {
   {
     changes1()->clear();
     changes2()->clear();
-    ASSERT_TRUE(DeleteWindow(ws2(), window_2_2));
+    ASSERT_TRUE(ws_client2()->DeleteWindow(window_2_2));
     EXPECT_TRUE(changes2()->empty());
 
     ws_client1_->WaitForChangeCount(1);
@@ -1024,7 +1021,7 @@ TEST_F(WindowTreeAppTest, DeleteWindow) {
 // Verifies DeleteWindow isn't allowed from a separate connection.
 TEST_F(WindowTreeAppTest, DeleteWindowFromAnotherConnectionDisallowed) {
   ASSERT_NO_FATAL_FAILURE(EstablishSecondConnection(true));
-  EXPECT_FALSE(DeleteWindow(ws2(), BuildWindowId(connection_id_1(), 1)));
+  EXPECT_FALSE(ws_client2()->DeleteWindow(BuildWindowId(connection_id_1(), 1)));
 }
 
 // Verifies if a window was deleted and then reused that other clients are
@@ -1050,7 +1047,7 @@ TEST_F(WindowTreeAppTest, ReuseDeletedWindowId) {
   // Delete 2.
   {
     changes1()->clear();
-    ASSERT_TRUE(DeleteWindow(ws2(), window_2_2));
+    ASSERT_TRUE(ws_client2()->DeleteWindow(window_2_2));
 
     ws_client1_->WaitForChangeCount(1);
     EXPECT_EQ("WindowDeleted window=" + IdToString(window_2_2),
@@ -1749,7 +1746,7 @@ TEST_F(WindowTreeAppTest, TransientWindowTracksTransientParentLifetime) {
             SingleChangeToDescription(*changes1()));
 
   changes1()->clear();
-  ASSERT_TRUE(DeleteWindow(ws2(), window_2_1));
+  ASSERT_TRUE(ws_client2()->DeleteWindow(window_2_1));
   ws_client1()->WaitForChangeCount(2);
   EXPECT_EQ("WindowDeleted window=" + IdToString(window_2_2),
             ChangesToDescription1(*changes1())[0]);
diff --git a/components/mus/ws/window_tree_impl.cc b/components/mus/ws/window_tree_impl.cc
index c9bcb47..18edf8a 100644
--- a/components/mus/ws/window_tree_impl.cc
+++ b/components/mus/ws/window_tree_impl.cc
@@ -634,8 +634,7 @@ void WindowTreeImpl::NewWindow(
       NewWindow(WindowIdFromTransportId(transport_window_id), properties));
 }
 
-void WindowTreeImpl::DeleteWindow(Id transport_window_id,
-                                  const Callback<void(bool)>& callback) {
+void WindowTreeImpl::DeleteWindow(uint32_t change_id, Id transport_window_id) {
   ServerWindow* window =
       GetWindow(WindowIdFromTransportId(transport_window_id));
   bool success = false;
@@ -646,7 +645,7 @@ void WindowTreeImpl::DeleteWindow(Id transport_window_id,
         connection_manager_->GetConnection(window->id().connection_id);
     success = connection && connection->DeleteWindowImpl(this, window);
   }
-  callback.Run(success);
+  client_->OnChangeCompleted(change_id, success);
 }
 
 void WindowTreeImpl::AddWindow(Id parent_id,
diff --git a/components/mus/ws/window_tree_impl.h b/components/mus/ws/window_tree_impl.h
index 7503302..e17d0d0 100644
--- a/components/mus/ws/window_tree_impl.h
+++ b/components/mus/ws/window_tree_impl.h
@@ -196,8 +196,7 @@ class WindowTreeImpl : public mojom::WindowTree, public AccessPolicyDelegate {
                  Id transport_window_id,
                  mojo::Map<mojo::String, mojo::Array<uint8_t>>
                      transport_properties) override;
-  void DeleteWindow(Id transport_window_id,
-                    const mojo::Callback<void(bool)>& callback) override;
+  void DeleteWindow(uint32_t change_id, Id transport_window_id) override;
   void AddWindow(Id parent_id,
                  Id child_id,
                  const mojo::Callback<void(bool)>& callback) override;