chromeos: adds support for RequestClose and wires up in ash

When a remote client creates a window, the remote client should control when it
is closed. This makes it to so that when the user attemps to close a window
created by a remote client, the remote client is asked to close the window.

BUG=842298
TEST=covered by test

Change-Id: I9ace655c0648fc0f8943425c67fb57e088ef00fa
Reviewed-on: https://chromium-review.googlesource.com/1087870
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564897}

ash/BUILD.gn

@@ -1880,6 +1880,7 @@ test("ash_unittests") {
     "wm/lock_state_controller_unittest.cc",
     "wm/mru_window_tracker_unittest.cc",
     "wm/native_cursor_manager_ash_unittest.cc",
+    "wm/non_client_frame_controller_unittest.cc",
     "wm/overlay_event_filter_unittest.cc",
     "wm/overview/cleanup_animation_observer_unittest.cc",
     "wm/overview/window_selector_unittest.cc",
@@ -2386,7 +2387,6 @@ source_set("mash_unittests") {
     "app_launch_unittest.cc",
     "display/display_synchronizer_unittest.cc",
     "window_manager_unittest.cc",
-    "wm/non_client_frame_controller_unittest.cc",
     "wm/top_level_window_factory_mash_unittest.cc",
   ]
 

ash/wm/non_client_frame_controller.cc

@@ -16,15 +16,18 @@
 #include "ash/public/cpp/ash_layout_constants.h"
 #include "ash/public/cpp/immersive/immersive_fullscreen_controller_delegate.h"
 #include "ash/public/cpp/window_properties.h"
+#include "ash/shell.h"
 #include "ash/wm/move_event_handler.h"
 #include "ash/wm/panels/panel_frame_view.h"
 #include "ash/wm/property_util.h"
 #include "ash/wm/window_properties.h"
 #include "ash/wm/window_util.h"
+#include "ash/ws/window_service_owner.h"
 #include "base/macros.h"
 #include "base/strings/utf_string_conversions.h"
 #include "services/ui/public/interfaces/window_manager.mojom.h"
 #include "services/ui/ws2/window_properties.h"
+#include "services/ui/ws2/window_service.h"
 #include "ui/aura/client/aura_constants.h"
 #include "ui/aura/client/transient_window_client.h"
 #include "ui/aura/mus/property_converter.h"
@@ -253,14 +256,18 @@ class ClientViewMus : public views::ClientView {
 
   // views::ClientView:
   bool CanClose() override {
-    // TODO(crbug.com/842298): Add support for window-service as a library.
-    if (!frame_controller_->window() ||
-        !frame_controller_->window_manager_client()) {
-      return true;
+    // CanClose() is called when the user requests the window to close (such as
+    // clicking the close button). As this window is managed by a remote client
+    // pass the request to the remote client and return false (to cancel the
+    // close). If the remote client wants the window to close, it will close it
+    // in a way that does not reenter this code path.
+    if (frame_controller_->window_manager_client()) {
+      frame_controller_->window_manager_client()->RequestClose(
+          frame_controller_->window());
+    } else {
+      Shell::Get()->window_service_owner()->window_service()->RequestClose(
+          frame_controller_->window());
     }
-
-    frame_controller_->window_manager_client()->RequestClose(
-        frame_controller_->window());
     return false;
   }
 

ash/wm/non_client_frame_controller_unittest.cc

@@ -5,6 +5,8 @@
 #include "ash/wm/non_client_frame_controller.h"
 
 #include "ash/public/cpp/ash_layout_constants.h"
+#include "ash/public/cpp/config.h"
+#include "ash/shell.h"
 #include "ash/test/ash_test_base.h"
 #include "ash/test/ash_test_helper.h"
 #include "ash/window_manager.h"
@@ -15,6 +17,8 @@
 #include "components/viz/common/quads/compositor_frame.h"
 #include "components/viz/common/quads/solid_color_draw_quad.h"
 #include "services/ui/public/interfaces/window_tree_constants.mojom.h"
+#include "services/ui/ws2/test_change_tracker.h"
+#include "services/ui/ws2/test_window_tree_client.h"
 #include "ui/aura/env.h"
 #include "ui/aura/window.h"
 #include "ui/compositor/compositor.h"
@@ -74,10 +78,10 @@ bool FindTiledContentQuad(const viz::CompositorFrame& frame,
 
 }  // namespace
 
-class NonClientFrameControllerTest : public AshTestBase {
+class NonClientFrameControllerMashTest : public AshTestBase {
  public:
-  NonClientFrameControllerTest() = default;
-  ~NonClientFrameControllerTest() override = default;
+  NonClientFrameControllerMashTest() = default;
+  ~NonClientFrameControllerMashTest() override = default;
 
   const viz::CompositorFrame& GetLastCompositorFrame() const {
     return context_factory_.GetLastCompositorFrame();
@@ -101,10 +105,13 @@ class NonClientFrameControllerTest : public AshTestBase {
   ui::FakeContextFactory context_factory_;
   ui::ContextFactory* context_factory_to_restore_ = nullptr;
 
-  DISALLOW_COPY_AND_ASSIGN(NonClientFrameControllerTest);
+  DISALLOW_COPY_AND_ASSIGN(NonClientFrameControllerMashTest);
 };
 
-TEST_F(NonClientFrameControllerTest, ContentRegionNotDrawnForClient) {
+TEST_F(NonClientFrameControllerMashTest, ContentRegionNotDrawnForClient) {
+  if (Shell::GetAshConfig() != Config::MASH)
+    return;  // TODO: decide if this test should be made to work with ws2.
+
   std::map<std::string, std::vector<uint8_t>> properties;
   auto* window_manager =
       ash_test_helper()->window_manager_service()->window_manager();
@@ -182,4 +189,21 @@ TEST_F(NonClientFrameControllerTest, ContentRegionNotDrawnForClient) {
   }
 }
 
+using NonClientFrameControllerTest = AshTestBase;
+
+TEST_F(NonClientFrameControllerTest, CallsRequestClose) {
+  std::unique_ptr<aura::Window> window = CreateTestWindow();
+  NonClientFrameController* non_client_frame_controller =
+      NonClientFrameController::Get(window.get());
+  ASSERT_TRUE(non_client_frame_controller);
+  non_client_frame_controller->GetWidget()->Close();
+  // Close should not have been scheduled on the widget yet (because the request
+  // goes to the remote client).
+  EXPECT_FALSE(non_client_frame_controller->GetWidget()->IsClosed());
+  auto* changes = GetTestWindowTreeClient()->tracker()->changes();
+  ASSERT_FALSE(changes->empty());
+  // The remote client should have a request to close the window.
+  EXPECT_EQ("RequestClose", ui::ws2::ChangeToDescription(changes->back()));
+}
+
 }  // namespace ash

services/ui/ws2/test_change_tracker.cc

@@ -147,6 +147,8 @@ std::string ChangeToDescription(const Change& change,
       return base::StringPrintf("OpacityChanged window_id=%s opacity=%.2f",
                                 WindowIdToString(change.window_id).c_str(),
                                 change.float_value);
+    case CHANGE_TYPE_REQUEST_CLOSE:
+      return "RequestClose";
     case CHANGE_TYPE_SURFACE_CHANGED:
       return base::StringPrintf("SurfaceCreated window_id=%s surface_id=%s",
                                 WindowIdToString(change.window_id).c_str(),
@@ -475,6 +477,13 @@ void TestChangeTracker::OnWindowSurfaceChanged(
   AddChange(change);
 }
 
+void TestChangeTracker::RequestClose(Id window_id) {
+  Change change;
+  change.type = CHANGE_TYPE_REQUEST_CLOSE;
+  change.window_id = window_id;
+  AddChange(change);
+}
+
 void TestChangeTracker::AddChange(const Change& change) {
   changes_.push_back(change);
   if (delegate_)

services/ui/ws2/test_change_tracker.h

@@ -44,6 +44,7 @@ enum ChangeType {
   CHANGE_TYPE_ON_CHANGE_COMPLETED,
   CHANGE_TYPE_ON_TOP_LEVEL_CREATED,
   CHANGE_TYPE_OPACITY,
+  CHANGE_TYPE_REQUEST_CLOSE,
   CHANGE_TYPE_SURFACE_CHANGED,
   CHANGE_TYPE_TRANSFORM_CHANGED,
 };
@@ -202,6 +203,7 @@ class TestChangeTracker {
                          bool drawn);
   void OnWindowSurfaceChanged(Id window_id,
                               const viz::SurfaceInfo& surface_info);
+  void RequestClose(Id window_id);
 
  private:
   void AddChange(const Change& change);

services/ui/ws2/test_window_tree_client.cc

@@ -272,7 +272,9 @@ void TestWindowTreeClient::OnChangeCompleted(uint32_t change_id, bool success) {
   tracker_.OnChangeCompleted(change_id, success);
 }
 
-void TestWindowTreeClient::RequestClose(Id window_id) {}
+void TestWindowTreeClient::RequestClose(Id window_id) {
+  tracker_.RequestClose(window_id);
+}
 
 void TestWindowTreeClient::GetWindowManager(
     mojo::AssociatedInterfaceRequest<mojom::WindowManager> internal) {}

services/ui/ws2/window_service.cc

@@ -68,6 +68,12 @@ bool WindowService::HasRemoteClient(aura::Window* window) {
   return ClientWindow::GetMayBeNull(window);
 }
 
+void WindowService::RequestClose(aura::Window* window) {
+  ClientWindow* client_window = ClientWindow::GetMayBeNull(window);
+  DCHECK(window && client_window->IsTopLevel());
+  client_window->owning_window_service_client()->RequestClose(client_window);
+}
+
 void WindowService::OnStart() {
   window_tree_factory_ = std::make_unique<WindowTreeFactory>(this);
 

services/ui/ws2/window_service.h

@@ -86,6 +86,10 @@ class COMPONENT_EXPORT(WINDOW_SERVICE) WindowService
 
   aura::client::FocusClient* focus_client() { return focus_client_; }
 
+  // Asks the client that created |window| to close |window|. |window| must be
+  // a top-level window.
+  void RequestClose(aura::Window* window);
+
   // service_manager::Service:
   void OnStart() override;
   void OnBindInterface(const service_manager::BindSourceInfo& remote_info,

services/ui/ws2/window_service_client.cc

@@ -141,6 +141,12 @@ bool WindowServiceClient::IsTopLevel(aura::Window* window) {
   return iter != client_roots_.end() && (*iter)->is_top_level();
 }
 
+void WindowServiceClient::RequestClose(ClientWindow* window) {
+  DCHECK(window->IsTopLevel());
+  DCHECK_EQ(this, window->owning_window_service_client());
+  window_tree_client_->RequestClose(TransportIdForWindow(window->window()));
+}
+
 ClientRoot* WindowServiceClient::CreateClientRoot(
     aura::Window* window,
     bool is_top_level,

services/ui/ws2/window_service_client.h

@@ -84,6 +84,9 @@ class COMPONENT_EXPORT(WINDOW_SERVICE) WindowServiceClient
   // NewTopLevelWindow().
   bool IsTopLevel(aura::Window* window);
 
+  // Asks the client to close |window|. |window| must be a top-level window.
+  void RequestClose(ClientWindow* window);
+
   WindowService* window_service() { return window_service_; }
 
  private:

testing/buildbot/filters/mash.ash_unittests.filter

@@ -189,6 +189,9 @@
 # TODO: Maybe due to dragging across displays. http://crbug.com/698888
 -NormalPanelPopup/PanelWindowResizerTransientTest.PanelWithTransientChild/0
 
+# Only works with classic.
+-NonClientFrameControllerTest.CallsRequestClose
+
 # TODO: Triage
 -OverviewGestureHandlerTest.HorizontalScrollInOverview
 -OverviewGestureHandlerTest.ScrollUpDownWithoutReleasing