Fix the TabScrubber always starting from the first tab in full screen mode.

chrome/browser/ui/views/ash/tab_scrubber_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/ash/tab_scrubber.h"
 
 #include <memory>
 
 #include "ash/display/event_transformation_handler.h"
 #include "ash/shell.h"
+#include "base/callback_helpers.h"
 #include "base/command_line.h"
 #include "base/macros.h"
 #include "base/run_loop.h"
 #include "build/build_config.h"
+#include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/tabs/tab_strip_model_observer.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/tabs/tab.h"
 #include "chrome/browser/ui/views/tabs/tab_strip.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/common/url_constants.h"
 #include "content/public/test/test_utils.h"
 #include "ui/aura/window.h"
 #include "ui/events/event_utils.h"
 #include "ui/events/test/event_generator.h"
 
 #if defined(OS_CHROMEOS)
 #include "chromeos/chromeos_switches.h"
 #endif
 
 namespace {
 
+// Waits until the immersive mode reveal ends, and therefore the top view of
+// the browser is no longer visible.
+class ImmersiveRevealEndedWaiter : public ImmersiveModeController::Observer {
+ public:
+  explicit ImmersiveRevealEndedWaiter(
+      ImmersiveModeController* immersive_controller)
+      : immersive_controller_(immersive_controller) {
+    immersive_controller_->AddObserver(this);
+  }
+
+  ~ImmersiveRevealEndedWaiter() override {
+    if (immersive_controller_)
+      immersive_controller_->RemoveObserver(this);
+  }
+
+  void Wait() {
+    if (!immersive_controller_ || !immersive_controller_->IsRevealed())
+      return;
+
+    base::RunLoop run_loop;
+    quit_closure_ = run_loop.QuitClosure();
+    run_loop.Run();
+  }
+
+ private:
+  void MaybeQuitRunLoop() {
+    if (!quit_closure_.is_null())
+      base::ResetAndReturn(&quit_closure_).Run();
+  }
+
+  // ImmersiveModeController::Observer:
+  void OnImmersiveRevealEnded() override { MaybeQuitRunLoop(); }
+
+  void OnImmersiveModeControllerDestroyed() override {
+    MaybeQuitRunLoop();
+    immersive_controller_ = nullptr;

[sky, 2017/02/02 22:29:45]

RemoveObserver?

[afakhry, 2017/02/06 18:34:33]

Done, but do we really need it? At this point we are already in the destructor
of the ImmersiveModeController looping through the observers.
https://cs.chromium.org/chromium/src/chrome/browser/ui/views/frame/immersive_...

[sky, 2017/02/06 22:14:03]

It's nice to have ObserverLists check if there are no lingering observers. But I
realize it doesn't matter here because the observerlist hasn't been configured
to DCHECK in this case.

+  }
+
+  ImmersiveModeController* immersive_controller_;
+  base::Closure quit_closure_;
+
+  DISALLOW_COPY_AND_ASSIGN(ImmersiveRevealEndedWaiter);
+};
+
 class TabScrubberTest : public InProcessBrowserTest,
                         public TabStripModelObserver {
  public:
   TabScrubberTest()
       : target_index_(-1) {
   }
 
   void SetUpCommandLine(base::CommandLine* command_line) override {
 #if defined(OS_CHROMEOS)
     command_line->AppendSwitch(chromeos::switches::kNaturalScrollDefault);
@@ skipping 228 matching lines @@
   Browser* browser2 = CreateBrowser(browser()->profile());
   browser2->window()->Activate();
   ASSERT_TRUE(browser2->window()->IsActive());
   ASSERT_FALSE(browser()->window()->IsActive());
   AddTabs(browser2, 1);
 
   Scrub(browser2, 0, EACH_TAB);
   EXPECT_EQ(0, browser2->tab_strip_model()->active_index());
 }
 
+// Tests that tab scrubbing works correctly for a full-screen browser.
+IN_PROC_BROWSER_TEST_F(TabScrubberTest, FullScreenBrowser) {
+  AddTabs(browser(), 6);
+  base::RunLoop().RunUntilIdle();

[sky, 2017/02/02 22:29:45]

Why do you need the RunUntilIdle here?

[afakhry, 2017/02/06 18:34:33]

Removed. I don't need it. This was a leftover from when I was trying to get this
test to work properly.

+  browser()->tab_strip_model()->ActivateTabAt(4, false);
+
+  chrome::ToggleFullscreenMode(browser());
+  BrowserView* browser_view = BrowserView::GetBrowserViewForNativeWindow(
+      browser()->window()->GetNativeWindow());
+  ImmersiveModeController* immersive_controller =
+      browser_view->immersive_mode_controller();
+  EXPECT_TRUE(immersive_controller->IsEnabled());
+
+  ImmersiveRevealEndedWaiter waiter(immersive_controller);
+  waiter.Wait();
+
+  EXPECT_FALSE(immersive_controller->IsRevealed());
+
+  EXPECT_EQ(4, browser()->tab_strip_model()->active_index());
+  Scrub(browser(), 0, EACH_TAB);
+  EXPECT_EQ(0, browser()->tab_strip_model()->active_index());
+  EXPECT_EQ(4U, activation_order_.size());
+  EXPECT_EQ(3, activation_order_[0]);
+  EXPECT_EQ(2, activation_order_[1]);
+  EXPECT_EQ(1, activation_order_[2]);
+  EXPECT_EQ(0, activation_order_[3]);
+}
+
 // Swipe 4 tabs in each direction with an extra swipe within each. The same
 // 4 tabs should become active.
 IN_PROC_BROWSER_TEST_F(TabScrubberTest, Repeated) {
   AddTabs(browser(), 4);
 
   Scrub(browser(), 0, REPEAT_TABS);
   ASSERT_EQ(4U, activation_order_.size());
   EXPECT_EQ(3, activation_order_[0]);
   EXPECT_EQ(2, activation_order_[1]);
   EXPECT_EQ(1, activation_order_[2]);
@@ skipping 174 matching lines @@
 
   SendScrubEvent(browser(), 1);
   EXPECT_TRUE(TabScrubber::GetInstance()->IsActivationPending());
   browser()->tab_strip_model()->ToggleSelectionAt(0);
   browser()->tab_strip_model()->ToggleSelectionAt(2);
   browser()->tab_strip_model()->MoveSelectedTabsTo(2);
   EXPECT_EQ(0, TabScrubber::GetInstance()->highlighted_tab());
 }
 
 #endif  // defined(OS_CHROMEOS)