Do not give instant focus if a view's toplevelwidget is not active

chrome/browser/ui/views/toolbar/toolbar_view_browsertest.cc

 // Copyright 2013 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.
 
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
 
 #include <stddef.h>
 
 #include "base/macros.h"
+#include "base/run_loop.h"
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/bookmarks/bookmark_model_factory.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_command_controller.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/view_ids.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/test/base/in_process_browser_test.h"
@@ skipping 18 matching lines @@
 };
 
 void ToolbarViewTest::RunToolbarCycleFocusTest(Browser* browser) {
   gfx::NativeWindow window = browser->window()->GetNativeWindow();
   views::Widget* widget = views::Widget::GetWidgetForNativeWindow(window);
   views::FocusManager* focus_manager = widget->GetFocusManager();
   CommandUpdater* updater = browser->command_controller()->command_updater();
 
   // Send focus to the toolbar as if the user pressed Alt+Shift+T.
   updater->ExecuteCommand(IDC_FOCUS_TOOLBAR);
+  base::RunLoop().RunUntilIdle();
 
   views::View* first_view = focus_manager->GetFocusedView();
   std::vector<int> ids;
 
   // Press Tab to cycle through all of the controls in the toolbar until
   // we end up back where we started.
   bool found_reload = false;
   bool found_location_bar = false;
   bool found_app_menu = false;
   const views::View* view = NULL;
@@ skipping 35 matching lines @@
   for (size_t i = 0; i < count - 1; i++)
     EXPECT_EQ(ids[i], reverse_ids[count - 2 - i]);
 }
 
 // The test is flaky on Win (http://crbug.com/152938) and crashes on CrOS under
 // AddressSanitizer (http://crbug.com/154657).
 IN_PROC_BROWSER_TEST_F(ToolbarViewTest, DISABLED_ToolbarCycleFocus) {
   RunToolbarCycleFocusTest(browser());
 }
 
-#if defined(OS_WIN)
-// http://crbug.com/152938 Flaky on win.
-#define MAYBE_ToolbarCycleFocusWithBookmarkBar \
-    DISABLED_ToolbarCycleFocusWithBookmarkBar
-#else
-#define MAYBE_ToolbarCycleFocusWithBookmarkBar ToolbarCycleFocusWithBookmarkBar
-#endif
-IN_PROC_BROWSER_TEST_F(ToolbarViewTest,
-                       MAYBE_ToolbarCycleFocusWithBookmarkBar) {
+IN_PROC_BROWSER_TEST_F(ToolbarViewTest, ToolbarCycleFocusWithBookmarkBar) {

[sky, 2016/10/10 22:47:21]

It seems as though this test relies on focus. If that's the case, then it
shouldn't in browser_tests, rather interactive_ui_tests. That's likely why it's
flaky. I don't think your patch helps it.

Also, you should document why the RunUntilIdle is necessary.

   CommandUpdater* updater = browser()->command_controller()->command_updater();
   updater->ExecuteCommand(IDC_SHOW_BOOKMARK_BAR);
 
   BookmarkModel* model =
       BookmarkModelFactory::GetForBrowserContext(browser()->profile());
   bookmarks::AddIfNotBookmarked(
       model, GURL("http://foo.com"), base::ASCIIToUTF16("Foo"));
 
   // We want to specifically test the case where the bookmark bar is
   // already showing when a window opens, so create a second browser
   // window with the same profile.
   Browser* second_browser = CreateBrowser(browser()->profile());
   RunToolbarCycleFocusTest(second_browser);
 }
 
 }  // namespace