A unit test that creates a situation that crashed (bug 130306)

ash/shell_unittest.cc

Index: ash/shell_unittest.cc
diff --git a/ash/shell_unittest.cc b/ash/shell_unittest.cc
index 3bf6cf396e6a73212c21799e4791a84c0256ed6c..10be8880106d6a7831edf5131b008b909fbb1e3e 100644
--- a/ash/shell_unittest.cc
+++ b/ash/shell_unittest.cc
@@ -7,6 +7,8 @@
 #include "ash/shell.h"
 #include "ash/shell_delegate.h"
 #include "ash/shell_window_ids.h"
+#include "ash/shell/shell_delegate_impl.h"
+#include "ash/shell/window_watcher.h"
 #include "ash/test/ash_test_base.h"
 #include "ash/wm/root_window_layout_manager.h"
 #include "ash/wm/shelf_layout_manager.h"
@@ -339,4 +341,32 @@ TEST_F(ShellTest2, DontCrashWhenWindowDeleted) {
   window_->Init(ui::LAYER_NOT_DRAWN);
 }
 
+#if defined(OS_LINUX) && defined(OS_CHROMEOS)

[sky, 2012/08/06 21:21:22]

Why is this specific to linux and chromeos?

[Harry McCleave, 2012/08/07 18:17:23]

Done.

+class WindowWatcherTest : public test::AshTestBase {

[sky, 2012/08/06 21:21:22]

This test is testing WindowWatcher and not so much shell. The test should be
there.

[Harry McCleave, 2012/08/07 18:17:23]

Done.

+public:

[sky, 2012/08/06 21:21:22]

nit: spacing is off here, 350 and 353. These should be indented one space.

[Harry McCleave, 2012/08/07 18:17:23]

Done.

+  WindowWatcherTest() {}
+  virtual ~WindowWatcherTest() {}
+
+protected:
+  scoped_ptr<ash::shell::WindowWatcher> window_watcher_;
+
+private:
+  DISALLOW_COPY_AND_ASSIGN(WindowWatcherTest);
+};
+// This test verifies that shell can be torn down without causing failures
+// bug http://code.google.com/p/chromium/issues/detail?id=130332
+TEST_F(WindowWatcherTest, ShellDeleteInstance) {
+  Shell::DeleteInstance();
+
+  shell::ShellDelegateImpl* delegate = new ash::shell::ShellDelegateImpl;
+  Shell::CreateInstance(delegate);
+
+  Shell::GetPrimaryRootWindow()->ShowRootWindow();
+  window_watcher_.reset(new ash::shell::WindowWatcher);
+
+  delegate->SetWatcher(window_watcher_.get());
+  Shell::GetPrimaryRootWindow()->Hide();
+  window_watcher_.reset();

[sky, 2012/08/06 21:21:22]

Don't you need to do a elegate->SetWatcher(NULL) too?

[Harry McCleave, 2012/08/07 18:17:23]

Done. Oddly in this case you don't as the desctructor/cleanup does not attempt
to destroy the local reference to the watcher, but I suppose it is good practice
so it has been added.

+}
+#endif
 }  // namespace ash