Fix toolbar keyboard accessibility if bookmarks bar is shown.

chrome/browser/ui/views/toolbar_view_browsertest.cc

 // 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.
 
 #include "chrome/browser/ui/views/toolbar_view.h"
 
+#include "base/utf_string_conversions.h"
 #include "chrome/app/chrome_command_ids.h"
+#include "chrome/browser/bookmarks/bookmark_model.h"
+#include "chrome/browser/bookmarks/bookmark_model_factory.h"
+#include "chrome/browser/bookmarks/bookmark_utils.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/browser_command_controller.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"
 #include "ui/views/focus/focus_manager.h"
 #include "ui/views/view.h"
 #include "ui/views/widget/widget.h"
 
 namespace {
 
 class ToolbarViewTest : public InProcessBrowserTest {
  public:
   ToolbarViewTest() {}
 
+  void RunToolbarCycleFocusTest();
+
  private:
   DISALLOW_COPY_AND_ASSIGN(ToolbarViewTest);
 };
 
-IN_PROC_BROWSER_TEST_F(ToolbarViewTest, ToolbarCycleFocus) {
+void ToolbarViewTest::RunToolbarCycleFocusTest() {
   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);
 
   views::View* first_view = focus_manager->GetFocusedView();
   std::vector<int> ids;
@@ skipping 36 matching lines @@
 
   // Assert that the views were focused in exactly the reverse order.
   // The sequences should be the same length, and the last element will
   // be the same, and the others are reverse.
   ASSERT_EQ(ids.size(), reverse_ids.size());
   size_t count = ids.size();
   for (size_t i = 0; i < count - 1; i++)
     EXPECT_EQ(ids[i], reverse_ids[count - 2 - i]);
 }
 
+IN_PROC_BROWSER_TEST_F(ToolbarViewTest, ToolbarCycleFocus) {
+  RunToolbarCycleFocusTest();
+}
+
+IN_PROC_BROWSER_TEST_F(ToolbarViewTest, PRE_ToolbarCycleFocusWithBookmarkBar) {

[sky, 2012/09/24 14:05:32]

Why does this need to be a separate test?

[dmazzoni, 2012/09/24 16:58:23]

See "span a restart" in in_process_browser_test.h if you're not familiar with
the magic "PRE_" prefix for a test.

http://code.google.com/p/chromium/source/search?q=%22span+a+restart%22&origq=...

This particular bug only reproduces if the bookmarks bar is visible when the
window opens, not if you open it later. It'd be possible to open the bookmarks
bar and then open a new browser window within the test, of course, but it'd be a
lot more boilerplate code.

+  // Specifically test for the case where the bookmark bar is open, with
+  // a focusable bookmark button, when the browser window opens. This
+  // "PRE" method runs first to show the bar and add the bookmark, then
+  // the test below runs with the same profile.
+
+  CommandUpdater* updater = browser()->command_controller()->command_updater();
+  updater->ExecuteCommand(IDC_SHOW_BOOKMARK_BAR);
+
+  BookmarkModel* model =
+      BookmarkModelFactory::GetForProfile(browser()->profile());
+  bookmark_utils::AddIfNotBookmarked(
+      model, GURL("http://foo.com"), ASCIIToUTF16("Foo"));
+}
+
+IN_PROC_BROWSER_TEST_F(ToolbarViewTest, ToolbarCycleFocusWithBookmarkBar) {
+  // See PRE_ToolbarCycleFocusWithBookmarkBar, above.
+  RunToolbarCycleFocusTest();
+}
+
 }  // namespace