[SxS] Extend Lifetime of GroupCollections to allow for group observation from tabstrip model.

In this CL, new APIs for Adding and Cleaning up detached groups are present. This is needed for group_model to eventually store raw pointers of the created Group tied to the Group Collection.

Bug: 392950198
Change-Id: I2cc1204db7d24c4c7c26bf56f37c7e2dd4b7018c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6249966
Commit-Queue: Shibalik Mohapatra <shibalik@chromium.org>
Reviewed-by: Eshwar Stalin <estalin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1418863}

chrome/browser/ui/tabs/tab_strip_collection.cc

@@ -229,6 +229,17 @@ size_t TabStripCollection::TotalTabCount() const {
   return TabCountRecursive();
 }
 
+void TabStripCollection::CreateGroup(
+    std::unique_ptr<tabs::TabGroupTabCollection> tab_group_collection) {
+  CHECK(tab_group_collection);
+  detached_group_collections_.push_back(std::move(tab_group_collection));
+}
+
+void TabStripCollection::CloseDetachedGroup(
+    const tab_groups::TabGroupId& group_id) {
+  PopDetachedGroupCollection(group_id).reset();
+}
+
 TabGroupTabCollection* TabStripCollection::MaybeCreateNewGroupCollectionForTab(
     int index,
     const tab_groups::TabGroupId& new_group) {
@@ -248,7 +259,25 @@ TabGroupTabCollection* TabStripCollection::MaybeCreateNewGroupCollectionForTab(
                 .value();
 
   return unpinned_collection_->AddTabGroup(
-      std::make_unique<TabGroupTabCollection>(new_group), dst_index);
+      PopDetachedGroupCollection(new_group), dst_index);
+}
+
+std::unique_ptr<tabs::TabGroupTabCollection>
+TabStripCollection::PopDetachedGroupCollection(
+    const tab_groups::TabGroupId& group_id) {
+  auto it = std::find_if(
+      detached_group_collections_.begin(), detached_group_collections_.end(),
+      [group_id](
+          const std::unique_ptr<tabs::TabGroupTabCollection>& collection) {
+        return collection->GetTabGroupId() == group_id;
+      });
+  CHECK(it != detached_group_collections_.end());
+
+  std::unique_ptr<tabs::TabGroupTabCollection> group_collection =
+      std::move(*it);
+  detached_group_collections_.erase(it);
+
+  return group_collection;
 }
 
 void TabStripCollection::MaybeRemoveGroupCollection(
@@ -257,11 +286,13 @@ void TabStripCollection::MaybeRemoveGroupCollection(
       unpinned_collection_->GetTabGroupCollection(group);
 
   if (group_collection && group_collection->TabCountRecursive() == 0) {
-    unpinned_collection_->CloseTabGroup(group_collection);
+    detached_group_collections_.push_back(
+        unpinned_collection_->RemoveGroup(group_collection));
   }
 }
 
 void TabStripCollection::ValidateData() {
+  CHECK(detached_group_collections_.empty());
   unpinned_collection_->ValidateCollections();
 }
 

chrome/browser/ui/tabs/tab_strip_collection.h

@@ -37,7 +37,6 @@ class TabStripCollection : public TabCollection {
   UnpinnedTabCollection* unpinned_collection() { return unpinned_collection_; }
 
   size_t IndexOfFirstNonPinnedTab() const;
-
   // Returns the tab at a particular index from the collection tree.
   // The index is a recursive index and if the index is invalid it returns
   // nullptr.
@@ -93,6 +92,14 @@ class TabStripCollection : public TabCollection {
       TabCollection* collection) override;
   size_t ChildCount() const override;
 
+  // Adds the `tab_group_collection` to `detached_group_collections_`
+  // so that it can be used when inserting a tab to a group.
+  void CreateGroup(
+      std::unique_ptr<tabs::TabGroupTabCollection> tab_group_collection);
+
+  // Clears all detached groups present in `detached_group_collections_`.
+  void CloseDetachedGroup(const tab_groups::TabGroupId& group_id);
+
   TabCollectionStorage* GetTabCollectionStorageForTesting() {
     return impl_.get();
   }
@@ -107,6 +114,11 @@ class TabStripCollection : public TabCollection {
       const tab_groups::TabGroupId& new_group);
   void MaybeRemoveGroupCollection(const tab_groups::TabGroupId& group);
 
+  // Removes the group collection with `group_id` from
+  // `detached_group_collections_`.
+  std::unique_ptr<tabs::TabGroupTabCollection> PopDetachedGroupCollection(
+      const tab_groups::TabGroupId& group_id);
+
   // Underlying implementation for the storage of children.
   std::unique_ptr<TabCollectionStorage> impl_;
 
@@ -119,6 +131,11 @@ class TabStripCollection : public TabCollection {
   // collection. This should be below `impl_` to avoid being a dangling pointer
   // during destruction.
   raw_ptr<UnpinnedTabCollection> unpinned_collection_;
+
+  // `tab_strip_model` creates this to allow extension of lifetime for groups to
+  // allow for group_model_ updates and observation methods.
+  std::vector<std::unique_ptr<tabs::TabGroupTabCollection>>
+      detached_group_collections_;
 };
 
 }  // namespace tabs

chrome/browser/ui/tabs/tab_strip_model.cc

@@ -63,12 +63,14 @@
 #include "chrome/browser/ui/tabs/tab_enums.h"
 #include "chrome/browser/ui/tabs/tab_group.h"
 #include "chrome/browser/ui/tabs/tab_group_model.h"
+#include "chrome/browser/ui/tabs/tab_group_tab_collection.h"
 #include "chrome/browser/ui/tabs/tab_model.h"
 #include "chrome/browser/ui/tabs/tab_strip_collection.h"
 #include "chrome/browser/ui/tabs/tab_strip_model_delegate.h"
 #include "chrome/browser/ui/tabs/tab_strip_model_observer.h"
 #include "chrome/browser/ui/tabs/tab_strip_user_gesture_details.h"
 #include "chrome/browser/ui/tabs/tab_utils.h"
+#include "chrome/browser/ui/tabs/unpinned_tab_collection.h"
 #include "chrome/browser/ui/thumbnails/thumbnail_tab_helper.h"
 #include "chrome/browser/ui/ui_features.h"
 #include "chrome/browser/ui/views/tabs/dragging/tab_drag_controller.h"
@@ -1208,6 +1210,8 @@ void TabStripModel::AddTabGroup(const tab_groups::TabGroupId group_id,
   CHECK(SupportsTabGroups());
   group_model_->AddTabGroup(group_id, visual_data,
                             base::PassKey<TabStripModel>());
+  contents_data_->CreateGroup(
+      std::make_unique<tabs::TabGroupTabCollection>(group_id));
 }
 
 tab_groups::TabGroupId TabStripModel::AddToNewGroup(
@@ -2716,6 +2720,8 @@ void TabStripModel::AddToNewGroupImpl(
 
   group_model_->AddTabGroup(new_group, visual_data,
                             base::PassKey<TabStripModel>());
+  contents_data_->CreateGroup(
+      std::make_unique<tabs::TabGroupTabCollection>(new_group));
 
   // Find a destination for the first tab that's not pinned or inside another
   // group. We will stack the rest of the tabs up to its right.
@@ -2920,12 +2926,12 @@ std::unique_ptr<tabs::TabModel> TabStripModel::RemoveTabFromIndexImpl(
     }
   }
 
-  ValidateTabStripModel();
-
   if (group_model_ && old_group) {
     TabGroupStateChanged(index, tab, old_group, std::nullopt);
   }
 
+  ValidateTabStripModel();
+
   return old_data;
 }
 
@@ -2948,13 +2954,10 @@ void TabStripModel::MoveTabToIndexImpl(
   }
 
   TabStripSelectionChange selection(GetActiveTab(), selection_model_);
-
   contents_data_->MoveTabRecursive(initial_index, final_index, group, pin);
 
   UpdateSelectionModelForMove(initial_index, final_index, select_after_move);
 
-  ValidateTabStripModel();
-
   selection.new_model = selection_model_;
   selection.new_tab = GetActiveTab();
   selection.new_contents = GetActiveWebContents();
@@ -2975,6 +2978,8 @@ void TabStripModel::MoveTabToIndexImpl(
       TabGroupStateChanged(final_index, tab, initial_group, tab->GetGroup());
     }
   }
+
+  ValidateTabStripModel();
 }
 
 void TabStripModel::MoveTabsToIndexImpl(
@@ -3005,8 +3010,6 @@ void TabStripModel::MoveTabsToIndexImpl(
 
   UpdateSelectionModelForMoves(tab_indices, destination_index);
 
-  ValidateTabStripModel();
-
   for (auto notification : notifications) {
     const int final_index = GetIndexOfTab(notification.tab);
     tabs::TabInterface* tab = GetTabAtIndex(final_index);
@@ -3022,6 +3025,8 @@ void TabStripModel::MoveTabsToIndexImpl(
       }
     }
   }
+
+  ValidateTabStripModel();
 }
 
 void TabStripModel::TabGroupStateChanged(
@@ -3070,6 +3075,7 @@ void TabStripModel::RemoveTabFromGroupModel(
 
   if (tab_group->IsEmpty()) {
     group_model_->RemoveTabGroup(group, base::PassKey<TabStripModel>());
+    contents_data_->CloseDetachedGroup(group);
   }
 }