Commit 1111902f authored by khmel's avatar khmel Committed by Commit bot

arc: Fix race condition in applying modal state.

There is race when request to set system modal received before
surface window is actually created. This CL keeps system model
property and apply it once window is created.

TEST=Manually, simulate situation for early system modal switch.
     Before fix it crashed with the same stack trace as crash
     reports. After this I observed warning message in log and
     system modal state is working as expected.
BUG=639511

Review-Url: https://codereview.chromium.org/2743053002
Cr-Commit-Position: refs/heads/master@{#456256}
parent 31e7b909
...@@ -496,9 +496,22 @@ void ShellSurface::SetSystemModal(bool system_modal) { ...@@ -496,9 +496,22 @@ void ShellSurface::SetSystemModal(bool system_modal) {
<< "Only a window in SystemModalContainer can change the modality"; << "Only a window in SystemModalContainer can change the modality";
return; return;
} }
if (system_modal == system_modal_)
return;
system_modal_ = system_modal;
if (widget_)
UpdateSystemModal();
}
void ShellSurface::UpdateSystemModal() {
DCHECK(widget_);
DCHECK_EQ(container_, ash::kShellWindowId_SystemModalContainer);
widget_->GetNativeWindow()->SetProperty( widget_->GetNativeWindow()->SetProperty(
aura::client::kModalKey, aura::client::kModalKey,
system_modal ? ui::MODAL_TYPE_SYSTEM : ui::MODAL_TYPE_NONE); system_modal_ ? ui::MODAL_TYPE_SYSTEM : ui::MODAL_TYPE_NONE);
} }
// static // static
...@@ -766,6 +779,8 @@ void ShellSurface::OnSurfaceCommit() { ...@@ -766,6 +779,8 @@ void ShellSurface::OnSurfaceCommit() {
DCHECK(!widget_->IsVisible()); DCHECK(!widget_->IsVisible());
pending_show_widget_ = false; pending_show_widget_ = false;
widget_->Show(); widget_->Show();
if (container_ == ash::kShellWindowId_SystemModalContainer)
UpdateSystemModal();
} }
} }
} }
......
...@@ -307,6 +307,9 @@ class ShellSurface : public SurfaceDelegate, ...@@ -307,6 +307,9 @@ class ShellSurface : public SurfaceDelegate,
// |pending_shadow_content_bounds_|. // |pending_shadow_content_bounds_|.
void UpdateShadow(); void UpdateShadow();
// Applies |system_modal_| to |widget_|.
void UpdateSystemModal();
views::Widget* widget_ = nullptr; views::Widget* widget_ = nullptr;
Surface* surface_; Surface* surface_;
aura::Window* parent_; aura::Window* parent_;
...@@ -347,6 +350,7 @@ class ShellSurface : public SurfaceDelegate, ...@@ -347,6 +350,7 @@ class ShellSurface : public SurfaceDelegate,
int top_inset_height_ = 0; int top_inset_height_ = 0;
int pending_top_inset_height_ = 0; int pending_top_inset_height_ = 0;
bool shadow_underlay_in_surface_ = true; bool shadow_underlay_in_surface_ = true;
bool system_modal_ = false;
DISALLOW_COPY_AND_ASSIGN(ShellSurface); DISALLOW_COPY_AND_ASSIGN(ShellSurface);
}; };
......
...@@ -471,6 +471,34 @@ TEST_F(ShellSurfaceTest, ModalWindow) { ...@@ -471,6 +471,34 @@ TEST_F(ShellSurfaceTest, ModalWindow) {
EXPECT_FALSE(ash::WmShell::Get()->IsSystemModalWindowOpen()); EXPECT_FALSE(ash::WmShell::Get()->IsSystemModalWindowOpen());
} }
TEST_F(ShellSurfaceTest, ModalWindowSetSystemModalBeforeCommit) {
std::unique_ptr<Surface> surface(new Surface);
std::unique_ptr<ShellSurface> shell_surface(new ShellSurface(
surface.get(), nullptr, ShellSurface::BoundsMode::SHELL, gfx::Point(),
true, false, ash::kShellWindowId_SystemModalContainer));
gfx::Size desktop_size(640, 480);
std::unique_ptr<Buffer> desktop_buffer(
new Buffer(exo_test_helper()->CreateGpuMemoryBuffer(desktop_size)));
surface->Attach(desktop_buffer.get());
surface->SetInputRegion(SkRegion());
// Set SetSystemModal before any commit happens. Widget is not created at
// this time.
EXPECT_FALSE(shell_surface->GetWidget());
shell_surface->SetSystemModal(true);
surface->Commit();
// It is expected that modal window is shown.
EXPECT_TRUE(shell_surface->GetWidget());
EXPECT_TRUE(ash::WmShell::Get()->IsSystemModalWindowOpen());
// Now widget is created and setting modal state should be applied
// immediately.
shell_surface->SetSystemModal(false);
EXPECT_FALSE(ash::WmShell::Get()->IsSystemModalWindowOpen());
}
TEST_F(ShellSurfaceTest, PopupWindow) { TEST_F(ShellSurfaceTest, PopupWindow) {
Surface parent_surface; Surface parent_surface;
ShellSurface parent(&parent_surface); ShellSurface parent(&parent_surface);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment