diff options
author | nkostylev@chromium.org <nkostylev@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-06-03 15:59:48 +0000 |
---|---|---|
committer | nkostylev@chromium.org <nkostylev@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-06-03 15:59:48 +0000 |
commit | 8358e0d98e1cb301992abd1ad482cdd6c681f54c (patch) | |
tree | 382b22198edf515a88aadce3bc6ace526fa745c7 | |
parent | 22c80f339e9bb472d75e5c14957f6df2caa8b2ff (diff) | |
download | chromium_src-8358e0d98e1cb301992abd1ad482cdd6c681f54c.zip chromium_src-8358e0d98e1cb301992abd1ad482cdd6c681f54c.tar.gz chromium_src-8358e0d98e1cb301992abd1ad482cdd6c681f54c.tar.bz2 |
Merge 203191 "Fix crash when JS alert from background page appea..."
> Fix crash when JS alert from background page appears during lock screen
>
> + Added test that caused crash without changes in ash/wm/stacking_controller.cc
> + Changes in chrome/browser/ui/views/frame/browser_view.cc prevents stealing focus loop
>
> BUG=226141
> TEST=ShellTest.CreateLockScreenModalWindow
>
> Review URL: https://chromiumcodereview.appspot.com/15690015
TBR=dpolukhin@chromium.org
Review URL: https://codereview.chromium.org/16140012
git-svn-id: svn://svn.chromium.org/chrome/branches/1500/src@203720 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | ash/shell_unittest.cc | 27 | ||||
-rw-r--r-- | ash/wm/stacking_controller.cc | 16 | ||||
-rw-r--r-- | chrome/browser/ui/views/frame/browser_view.cc | 6 |
3 files changed, 41 insertions, 8 deletions
diff --git a/ash/shell_unittest.cc b/ash/shell_unittest.cc index 70ca5c2..be1d1cd 100644 --- a/ash/shell_unittest.cc +++ b/ash/shell_unittest.cc @@ -25,6 +25,7 @@ #include "ui/gfx/size.h" #include "ui/views/widget/widget.h" #include "ui/views/widget/widget_delegate.h" +#include "ui/views/window/dialog_delegate.h" using aura::RootWindow; @@ -207,6 +208,16 @@ TEST_F(ShellTest, CreateModalWindow) { widget->Close(); } +class TestModalDialogDelegate : public views::DialogDelegateView { + public: + TestModalDialogDelegate() {} + + // Overridden from views::WidgetDelegate: + virtual ui::ModalType GetModalType() const OVERRIDE { + return ui::MODAL_TYPE_SYSTEM; + } +}; + TEST_F(ShellTest, CreateLockScreenModalWindow) { views::Widget::InitParams widget_params( views::Widget::InitParams::TYPE_WINDOW); @@ -214,6 +225,7 @@ TEST_F(ShellTest, CreateLockScreenModalWindow) { // Create a normal window. views::Widget* widget = CreateTestWindow(widget_params); widget->Show(); + EXPECT_TRUE(widget->GetNativeView()->HasFocus()); // It should be in default container. EXPECT_TRUE(GetDefaultContainer()->Contains( @@ -227,6 +239,7 @@ TEST_F(ShellTest, CreateLockScreenModalWindow) { ash::internal::kShellWindowId_LockScreenContainer)-> AddChild(lock_widget->GetNativeView()); lock_widget->Show(); + EXPECT_TRUE(lock_widget->GetNativeView()->HasFocus()); // It should be in LockScreen container. aura::Window* lock_screen = Shell::GetContainer( @@ -238,6 +251,7 @@ TEST_F(ShellTest, CreateLockScreenModalWindow) { views::Widget* lock_modal_widget = views::Widget::CreateWindowWithParent( new ModalWindow(), lock_widget->GetNativeView()); lock_modal_widget->Show(); + EXPECT_TRUE(lock_modal_widget->GetNativeView()->HasFocus()); // It should be in LockScreen modal container. aura::Window* lock_modal_container = Shell::GetContainer( @@ -250,6 +264,9 @@ TEST_F(ShellTest, CreateLockScreenModalWindow) { views::Widget* modal_widget = views::Widget::CreateWindowWithParent( new ModalWindow(), widget->GetNativeView()); modal_widget->Show(); + // Window on lock screen shouldn't lost focus. + EXPECT_FALSE(modal_widget->GetNativeView()->HasFocus()); + EXPECT_TRUE(lock_modal_widget->GetNativeView()->HasFocus()); // It should be in non-LockScreen modal container. aura::Window* modal_container = Shell::GetContainer( @@ -257,6 +274,16 @@ TEST_F(ShellTest, CreateLockScreenModalWindow) { ash::internal::kShellWindowId_SystemModalContainer); EXPECT_EQ(modal_container, modal_widget->GetNativeWindow()->parent()); + // Modal dialog without parent, caused crash see crbug.com/226141 + views::Widget* modal_dialog = views::DialogDelegate::CreateDialogWidget( + new TestModalDialogDelegate(), CurrentContext(), NULL); + + modal_dialog->Show(); + EXPECT_FALSE(modal_dialog->GetNativeView()->HasFocus()); + EXPECT_TRUE(lock_modal_widget->GetNativeView()->HasFocus()); + + modal_dialog->Close(); + modal_widget->Close(); modal_widget->Close(); lock_modal_widget->Close(); lock_widget->Close(); diff --git a/ash/wm/stacking_controller.cc b/ash/wm/stacking_controller.cc index 3802440b..7b3337f 100644 --- a/ash/wm/stacking_controller.cc +++ b/ash/wm/stacking_controller.cc @@ -119,22 +119,22 @@ aura::Window* StackingController::GetSystemModalContainer( // If screen lock is not active and user session is active, // all modal windows are placed into the normal modal container. - if (!Shell::GetInstance()->session_state_delegate()->IsScreenLocked() && - Shell::GetInstance()->session_state_delegate()-> - IsActiveUserSessionStarted()) { + // In case of missing transient parent (it could happen for alerts from + // background pages) assume that the window belongs to user session. + SessionStateDelegate* session_state_delegate = + Shell::GetInstance()->session_state_delegate(); + if ((!session_state_delegate->IsScreenLocked() && + session_state_delegate->IsActiveUserSessionStarted()) || + !window->transient_parent()) { return GetContainerById(root, internal::kShellWindowId_SystemModalContainer); } // Otherwise those that originate from LockScreen container and above are // placed in the screen lock modal container. - aura::Window* lock_container = - GetContainerById(root, internal::kShellWindowId_LockScreenContainer); - int lock_container_id = lock_container->id(); int window_container_id = window->transient_parent()->parent()->id(); - aura::Window* container = NULL; - if (window_container_id < lock_container_id) { + if (window_container_id < internal::kShellWindowId_LockScreenContainer) { container = GetContainerById( root, internal::kShellWindowId_SystemModalContainer); } else { diff --git a/chrome/browser/ui/views/frame/browser_view.cc b/chrome/browser/ui/views/frame/browser_view.cc index dccae8b..a150c15 100644 --- a/chrome/browser/ui/views/frame/browser_view.cc +++ b/chrome/browser/ui/views/frame/browser_view.cc @@ -1505,6 +1505,11 @@ bool BrowserView::CanActivate() const { if (!AppModalDialogQueue::GetInstance()->active_dialog()) return true; +#if defined(USE_AURA) && defined(OS_CHROMEOS) + // On Aura window manager controls all windows so settings focus via PostTask + // will make only worse because posted task will keep trying to steal focus. + AppModalDialogQueue::GetInstance()->ActivateModalDialog(); +#else // If another browser is app modal, flash and activate the modal browser. This // has to be done in a post task, otherwise if the user clicked on a window // that doesn't have the modal dialog the windows keep trying to get the focus @@ -1513,6 +1518,7 @@ bool BrowserView::CanActivate() const { FROM_HERE, base::Bind(&BrowserView::ActivateAppModalDialog, activate_modal_dialog_factory_.GetWeakPtr())); +#endif return false; } |