Attempt at making BrowserKeyboardAccessibility lets flaky. If an

accessibility event was received before the tab key was seen, then we
would prematurely quit the message loop and end up in
ui_controls::SendKeyPressNotifyWhenDone again, resulting in hitting
the DCHECK in ui_controls. Additionally if the test timed out we would
keep running other tests, resulting in hitting the DCHECK in ui_controls. I'm not sure which was happening, but either could have caused the DCHECK to get hit.

I'm going to leave the debugging code in for a few cycles.

I think we need a SendKeyPressAndBlock and callers ASSERT that it returns true. I'll look into that later though.

BUG=50864
TEST=this is only a test change

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

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

2 files changed, 104 insertions, 81 deletions

diff --git a/chrome/browser/views/browser_keyboard_accessibility_test_win.cc b/chrome/browser/views/browser_keyboard_accessibility_test_win.cc
index f5b942f..0e2f592 100644
--- a/chrome/browser/views/browser_keyboard_accessibility_test_win.cc
+++ b/chrome/browser/views/browser_keyboard_accessibility_test_win.cc
@@ -15,12 +15,63 @@
 
 namespace {
 
+// FocusWaiter is used to wait for focus on a NativeWindow. It is ViewsDelegate
+// to know when focus changes.
+class FocusWaiter : public ChromeViewsDelegate {
+ public:
+  // Wait for focus to be received on the window. Returns true if successful.
+  static bool WaitForFocus(gfx::NativeWindow window) {
+    FocusWaiter waiter(window);
+    ui_test_utils::RunMessageLoop();
+    // FocusWaiter exits the message loop when done.
+    return waiter.quit_message_loop_;
+  }
+
+ private:
+  FocusWaiter(gfx::NativeWindow window)
+      : window_(window),
+        last_delegate_(ViewsDelegate::views_delegate),
+        quit_message_loop_(false) {
+    ViewsDelegate::views_delegate = this;
+  }
+
+  ~FocusWaiter() {
+    DCHECK_EQ(this, ViewsDelegate::views_delegate);
+    ViewsDelegate::views_delegate = last_delegate_;
+  }
+
+  // Overidden from ChromeViewsDelegate.
+  virtual void NotifyAccessibilityEvent(views::View* view,
+                                        AccessibilityTypes::Event event_type) {
+    if (!quit_message_loop_ && view->GetWidget()->GetNativeView() == window_) {
+      quit_message_loop_ = true;
+      MessageLoop::current()->Quit();
+    }
+
+    // Notify the existing delegate so that it can maintain whatever state it
+    // needs to.
+    if (last_delegate_)
+      last_delegate_->NotifyAccessibilityEvent(view, event_type);
+  }
+
+ private:
+  // Window we're waiting for focus on.
+  gfx::NativeWindow window_;
+
+  // ViewsDelegate at the time we were created.
+  ViewsDelegate* last_delegate_;
+
+  // Have we quit the message loop yet?
+  bool quit_message_loop_;
+
+  DISALLOW_COPY_AND_ASSIGN(FocusWaiter);
+};
+
 class BrowserKeyboardAccessibility : public InProcessBrowserTest,
                                      public ChromeViewsDelegate {
  public:
   BrowserKeyboardAccessibility()
-      : is_waiting_(false),
-        current_view_(NULL),
+      : current_view_(NULL),
         current_event_type_(AccessibilityTypes::EVENT_FOCUS) {
     // Set ourselves as the currently active ViewsDelegate.
     ViewsDelegate::views_delegate = this;
@@ -32,22 +83,13 @@ class BrowserKeyboardAccessibility : public InProcessBrowserTest,
   // Save the last notification sent by views::View::NotifyAccessibilityEvent.
   virtual void NotifyAccessibilityEvent(
       views::View* view, AccessibilityTypes::Event event_type) {
-    current_view_  = view;
+    current_view_ = view;
     current_event_type_ = event_type;
-
-    // Are we within a message loop waiting for a particular event?
-    // TODO(phajdan.jr): remove logging after fixing http://crbug.com/50663.
-    LOG(ERROR) << "Got notification; is_waiting_ is "
-               << (is_waiting_ ? "true" : "false");
-    if (is_waiting_) {
-      is_waiting_ = false;
-      MessageLoop::current()->Quit();
-    }
   }
 
   // Helper that performs tabbing until it cycles back to the original focus.
-  void TabCyclerForwardAndBack(gfx::NativeWindow hwnd);
-  void TabCycler(gfx::NativeWindow hwnd, bool forward_tab);
+  void TabCyclerForwardAndBack();
+  void TabCycler(bool forward_tab);
 
   views::View* current_view() const { return current_view_; }
 
@@ -59,12 +101,7 @@ class BrowserKeyboardAccessibility : public InProcessBrowserTest,
     return current_event_type_;
   }
 
-  void set_waiting(bool value) { is_waiting_ = value; }
-
  private:
-  // Are we waiting for an event?
-  bool is_waiting_;
-
   // View of interest (i.e. for testing or one we are waiting to gain focus).
   views::View* current_view_;
 
@@ -72,49 +109,45 @@ class BrowserKeyboardAccessibility : public InProcessBrowserTest,
   AccessibilityTypes::Event current_event_type_;
 };
 
-// This test is disabled, see bug 50864.
-IN_PROC_BROWSER_TEST_F(BrowserKeyboardAccessibility,
-                       DISABLED_TabInAboutChromeDialog) {
+IN_PROC_BROWSER_TEST_F(BrowserKeyboardAccessibility, TabInAboutChromeDialog) {
   views::Window* about_chrome_window =
       BrowserView::GetBrowserViewForNativeWindow(
           browser()->window()->GetNativeHandle())->ShowAboutChromeDialog();
 
-  TabCyclerForwardAndBack(about_chrome_window->GetNativeWindow());
+  if (current_view_native_window() != about_chrome_window->GetNativeWindow()) {
+    // Wait for 'about' to get focus.
+    ASSERT_TRUE(
+        FocusWaiter::WaitForFocus(about_chrome_window->GetNativeWindow()));
+  }
+  TabCyclerForwardAndBack();
 }
 
-// This test is disabled, see bug 50864.
 IN_PROC_BROWSER_TEST_F(BrowserKeyboardAccessibility,
-                       DISABLED_TabInClearBrowsingDataDialog) {
+                       TabInClearBrowsingDataDialog) {
   browser()->OpenClearBrowsingDataDialog();
-  TabCyclerForwardAndBack(current_view_native_window());
+  TabCyclerForwardAndBack();
 }
 
-// This test is disabled, see bug 50864.
 IN_PROC_BROWSER_TEST_F(BrowserKeyboardAccessibility,
-                       DISABLED_TabInImportSettingsDialog) {
+                       TabInImportSettingsDialog) {
   browser()->OpenImportSettingsDialog();
-  TabCyclerForwardAndBack(current_view_native_window());
+  TabCyclerForwardAndBack();
 }
 
-// This test is disabled, see bug 50864.
-IN_PROC_BROWSER_TEST_F(BrowserKeyboardAccessibility,
-                       DISABLED_TabInKeywordEditor) {
+IN_PROC_BROWSER_TEST_F(BrowserKeyboardAccessibility, TabInKeywordEditor) {
   browser()->OpenKeywordEditor();
-  TabCyclerForwardAndBack(current_view_native_window());
+  TabCyclerForwardAndBack();
 }
 
-// This test is disabled, see bug 50864.
-IN_PROC_BROWSER_TEST_F(BrowserKeyboardAccessibility,
-                       DISABLED_TabInOptionsDialog) {
+IN_PROC_BROWSER_TEST_F(BrowserKeyboardAccessibility, TabInOptionsDialog) {
   browser()->OpenOptionsDialog();
 
   // Tab through each of the three tabs.
-  for (int i = 0; i < 3; ++i) {
+  for (int i = 0; i < 3 && !HasFatalFailure(); ++i) {
     // TODO(phajdan.jr): remove logging after fixing http://crbug.com/50663.
     LOG(ERROR) << "Iteration no. " << i;
 
-    TabCyclerForwardAndBack(current_view_native_window());
-
+    TabCyclerForwardAndBack();
 
     // TODO(phajdan.jr): remove logging after fixing http://crbug.com/50663.
     LOG(ERROR) << "Sending TAB key event...";
@@ -122,87 +155,76 @@ IN_PROC_BROWSER_TEST_F(BrowserKeyboardAccessibility,
                                             base::VKEY_TAB,
                                             true, false, false, false,
                                             new MessageLoop::QuitTask());
-    set_waiting(true);
     ui_test_utils::RunMessageLoop();
   }
 }
 
-// This test is disabled, see bug 50864.
-IN_PROC_BROWSER_TEST_F(BrowserKeyboardAccessibility,
-                       DISABLED_TabInPasswordManager) {
+IN_PROC_BROWSER_TEST_F(BrowserKeyboardAccessibility, TabInPasswordManager) {
   browser()->OpenPasswordManager();
-  TabCyclerForwardAndBack(current_view_native_window());
+  TabCyclerForwardAndBack();
 }
 
 // TODO(dtseng): http://www.crbug.com/50402
 IN_PROC_BROWSER_TEST_F(BrowserKeyboardAccessibility,
                        FAILS_TabInSyncMyBookmarksDialog) {
   browser()->OpenSyncMyBookmarksDialog();
-  TabCyclerForwardAndBack(current_view_native_window());
+  TabCyclerForwardAndBack();
 }
 
-// This test is disabled, see bug 50864.
-IN_PROC_BROWSER_TEST_F(BrowserKeyboardAccessibility,
-                       DISABLED_TabInTaskManager) {
+IN_PROC_BROWSER_TEST_F(BrowserKeyboardAccessibility, TabInTaskManager) {
   browser()->OpenTaskManager();
-  TabCyclerForwardAndBack(current_view_native_window());
+  TabCyclerForwardAndBack();
 }
 
-// This test is disabled, see bug 50864.
-IN_PROC_BROWSER_TEST_F(BrowserKeyboardAccessibility,
-                       DISABLED_TabInToolbar) {
+IN_PROC_BROWSER_TEST_F(BrowserKeyboardAccessibility, TabInToolbar) {
   gfx::NativeWindow native_window = browser()->window()->GetNativeHandle();
   ui_controls::SendKeyPressNotifyWhenDone(native_window,
                                           base::VKEY_T,
                                           false, true, true, false,
                                           new MessageLoop::QuitTask());
-  set_waiting(true);
   ui_test_utils::RunMessageLoop();
-  TabCyclerForwardAndBack(current_view_native_window());
+
+  // TODO: this needs to assert the toolbar got focus.
+
+  TabCyclerForwardAndBack();
 }
 
-// This test is disabled, see bug 50864.
-IN_PROC_BROWSER_TEST_F(BrowserKeyboardAccessibility,
-                       DISABLED_TabInUpdateChromeDialog) {
+IN_PROC_BROWSER_TEST_F(BrowserKeyboardAccessibility, TabInUpdateChromeDialog) {
   browser()->OpenUpdateChromeDialog();
-  TabCyclerForwardAndBack(current_view_native_window());
+  TabCyclerForwardAndBack();
 }
 
-void BrowserKeyboardAccessibility::TabCyclerForwardAndBack(
-    gfx::NativeWindow hwnd) {
-  TabCycler(hwnd, true);
-  TabCycler(hwnd, false);
+void BrowserKeyboardAccessibility::TabCyclerForwardAndBack() {
+  if (HasFatalFailure())
+    return;
+
+  TabCycler(true);
+
+  if (HasFatalFailure())
+    return;
+
+  TabCycler(false);
 }
 
-void BrowserKeyboardAccessibility::TabCycler(gfx::NativeWindow hwnd,
-                                             bool forward_tab) {
-  // Wait for a focus event on the provided native window.
-  while (current_event() != AccessibilityTypes::EVENT_FOCUS ||
-         current_view_native_window() != hwnd) {
-    // TODO(phajdan.jr): remove logging after fixing http://crbug.com/50663.
-    LOG(ERROR) << "Runnig message loop...";
-    set_waiting(true);
-    ui_test_utils::RunMessageLoop();
-  }
-  // TODO(phajdan.jr): remove logging after fixing http://crbug.com/50663.
-  LOG(ERROR) << "After the loop.";
+void BrowserKeyboardAccessibility::TabCycler(bool forward_tab) {
+  gfx::NativeWindow hwnd = current_view_native_window();
+  ASSERT_TRUE(hwnd);
 
   views::View* first_focused_item = current_view();
 
   ASSERT_TRUE(first_focused_item != NULL);
 
-  views::View* next_focused_item = first_focused_item;
-
   // Keep tabbing until we reach the originally focused view.
   do {
     // TODO(phajdan.jr): remove logging after fixing http://crbug.com/50663.
     LOG(ERROR) << "Sending TAB key event.";
     ui_controls::SendKeyPressNotifyWhenDone(hwnd, base::VKEY_TAB,
         false, !forward_tab, false, false, new MessageLoop::QuitTask());
-    set_waiting(true);
+
+    current_view_ = NULL;
+
     ui_test_utils::RunMessageLoop();
-    next_focused_item = current_view();
-  } while (first_focused_item != next_focused_item);
+  } while (first_focused_item != current_view() && !HasFatalFailure());
   // TODO(phajdan.jr): remove logging after fixing http://crbug.com/50663.
   LOG(ERROR) << "After second loop.";
 }
diff --git a/chrome/test/in_process_browser_test.cc b/chrome/test/in_process_browser_test.cc
index 5d7e2be..39d9681 100644
--- a/chrome/test/in_process_browser_test.cc
+++ b/chrome/test/in_process_browser_test.cc
@@ -334,9 +334,10 @@ void InProcessBrowserTest::TimedOut() {
   error_message += base::IntToString(initial_timeout_);
   error_message += " ms (kInitialTimeoutInMS).";
 
-  GTEST_NONFATAL_FAILURE_(error_message.c_str());
-
   MessageLoopForUI::current()->Quit();
+
+  // WARNING: This must be after Quit as it returns.
+  FAIL() << error_message;
 }
 
 void InProcessBrowserTest::SetInitialTimeoutInMS(int timeout_value) {