Reorder a couple calls in TabDragController::Attach()

In order to set proper bounds for active tab which is being dragged,
TabStrin::StartedDraggingTabs() should be called after ResetSelection().

Bug: 836004
Change-Id: I7243bf75b1d3243e55892386f2a5debc092bf925
Reviewed-on: https://chromium-review.googlesource.com/1027590
Commit-Queue: Sang Woo Ko <sangwoo108@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555295}

chrome/browser/ui/views/tabs/tab_drag_controller.cc

@@ -931,10 +931,12 @@ void TabDragController::Attach(TabStrip* attached_tabstrip,
   for (size_t i = 0; i < drag_data_.size(); ++i)
     drag_data_[i].attached_tab = tabs[i];
 
-  attached_tabstrip_->StartedDraggingTabs(tabs);
-
   ResetSelection(GetModel(attached_tabstrip_));
 
+  // This should be called after ResetSelection() in order to generate
+  // bounds correctly. http://crbug.com/836004
+  attached_tabstrip_->StartedDraggingTabs(tabs);
+
   // The size of the dragged tab may have changed. Adjust the x offset so that
   // ratio of mouse_offset_ to original width is maintained.
   std::vector<Tab*> tabs_to_source(tabs);

chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc

@@ -216,6 +216,11 @@ void TabDragControllerTest::SetWindowFinderForTabStrip(
   tab_strip->drag_controller_->window_finder_ = std::move(window_finder);
 }
 
+void TabDragControllerTest::HandleGestureEvent(TabStrip* tab_strip,
+                                               ui::GestureEvent* event) {
+  tab_strip->OnGestureEvent(event);
+}
+
 void TabDragControllerTest::SetUp() {
 #if defined(USE_AURA)
   // This needs to be disabled as it can interfere with when events are
@@ -380,7 +385,7 @@ IN_PROC_BROWSER_TEST_F(TabDragControllerTest, GestureEndShouldEndDragTest) {
   ui::GestureEvent gesture_end(tab_1_center.x(), tab_1_center.x(), 0,
                                base::TimeTicks(),
                                ui::GestureEventDetails(ui::ET_GESTURE_END));
-  tab_strip->OnGestureEvent(&gesture_end);
+  HandleGestureEvent(tab_strip, &gesture_end);
   EXPECT_FALSE(TabDragController::IsActive());
   EXPECT_FALSE(tab_strip->IsDragSessionActive());
 }

chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.h

@@ -20,6 +20,10 @@ namespace content {
 class WebContents;
 }
 
+namespace ui {
+class GestureEvent;
+}
+
 // TabDragControllerTest is the basis for the two tests that exercise
 // TabDragController.
 class TabDragControllerTest : public InProcessBrowserTest {
@@ -44,6 +48,8 @@ class TabDragControllerTest : public InProcessBrowserTest {
   const BrowserList* browser_list;
 
  protected:
+  void HandleGestureEvent(TabStrip* tab_strip, ui::GestureEvent* event);
+
   // InProcessBrowserTest:
   void SetUp() override;
   void SetUpCommandLine(base::CommandLine* command_line) override;

chrome/browser/ui/views/tabs/tab_strip.h

@@ -296,12 +296,6 @@ class TabStrip : public views::View,
   friend class TabDragController;
   friend class TabDragControllerTest;
   friend class TabStripTest;
-  FRIEND_TEST_ALL_PREFIXES(TabDragControllerTest, GestureEndShouldEndDragTest);
-  FRIEND_TEST_ALL_PREFIXES(TabStripTest, TabForEventWhenStacked);
-  FRIEND_TEST_ALL_PREFIXES(TabStripTest, TabCloseButtonVisibilityWhenStacked);
-  FRIEND_TEST_ALL_PREFIXES(TabStripTest,
-                           TabCloseButtonVisibilityWhenNotStacked);
-  FRIEND_TEST_ALL_PREFIXES(TabStripTest, ActiveTabWidthWhenTabsAreTiny);
 
   // Used during a drop session of a url. Tracks the position of the drop as
   // well as a window used to highlight where the drop occurs.

chrome/browser/ui/views/tabs/tab_strip_unittest.cc

@@ -147,8 +147,22 @@ class TabStripTest : public ChromeViewsTestBase,
     return tab->HitTestPoint(point_in_tab_coords);
   }
 
+  Tab* FindTabForEvent(const gfx::Point& point) {
+    return tab_strip_->FindTabForEvent(point);
+  }
+
   void DoLayout() { tab_strip_->DoLayout(); }
 
+  void AnimateToIdealBounds() { tab_strip_->AnimateToIdealBounds(); }
+
+  int current_inactive_width() const {
+    return tab_strip_->current_inactive_width_;
+  }
+
+  const StackedTabStripLayout* touch_layout() const {
+    return tab_strip_->touch_layout_.get();
+  }
+
   // Owned by TabStrip.
   FakeBaseTabStripController* controller_ = nullptr;
   // Owns |tab_strip_|.
@@ -307,7 +321,7 @@ TEST_P(TabStripTest, TabForEventWhenStacked) {
 
   // Switch to stacked layout mode and force a layout to ensure tabs stack.
   tab_strip_->SetStackedLayout(true);
-  tab_strip_->DoLayout();
+  DoLayout();
 
   gfx::Point p;
   for (int y : {0, tab_strip_->height() / 2, tab_strip_->height() - 1}) {
@@ -315,7 +329,7 @@ TEST_P(TabStripTest, TabForEventWhenStacked) {
     int previous_tab = -1;
     for (int x = 0; x < tab_strip_->width(); ++x) {
       p.set_x(x);
-      int tab = tab_strip_->GetModelIndexOfTab(tab_strip_->FindTabForEvent(p));
+      int tab = tab_strip_->GetModelIndexOfTab(FindTabForEvent(p));
       if (tab == previous_tab)
         continue;
       if ((tab != -1) || (previous_tab != tab_strip_->tab_count() - 1))
@@ -351,9 +365,9 @@ TEST_P(TabStripTest, TabCloseButtonVisibilityWhenStacked) {
   EXPECT_TRUE(tab2->showing_close_button_);
 
   // Enter stacked layout mode and verify this sets |touch_layout_|.
-  ASSERT_FALSE(tab_strip_->touch_layout_.get());
+  ASSERT_FALSE(touch_layout());
   tab_strip_->SetStackedLayout(true);
-  ASSERT_TRUE(tab_strip_->touch_layout_.get());
+  ASSERT_TRUE(touch_layout());
 
   // Only the close button of the active tab should be visible in stacked
   // layout mode.
@@ -392,7 +406,7 @@ TEST_P(TabStripTest, TabCloseButtonVisibilityWhenStacked) {
 
   // All tab close buttons should be shown when disengaging stacked tab mode.
   tab_strip_->SetStackedLayout(false);
-  ASSERT_FALSE(tab_strip_->touch_layout_.get());
+  ASSERT_FALSE(touch_layout());
   EXPECT_TRUE(tab0->showing_close_button_);
   EXPECT_TRUE(tab2->showing_close_button_);
   EXPECT_TRUE(tab3->showing_close_button_);
@@ -424,7 +438,7 @@ TEST_P(TabStripTest, TabCloseButtonVisibilityWhenNotStacked) {
   ASSERT_FALSE(tab2->IsActive());
 
   // Ensure this is not in stacked layout mode.
-  ASSERT_FALSE(tab_strip_->touch_layout_.get());
+  ASSERT_FALSE(touch_layout());
 
   // Ensure that all tab close buttons are initially visible.
   EXPECT_TRUE(tab0->showing_close_button_);
@@ -476,7 +490,7 @@ TEST_P(TabStripTest, TabCloseButtonVisibilityWhenNotStacked) {
   tab_strip_->CloseTab(tab2, CLOSE_TAB_FROM_TOUCH);
   tab2 = nullptr;
   ASSERT_TRUE(tab3->IsActive());
-  tab_strip_->DoLayout();
+  DoLayout();
   EXPECT_FALSE(tab0->showing_close_button_);
   EXPECT_FALSE(tab1->showing_close_button_);
   EXPECT_TRUE(tab3->showing_close_button_);
@@ -657,9 +671,9 @@ TEST_P(TabStripTest, ActiveTabWidthWhenTabsAreTiny) {
   tab_strip_->GetWidget()->Show();
   tab_strip_->SetBounds(0, 0, 200, 20);
 
-  const int min_inactive_width = Tab::GetMinimumInactiveSize().width();
   // Create a lot of tabs in order to make inactive tabs tiny.
-  while (tab_strip_->current_inactive_width_ != min_inactive_width)
+  const int min_inactive_width = Tab::GetMinimumInactiveSize().width();
+  while (current_inactive_width() != min_inactive_width)
     controller_->CreateNewTab();
 
   const int min_active_width = Tab::GetMinimumActiveSize().width();