chromeos: remove restriction that system modal windows must be in a display

Prior to this patch system modal windows had to be attached to a
display, otherwise the request to set modal failed. This is
problematic as the order done by NativeWidgetAura is to set modality,
then add.

BUG=692282
TEST=covered by test

Change-Id: Ibb5f96f7e44eac205eb39244bd61bca23ce45b03
Reviewed-on: https://chromium-review.googlesource.com/599147Reviewed-by: Elliot Glaysher <erg@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491574}

services/ui/ws/window_server.cc

@@ -717,9 +717,10 @@ void WindowServer::OnWindowHierarchyChanged(ServerWindow* window,
 
   WindowManagerDisplayRoot* display_root =
       display_manager_->GetWindowManagerDisplayRoot(window);
-  if (display_root)
+  if (display_root) {
     display_root->window_manager_state()
         ->ReleaseCaptureBlockedByAnyModalWindow();
+  }
 
   ProcessWindowHierarchyChanged(window, new_parent, old_parent);
 
@@ -732,6 +733,25 @@ void WindowServer::OnWindowHierarchyChanged(ServerWindow* window,
                                                     window->frame_sink_id());
   }
 
+  if (!pending_system_modal_windows_.windows().empty()) {
+    // Windows that are now in a display are put here, then removed. We do this
+    // in two passes to avoid removing from a list we're iterating over.
+    std::set<ServerWindow*> no_longer_pending;
+    for (ServerWindow* system_modal_window :
+         pending_system_modal_windows_.windows()) {
+      DCHECK_EQ(MODAL_TYPE_SYSTEM, system_modal_window->modal_type());
+      WindowManagerDisplayRoot* display_root =
+          display_manager_->GetWindowManagerDisplayRoot(system_modal_window);
+      if (display_root) {
+        no_longer_pending.insert(system_modal_window);
+        display_root->window_manager_state()->AddSystemModalWindow(window);
+      }
+    }
+
+    for (ServerWindow* system_modal_window : no_longer_pending)
+      pending_system_modal_windows_.Remove(system_modal_window);
+  }
+
   UpdateNativeCursorFromMouseLocation(window);
 }
 
@@ -864,6 +884,25 @@ void WindowServer::OnTransientWindowRemoved(ServerWindow* window,
   }
 }
 
+void WindowServer::OnWindowModalTypeChanged(ServerWindow* window,
+                                            ModalType old_modal_type) {
+  WindowManagerDisplayRoot* display_root =
+      display_manager_->GetWindowManagerDisplayRoot(window);
+  if (window->modal_type() == MODAL_TYPE_SYSTEM) {
+    if (display_root)
+      display_root->window_manager_state()->AddSystemModalWindow(window);
+    else
+      pending_system_modal_windows_.Add(window);
+  } else {
+    pending_system_modal_windows_.Remove(window);
+  }
+
+  if (display_root && window->modal_type() != MODAL_TYPE_NONE) {
+    display_root->window_manager_state()
+        ->ReleaseCaptureBlockedByAnyModalWindow();
+  }
+}
+
 void WindowServer::OnGpuServiceInitialized() {
   delegate_->StartDisplayInit();
 }

services/ui/ws/window_server.h

@@ -22,6 +22,7 @@
 #include "services/ui/ws/operation.h"
 #include "services/ui/ws/server_window_delegate.h"
 #include "services/ui/ws/server_window_observer.h"
+#include "services/ui/ws/server_window_tracker.h"
 #include "services/ui/ws/user_display_manager_delegate.h"
 #include "services/ui/ws/user_id_tracker.h"
 #include "services/ui/ws/user_id_tracker_observer.h"
@@ -357,6 +358,8 @@ class WindowServer : public ServerWindowDelegate,
                               ServerWindow* transient_child) override;
   void OnTransientWindowRemoved(ServerWindow* window,
                                 ServerWindow* transient_child) override;
+  void OnWindowModalTypeChanged(ServerWindow* window,
+                                ModalType old_modal_type) override;
 
   // GpuHostDelegate:
   void OnGpuServiceInitialized() override;
@@ -412,6 +415,10 @@ class WindowServer : public ServerWindowDelegate,
   // Provides interfaces to create and manage FrameSinks.
   std::unique_ptr<viz::mojom::FrameSinkManager> frame_sink_manager_;
 
+  // System modal windows not attached to a display are added here. Once
+  // attached to a display they are removed.
+  ServerWindowTracker pending_system_modal_windows_;
+
   DisplayCreationConfig display_creation_config_;
 
   DISALLOW_COPY_AND_ASSIGN(WindowServer);

services/ui/ws/window_tree.cc

@@ -552,26 +552,7 @@ bool WindowTree::SetModalType(const ClientWindowId& window_id,
   if (window->modal_type() == modal_type)
     return true;
 
-  auto* display_root = GetWindowManagerDisplayRoot(window);
-  switch (modal_type) {
-    case MODAL_TYPE_SYSTEM:
-      if (!display_root) {
-        DVLOG(1) << "SetModalType failed (no display root)";
-        return false;
-      }
-      window->SetModalType(modal_type);
-      display_root->window_manager_state()->AddSystemModalWindow(window);
-      break;
-    case MODAL_TYPE_NONE:
-    case MODAL_TYPE_WINDOW:
-    case MODAL_TYPE_CHILD:
-      window->SetModalType(modal_type);
-      break;
-  }
-  if (display_root && modal_type != MODAL_TYPE_NONE) {
-    display_root->window_manager_state()->ReleaseCaptureBlockedByModalWindow(
-        window);
-  }
+  window->SetModalType(modal_type);
   return true;
 }
 

services/ui/ws/window_tree_unittest.cc

@@ -745,6 +745,29 @@ TEST_F(WindowTreeTest, ModalTypeSystemToModalTypeNone) {
             modal_window_controller_test_api.GetActiveSystemModalWindow());
 }
 
+TEST_F(WindowTreeTest, ModalTypeSystemUnparentedThenParented) {
+  const ClientWindowId test_window_id = BuildClientWindowId(wm_tree(), 21);
+  EXPECT_TRUE(wm_tree()->NewWindow(test_window_id, ServerWindow::Properties()));
+  ServerWindow* test_window = wm_tree()->GetWindowByClientId(test_window_id);
+  ASSERT_TRUE(test_window);
+  test_window->SetVisible(true);
+  const ClientWindowId wm_root_id = FirstRootId(wm_tree());
+  EXPECT_TRUE(wm_tree()->SetModalType(test_window_id, MODAL_TYPE_SYSTEM));
+  WindowManagerState* wms =
+      display()->GetActiveWindowManagerDisplayRoot()->window_manager_state();
+  ModalWindowControllerTestApi modal_window_controller_test_api(
+      EventDispatcherTestApi(wms->event_dispatcher())
+          .modal_window_controller());
+  EXPECT_EQ(nullptr,
+            modal_window_controller_test_api.GetActiveSystemModalWindow());
+  EXPECT_TRUE(wm_tree()->AddWindow(wm_root_id, test_window_id));
+  EXPECT_EQ(test_window,
+            modal_window_controller_test_api.GetActiveSystemModalWindow());
+  EXPECT_TRUE(wm_tree()->SetModalType(test_window_id, MODAL_TYPE_NONE));
+  EXPECT_EQ(nullptr,
+            modal_window_controller_test_api.GetActiveSystemModalWindow());
+}
+
 // Establish client, call NewTopLevelWindow(), make sure get id, and make
 // sure client paused.
 TEST_F(WindowTreeTest, NewTopLevelWindow) {
@@ -1117,7 +1140,7 @@ TEST_F(WindowTreeTest, MoveCaptureWindowToModalParent) {
   EXPECT_NO_FATAL_FAILURE(SetupEventTargeting(&embed_client, &tree, &w1));
 
   w1->SetBounds(gfx::Rect(10, 10, 30, 30));
-  const ServerWindow* root_window = *tree->roots().begin();
+  ServerWindow* root_window = FirstRoot(tree);
   ClientWindowId root_window_id = ClientWindowIdForWindow(tree, root_window);
   ClientWindowId w1_id = ClientWindowIdForWindow(tree, w1);
   Display* display = tree->GetDisplay(w1);