arc: Fix race condition in applying modal state.

components/exo/shell_surface.cc

 // Copyright 2015 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 "components/exo/shell_surface.h"
 
 #include <algorithm>
 
 #include "ash/common/frame/custom_frame_view_ash.h"
 #include "ash/common/shelf/wm_shelf.h"
@@ skipping 478 matching lines @@
   // managed system modal dialogs using a single ShellSurface instance.
   // Hit-test region will be non-empty when at least one dialog exists on
   // the client side. Here we detect the transition between no client side
   // dialog and at least one dialog so activatable state is properly
   // updated.
   if (container_ != ash::kShellWindowId_SystemModalContainer) {
     LOG(ERROR)
         << "Only a window in SystemModalContainer can change the modality";
     return;
   }
+
+  if (system_modal == system_modal_)
+    return;
+
+  system_modal_ = system_modal;
+
+  if (!widget_) {

[reveman, 2017/03/11 00:45:08]

nit: I don't think we should have this warning. Seems like perfectly valid
behavior by a client to call this before the widget exists now. Please change it
to "if (widget_) UpdateSystemModal();".

[khmel, 2017/03/11 01:33:32]

Nice, done

+    DLOG(WARNING) << "Skip setting system modal state to " << system_modal_
+                  << " because window is not ready yet.";
+    return;
+  }
+
+  UpdateSystemModal();
+}
+
+void ShellSurface::UpdateSystemModal() {
+  DCHECK(widget_);
+  DCHECK(!system_modal_ ||
+         container_ == ash::kShellWindowId_SystemModalContainer);

[reveman, 2017/03/11 00:45:09]

nit: just DCHECK_EQ(container_, ash::kShellWindowId_SystemModalContainer) here
and add a if "container_ == ash::kShellWindowId_SystemModalContainer" check to
OnSurfaceCommit

[khmel, 2017/03/11 01:33:32]

Done.

   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);
 }
 
 // static
 void ShellSurface::SetApplicationId(aura::Window* window,
                                     const std::string& id) {
   TRACE_EVENT1("exo", "ShellSurface::SetApplicationId", "application_id", id);
   window->SetProperty(aura::client::kAppIdKey, new std::string(id));
 }
 
 // static
@@ skipping 247 matching lines @@
       surface_->window()->SetTransform(transform);
       scale_ = pending_scale_;
     }
 
     // Show widget if needed.
     if (pending_show_widget_) {
       DCHECK(!widget_->IsClosed());
       DCHECK(!widget_->IsVisible());
       pending_show_widget_ = false;
       widget_->Show();
+      UpdateSystemModal();
     }
   }
 }
 
 bool ShellSurface::IsSurfaceSynchronized() const {
   // A shell surface is always desynchronized.
   return false;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ skipping 791 matching lines @@
     // small style shadow for them.
     if (!activatable_)
       shadow->SetElevation(wm::ShadowElevation::SMALL);
     // We don't have rounded corners unless frame is enabled.
     if (!frame_enabled_)
       shadow->SetRoundedCornerRadius(0);
   }
 }
 
 }  // namespace exo