Speculative fix for sticky keys overlay crash.

ash/sticky_keys/sticky_keys_overlay_unittest.cc

Index: ash/sticky_keys/sticky_keys_overlay_unittest.cc
diff --git a/ash/sticky_keys/sticky_keys_overlay_unittest.cc b/ash/sticky_keys/sticky_keys_overlay_unittest.cc
index b6078255bb72b2d8ed6a4efe022721518e5430e2..ed48f1a7e57325a68dbf73f1986b9e934203fcc7 100644
--- a/ash/sticky_keys/sticky_keys_overlay_unittest.cc
+++ b/ash/sticky_keys/sticky_keys_overlay_unittest.cc
@@ -4,6 +4,7 @@
 
 #include "ash/sticky_keys/sticky_keys_overlay.h"
 
+#include "ash/display/display_manager.h"
 #include "ash/shell.h"
 #include "ash/sticky_keys/sticky_keys_controller.h"
 #include "ash/test/ash_test_base.h"
@@ -11,11 +12,7 @@
 
 namespace ash {
 
-class StickyKeysOverlayTest : public test::AshTestBase {
- public:
-  StickyKeysOverlayTest() {}
-  virtual ~StickyKeysOverlayTest() {}
-};
+using StickyKeysOverlayTest = test::AshTestBase;
 
 TEST_F(StickyKeysOverlayTest, OverlayVisibility) {
   StickyKeysOverlay overlay;
@@ -41,6 +38,31 @@ TEST_F(StickyKeysOverlayTest, ModifierKeyState) {
             overlay.GetModifierKeyState(ui::EF_COMMAND_DOWN));
 }
 
+// This test addresses the crash report at crbug.com/435600, speculated to be
+// caused by using sticky keys with multiple displays.
+TEST_F(StickyKeysOverlayTest, OverlayNotDestroyedAfterDisplayRemoved) {
+  // Add a secondary display to the left of the primary one.
+  UpdateDisplay("1280x1024, 1980x1080");

[James Cook, 2014/12/04 20:43:17]

super nit: No space after ,

[Tim Song, 2014/12/04 21:17:43]

Done.

+  DisplayManager* display_manager = Shell::GetInstance()->display_manager();
+  display_manager->SetLayoutForCurrentDisplays(
+      DisplayLayout(DisplayLayout::LEFT, 0));
+
+  // The overlay should belong to the secondary root window.

[James Cook, 2014/12/04 20:43:17]

Is there an assert or expect you could make here?

[Tim Song, 2014/12/04 21:17:43]

Done. I added some asserts to make sure the widget is in the expected root
window.

+  StickyKeysOverlay overlay;
+  overlay.SetModifierKeyState(ui::EF_SHIFT_DOWN, STICKY_KEY_STATE_DISABLED);
+
+  // Removing the second display should move the overlay to the primary root
+  // window.
+  UpdateDisplay("1280x1024");
+
+  overlay.SetModifierKeyState(ui::EF_SHIFT_DOWN, STICKY_KEY_STATE_ENABLED);
+  EXPECT_EQ(STICKY_KEY_STATE_ENABLED,

[James Cook, 2014/12/04 20:43:17]

Likewise I don't see how these assertions really test the reparenting. If your
sole goal with this test is "make sure it doesn't crash" then your comments
should make that explicit. Or you should have an EXPECT line that validates the
reparenting.

[Tim Song, 2014/12/04 21:17:43]

Done.

+            overlay.GetModifierKeyState(ui::EF_SHIFT_DOWN));
+  overlay.SetModifierKeyState(ui::EF_SHIFT_DOWN, STICKY_KEY_STATE_DISABLED);
+  EXPECT_EQ(STICKY_KEY_STATE_DISABLED,
+            overlay.GetModifierKeyState(ui::EF_SHIFT_DOWN));
+}
+
 // Additional sticky key overlay tests that depend on chromeos::EventRewriter
 // are now in chrome/browser/chromeos/events/event_rewriter_unittest.cc .