[Tab Groups] Pressing ESCAPE while in a drag will revert the tab group.

Bug: 905491
Change-Id: Ib9cd7e16a8d665fe6403a5b1b76ca872fe9d9383
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1804189
Commit-Queue: Charlene Yan <cyan@chromium.org>
Reviewed-by: Taylor Bergquist <tbergquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#696982}

chrome/browser/ui/tabs/tab_strip_model.cc

@@ -1105,6 +1105,41 @@ void TabStripModel::AddToGroupForRestore(const std::vector<int>& indices,
     AddToNewGroupImpl(indices, group);
 }
 
+void TabStripModel::UpdateGroupForDragRevert(int index,
+                                             base::Optional<TabGroupId> group) {
+  DCHECK(!reentrancy_guard_);
+  base::AutoReset<bool> resetter(&reentrancy_guard_, true);
+
+  // Ungroup tab before moving, so that if this is the last tab in the group
+  // observers can delete that group.
+  base::Optional<TabGroupId> old_group = UngroupTab(index);
+
+  if (group.has_value()) {
+    auto add_to_group_and_notify = [&]() {
+      contents_data_[index]->set_group(group.value());
+      group_data_.at(group.value()).TabAdded();
+      NotifyGroupChange(index, old_group, group);
+    };
+
+    const bool group_exists = base::Contains(group_data_, group.value());
+    if (group_exists) {
+      add_to_group_and_notify();
+    } else {
+      auto data_it =
+          group_data_.emplace(group.value(), GroupData(TabGroupVisualData()))
+              .first;
+      add_to_group_and_notify();
+      // Notify observers about the initial visual data.
+      for (auto& observer : observers_) {
+        observer.OnTabGroupVisualDataChanged(this, group.value(),
+                                             &data_it->second.visual_data());
+      }
+    }
+  }
+
+  NotifyGroupChange(index, old_group, group);
+}
+
 void TabStripModel::RemoveFromGroup(const std::vector<int>& indices) {
   DCHECK(!reentrancy_guard_);
   base::AutoReset<bool> resetter(&reentrancy_guard_, true);

chrome/browser/ui/tabs/tab_strip_model.h

@@ -436,8 +436,15 @@ class TabStripModel {
   // Similar to AddToExistingGroup(), but creates a group with id |group| if it
   // doesn't exist. This is only intended to be called from session restore
   // code.
+  // TODO(crbug.com/1004346): Update the group's old metadata (title and color).
   void AddToGroupForRestore(const std::vector<int>& indices, TabGroupId group);
 
+  // Updates the tab group of the tab at |index|. If |group| is nullopt, the tab
+  // will be removed from the current group. If |group| does not exist, it will
+  // create the group then add the tab to the group.
+  // TODO(crbug.com/1004346): Update the group's old metadata (title and color).
+  void UpdateGroupForDragRevert(int index, base::Optional<TabGroupId> group);
+
   // Removes the set of tabs pointed to by |indices| from the the groups they
   // are in, if any. The tabs are moved out of the group if necessary. |indices|
   // must be sorted in ascending order. This feature is in development and gated

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

@@ -633,10 +633,13 @@ void TabDragController::EndDrag(EndDragReason reason) {
 void TabDragController::InitTabDragData(Tab* tab,
                                         TabDragData* drag_data) {
   TRACE_EVENT0("views", "TabDragController::InitTabDragData");
-  drag_data->source_model_index = source_context_->GetIndexOf(tab);
+  const int source_model_index = source_context_->GetIndexOf(tab);
+  drag_data->source_model_index = source_model_index;
   drag_data->contents = source_context_->GetTabStripModel()->GetWebContentsAt(
       drag_data->source_model_index);
   drag_data->pinned = source_context_->IsTabPinned(tab);
+  drag_data->group_id = source_context_->GetTabStripModel()->GetTabGroupForTab(
+      source_model_index);
 }
 
 void TabDragController::OnWidgetBoundsChanged(views::Widget* widget,
@@ -1669,6 +1672,8 @@ void TabDragController::RevertDragAt(size_t drag_index) {
         data->source_model_index, std::move(data->owned_contents),
         (data->pinned ? TabStripModel::ADD_PINNED : 0));
   }
+  source_context_->GetTabStripModel()->UpdateGroupForDragRevert(
+      data->source_model_index, data->group_id);
 }
 
 void TabDragController::CompleteDrag() {

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

@@ -236,6 +236,9 @@ class TabDragController : public views::WidgetObserver {
     // Is the tab pinned?
     bool pinned;
 
+    // Stores the group the tab is in, or nullopt if tab is not grouped.
+    base::Optional<TabGroupId> group_id;
+
    private:
     DISALLOW_COPY_AND_ASSIGN(TabDragData);
   };

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

@@ -856,6 +856,73 @@ IN_PROC_BROWSER_TEST_P(DetachToBrowserTabDragControllerTest,
   EXPECT_THAT(model->ListTabsInGroup(group1), testing::ElementsAre(0, 1, 2, 3));
 }
 
+// Creates a browser with four tabs. The first two tabs are in Tab Group 1.
+// Dragging the third tab over one to the left will result in the tab joining
+// Tab Group 1. While this drag is still in session, pressing escape will revert
+// group of the tab to before the drag session started.
+IN_PROC_BROWSER_TEST_P(DetachToBrowserTabDragControllerTest,
+                       RevertDragSingleTabIntoGroup) {
+  scoped_feature_list_.InitAndEnableFeature(features::kTabGroups);
+
+  TabStrip* tab_strip = GetTabStripForBrowser(browser());
+  TabStripModel* model = browser()->tab_strip_model();
+
+  AddTabsAndResetBrowser(browser(), 3);
+  TabGroupId group1 = model->AddToNewGroup({0, 1});
+  StopAnimating(tab_strip);
+
+  // Dragging the tab in the second index to the tab in the first index switches
+  // the tabs and adds the dragged tab to the group.
+  ASSERT_TRUE(PressInput(GetCenterInScreenCoordinates(tab_strip->tab_at(2))));
+  ASSERT_TRUE(DragInputTo(GetCenterInScreenCoordinates(tab_strip->tab_at(1))));
+
+  EXPECT_EQ("0 2 1 3", IDString(model));
+  EXPECT_THAT(model->ListTabsInGroup(group1), testing::ElementsAre(0, 1, 2));
+
+  ASSERT_TRUE(TabDragController::IsActive());
+
+  // Pressing escape will revert the tabs to original state before the drag.
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(browser(), ui::VKEY_ESCAPE, false,
+                                              false, false, false));
+  EXPECT_EQ("0 1 2 3", IDString(model));
+  EXPECT_THAT(model->ListTabsInGroup(group1), testing::ElementsAre(0, 1));
+}
+
+// Creates a browser with four tabs. The last two tabs are in Tab Group 1. The
+// second tab is in Tab Group 2. Dragging the second tab over one to the right
+// will result in the tab joining Tab Group 1. While this drag is still in
+// session, pressing escape will revert group of the tab to before the drag
+// session started.
+IN_PROC_BROWSER_TEST_P(DetachToBrowserTabDragControllerTest,
+                       RevertDragSingleTabGroupIntoGroup) {
+  scoped_feature_list_.InitAndEnableFeature(features::kTabGroups);
+
+  TabStrip* tab_strip = GetTabStripForBrowser(browser());
+  TabStripModel* model = browser()->tab_strip_model();
+
+  AddTabsAndResetBrowser(browser(), 3);
+  TabGroupId group1 = model->AddToNewGroup({2, 3});
+  TabGroupId group2 = model->AddToNewGroup({1});
+  StopAnimating(tab_strip);
+
+  // Dragging the tab in the first index to the tab in the second index switches
+  // the tabs and adds the dragged tab to the group.
+  ASSERT_TRUE(PressInput(GetCenterInScreenCoordinates(tab_strip->tab_at(1))));
+  ASSERT_TRUE(DragInputTo(GetCenterInScreenCoordinates(tab_strip->tab_at(2))));
+
+  EXPECT_EQ("0 2 1 3", IDString(model));
+  EXPECT_THAT(model->ListTabsInGroup(group1), testing::ElementsAre(1, 2, 3));
+
+  ASSERT_TRUE(TabDragController::IsActive());
+
+  // Pressing escape will revert the tabs to original state before the drag.
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(browser(), ui::VKEY_ESCAPE, false,
+                                              false, false, false));
+  EXPECT_EQ("0 1 2 3", IDString(model));
+  EXPECT_THAT(model->ListTabsInGroup(group1), testing::ElementsAre(2, 3));
+  EXPECT_THAT(model->ListTabsInGroup(group2), testing::ElementsAre(1));
+}
+
 // Drags a tab within the window (without dragging the whole window) then
 // pressing a key ends the drag.
 IN_PROC_BROWSER_TEST_P(DetachToBrowserTabDragControllerTest,