Fixes crash in ShelfLayoutManager that would occur when closing window

with a full screen window. The problem was we deleted the launcher
and left other objects around so that if one of those other objects
needed the launcher we would crash.

BUG=110205
TEST=covered by unit tests.
R=ben@chromium.org


Review URL: http://codereview.chromium.org/9133017

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@117906 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 35 insertions, 6 deletions

diff --git a/ash/shell.cc b/ash/shell.cc
index 6627f8e..c2a4130 100644
--- a/ash/shell.cc
+++ b/ash/shell.cc
@@ -159,6 +159,17 @@ Shell::~Shell() {
   RemoveRootWindowEventFilter(tooltip_controller_.get());
   aura::client::SetTooltipClient(NULL);
 
+  // The LayoutManagers for the default and status containers talk to
+  // ShelfLayoutManager (LayoutManager installed on the launcher container).
+  // ShelfLayoutManager has a reference to the launcher widget. To avoid any of
+  // these trying to reference launcher after it's deleted we delete them all,
+  // then the launcher.
+  if (!CommandLine::ForCurrentProcess()->
+      HasSwitch(switches::kAuraWorkspaceManager)) {
+    ResetLayoutManager(internal::kShellWindowId_DefaultContainer);
+  }
+  ResetLayoutManager(internal::kShellWindowId_StatusContainer);
+  ResetLayoutManager(internal::kShellWindowId_LauncherContainer);
   // Make sure we delete WorkspaceController before launcher is
   // deleted as it has a reference to launcher model.
   workspace_controller_.reset();
@@ -413,4 +424,8 @@ void Shell::EnableWorkspaceManager() {
           workspace_controller_->workspace_manager()));
 }
 
+void Shell::ResetLayoutManager(int container_id) {
+  GetContainer(container_id)->SetLayoutManager(NULL);
+}
+
 }  // namespace ash
diff --git a/ash/shell.h b/ash/shell.h
index 7bb5d0b..e9251ed 100644
--- a/ash/shell.h
+++ b/ash/shell.h
@@ -147,6 +147,10 @@ class ASH_EXPORT Shell {
   // Enables WorkspaceManager.
   void EnableWorkspaceManager();
 
+  // Sets the LayoutManager of the container with the specified id to NULL. This
+  // has the effect of deleting the current LayoutManager.
+  void ResetLayoutManager(int container_id);
+
   static Shell* instance_;
 
   std::vector<WindowAndBoundsPair> to_restore_;
diff --git a/ash/wm/shelf_layout_manager.cc b/ash/wm/shelf_layout_manager.cc
index 87b9d6c..fd55b1b 100644
--- a/ash/wm/shelf_layout_manager.cc
+++ b/ash/wm/shelf_layout_manager.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -42,8 +42,7 @@ ShelfLayoutManager::ShelfLayoutManager(views::Widget* launcher,
 }
 
 ShelfLayoutManager::~ShelfLayoutManager() {
-  // Do not try to remove observer from layer as the Launcher is
-  // already deleted.
+  GetLayer(launcher_)->GetAnimator()->RemoveObserver(this);
 }
 
 void ShelfLayoutManager::LayoutShelf() {
diff --git a/ash/wm/shelf_layout_manager.h b/ash/wm/shelf_layout_manager.h
index 0bdf8c5..3c35389 100644
--- a/ash/wm/shelf_layout_manager.h
+++ b/ash/wm/shelf_layout_manager.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -28,7 +28,7 @@ namespace internal {
 // To respond to bounds changes in the status area StatusAreaLayoutManager works
 // closely with ShelfLayoutManager.
 class ASH_EXPORT ShelfLayoutManager : public aura::LayoutManager,
-                                             public ui::LayerAnimationObserver {
+                                      public ui::LayerAnimationObserver {
  public:
   ShelfLayoutManager(views::Widget* launcher, views::Widget* status);
   virtual ~ShelfLayoutManager();
diff --git a/ash/wm/shelf_layout_manager_unittest.cc b/ash/wm/shelf_layout_manager_unittest.cc
index d4e80c6..b09e2f1 100644
--- a/ash/wm/shelf_layout_manager_unittest.cc
+++ b/ash/wm/shelf_layout_manager_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -130,5 +130,16 @@ TEST_F(ShelfLayoutManagerTest, LauncherUpdatedWhenStatusAreaChangesSize) {
   EXPECT_EQ(200, launcher->GetStatusWidth());
 }
 
+// Verifies when the shell is deleted with a full screen window we don't
+// crash. This test is here as originally the crash was in ShelfLayoutManager.
+TEST_F(ShelfLayoutManagerTest, DontReferenceLauncherAfterDeletion) {
+  views::Widget* widget = new views::Widget;
+  views::Widget::InitParams params(views::Widget::InitParams::TYPE_WINDOW);
+  params.bounds = gfx::Rect(0, 0, 200, 200);
+  // Widget is now owned by the parent window.
+  widget->Init(params);
+  widget->SetFullscreen(true);
+}
+
 }  // namespace internal
 }  // namespace ash