Make WindowTree::SetModal() take the type.

services/ui/ws/window_tree.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "services/ui/ws/window_tree.h"
 
 #include <stddef.h>
 
 #include <utility>
 
@@ skipping 334 matching lines @@
   if (!access_policy_->CanDeleteWindow(window) &&
       !ShouldRouteToWindowManager(window)) {
     return false;
   }
 
   // Have the owner of the tree service the actual delete.
   WindowTree* tree = window_server_->GetTreeWithId(window->id().client_id);
   return tree && tree->DeleteWindowImpl(this, window);
 }
 
-bool WindowTree::SetModal(const ClientWindowId& window_id) {
+bool WindowTree::SetModal(const ClientWindowId& window_id,
+                          ModalType modal_type) {
   ServerWindow* window = GetWindowByClientId(window_id);
-  if (window && access_policy_->CanSetModal(window)) {
-    WindowManagerDisplayRoot* display_root =
-        GetWindowManagerDisplayRoot(window);
-    if (window->transient_parent()) {
-      window->SetModal();
-    } else if (user_id_ != InvalidUserId()) {
-      if (display_root)
-        display_root->window_manager_state()->AddSystemModalWindow(window);
-    } else {
-      return false;
-    }
-    if (display_root)
-      display_root->window_manager_state()->ReleaseCaptureBlockedByModalWindow(
-          window);
-    return true;
+  if (!window || !access_policy_->CanSetModal(window))
+    return false;
+  // TODO(moshayedi): crbug.com/697176. When modality of a window that used to
+  // be a system modal changes, notify window manager state.
+  auto* display_root = GetWindowManagerDisplayRoot(window);
+  switch (modal_type) {

[sky, 2017/03/01 00:13:35]

Early out if modal_type matches?

[Hadi, 2017/03/06 20:07:05]

Done. Should we return true or false in this case? (I assumed true).

[sky, 2017/03/06 22:26:45]

Return true.

+    case MODAL_TYPE_NONE:

[sky, 2017/03/01 00:13:35]

Move this to MODEL_TYPE_WINDOW as it's the same implementation.

[Hadi, 2017/03/06 20:07:05]

I thought we shouldn't release capture when MODAL_TYPE_NONE. I agree that my
implementation was confusing and I should probably have checked this in the if()
for release capture. Changed this.

+      window->SetModal(MODAL_TYPE_NONE);
+      return true;
+    case MODAL_TYPE_SYSTEM:
+      if (user_id_ == InvalidUserId() || !display_root)

[sky, 2017/03/01 00:13:35]

Why the InvalidUserId check?

[Hadi, 2017/03/06 20:07:05]

The previous implementation checks for InvalidUserId for system modals. Also
window_tree.mojom says SetModal() can fail if client doesn't have a valid user
id.

+        return false;
+      display_root->window_manager_state()->AddSystemModalWindow(window);

[sky, 2017/03/01 00:13:35]

How come you don't SetModal(modal_type) in this case?

[Hadi, 2017/03/06 20:07:05]

AddSystemModalWindow() does this. I agree my implementation was a bit confusing,
so I also add SetModalType() to this case.

+      break;
+    case MODAL_TYPE_WINDOW:
+    case MODAL_TYPE_CHILD:
+      window->SetModal(modal_type);
+      break;
   }
-  return false;
+  if (display_root) {
+    display_root->window_manager_state()->ReleaseCaptureBlockedByModalWindow(
+        window);
+  }
+  return true;
 }
 
 std::vector<const ServerWindow*> WindowTree::GetWindowTree(
     const ClientWindowId& window_id) const {
   const ServerWindow* window = GetWindowByClientId(window_id);
   std::vector<const ServerWindow*> windows;
   if (window)
     GetWindowTreeImpl(window, &windows);
   return windows;
 }
@@ skipping 887 matching lines @@
       access_policy_->CanRemoveTransientWindowFromParent(transient_window)) {
     success = true;
     Operation op(this, window_server_,
                  OperationType::REMOVE_TRANSIENT_WINDOW_FROM_PARENT);
     transient_window->transient_parent()->RemoveTransientWindow(
         transient_window);
   }
   client()->OnChangeCompleted(change_id, success);
 }
 
-void WindowTree::SetModal(uint32_t change_id, Id window_id) {
-  client()->OnChangeCompleted(change_id, SetModal(ClientWindowId(window_id)));
+void WindowTree::SetModal(uint32_t change_id,
+                          Id window_id,
+                          ModalType modal_type) {
+  client()->OnChangeCompleted(change_id,
+                              SetModal(ClientWindowId(window_id), modal_type));
 }
 
 void WindowTree::ReorderWindow(uint32_t change_id,
                                Id window_id,
                                Id relative_window_id,
                                mojom::OrderDirection direction) {
   // TODO(erg): This implementation allows reordering two windows that are
   // children of a parent window which the two implementations can't see. There
   // should be a security check to prevent this.
   bool success = false;
@@ skipping 863 matching lines @@
   client()->OnCompleteDrop(client_window_id.id, event_flags, cursor_offset,
                            effect_bitmask, callback);
 }
 
 void WindowTree::PerformOnDragDropDone() {
   client()->OnDragDropDone();
 }
 
 }  // namespace ws
 }  // namespace ui