arc: Fix race condition in applying modal state.

components/exo/shell_surface.cc

Index: components/exo/shell_surface.cc
diff --git a/components/exo/shell_surface.cc b/components/exo/shell_surface.cc
index 00a36dafe34afe065a4edb1d2dce8b858e4dc83e..8c0cd6230176fb8f0b08b07168351f295c2b261f 100644
--- a/components/exo/shell_surface.cc
+++ b/components/exo/shell_surface.cc
@@ -496,9 +496,17 @@ void ShellSurface::SetSystemModal(bool system_modal) {
         << "Only a window in SystemModalContainer can change the modality";
     return;
   }
+
+  system_modal_ = system_modal;

[reveman, 2017/03/10 21:38:22]

should we return early if it didn't change?

[khmel, 2017/03/10 22:11:32]

Done.

+  if (!widget_) {
+    LOG(WARNING) << "Skip setting system modal state to " << system_modal_

[reveman, 2017/03/10 21:38:22]

We shouldn't LOG things that can be triggered by a client as that allows a
malicious client to fill our log files. Maybe DLOG if important but in this case
it seems like it's just something we should handle silently.

[khmel, 2017/03/10 22:11:32]

Done.

+                 << " because window is not ready yet.";
+    return;
+  }
+
   widget_->GetNativeWindow()->SetProperty(
       aura::client::kModalKey,
-      system_modal ? ui::MODAL_TYPE_SYSTEM : ui::MODAL_TYPE_NONE);
+      system_modal_ ? ui::MODAL_TYPE_SYSTEM : ui::MODAL_TYPE_NONE);
 }

[reveman, 2017/03/10 21:38:22]

Please add a private UpdateSystemModal() function that contains l.502-l.510 and
call that here instead.

[khmel, 2017/03/10 22:11:32]

Done.

 
 // static
@@ -766,6 +774,8 @@ void ShellSurface::OnSurfaceCommit() {
       DCHECK(!widget_->IsVisible());
       pending_show_widget_ = false;
       widget_->Show();
+      if (system_modal_)

[reveman, 2017/03/10 21:38:22]

Please replace these two lines with a call to UpdateSystemModal().

[khmel, 2017/03/10 22:11:32]

Done.

+        SetSystemModal(true);
     }
   }
 }