[TabStrip] Improve const correctness and toggle efficiency

- Updated `IsCommandEnabledForTab` and `ExecuteCommandForTab` to use `const Tab*`.

Change-Id: I8c98f4752cb14816bcf568e94f9ea36204b4b1c8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5442047
Reviewed-by: Shibalik Mohapatra <shibalik@chromium.org>
Commit-Queue: Darryl James <dljames@chromium.org>
Reviewed-by: Darryl James <dljames@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1300717}

AUTHORS

@@ -656,8 +656,8 @@ Jincheol Jo <jincheol.jo@navercorp.com>
 Jinfeng Ma <majinfeng1@xiaomi.com>
 Jing Zhao <zhaojing7@xiaomi.com>
 Jinglong Zuo <zuojinglong@xiaomi.com>
-Jingqi Sun <sunjingqi47@gmail.com>
 Jingqi Sun <jingqi.sun@hotmail.com>
+Jingqi Sun <sunjingqi47@gmail.com>
 Jingwei Liu <kingweiliu@gmail.com>
 Jingyi Wei <wjywbs@gmail.com>
 Jinho Bang <jinho.bang@samsung.com>

chrome/browser/ui/tabs/tab_group.cc

@@ -33,18 +33,21 @@ TabGroup::TabGroup(TabGroupController* controller,
 
 TabGroup::~TabGroup() = default;
 
-void TabGroup::SetVisualData(const tab_groups::TabGroupVisualData& visual_data,
+void TabGroup::SetVisualData(tab_groups::TabGroupVisualData visual_data,
                              bool is_customized) {
+  // Move current visuals to old_visuals before updating
   std::unique_ptr<tab_groups::TabGroupVisualData> old_visuals =
       std::move(visual_data_);
   TabGroupChange::VisualsChange visuals;
   visuals.old_visuals = old_visuals.get();
   visuals.new_visuals = &visual_data;
+
   visual_data_ = std::make_unique<tab_groups::TabGroupVisualData>(visual_data);
 
   // Once the visual data is customized, it should stay customized.
   is_customized_ |= is_customized;
 
+  // Notify the controller of the visual change
   controller_->ChangeTabGroupVisuals(id_, visuals);
 }
 

chrome/browser/ui/tabs/tab_group.h

@@ -42,7 +42,7 @@ class TabGroup {
   // method is called from the user explicitly setting the data and defaults to
   // false for callsites that may set the data such as tab restore. Once set to
   // true, |is_customized| cannot be reset to false.
-  void SetVisualData(const tab_groups::TabGroupVisualData& visual_data,
+  void SetVisualData(tab_groups::TabGroupVisualData visual_data,
                      bool is_customized = false);
 
   // Returns a user-visible string describing the contents of the group, such as

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

@@ -217,7 +217,7 @@ void BrowserTabStripController::InitFromModel(TabStrip* tabstrip) {
 
 bool BrowserTabStripController::IsCommandEnabledForTab(
     TabStripModel::ContextMenuCommand command_id,
-    Tab* tab) const {
+    const Tab* tab) const {
   const std::optional<int> model_index = tabstrip_->GetModelIndexOf(tab);
   return model_index.has_value() ? model_->IsContextMenuCommandEnabled(
                                        model_index.value(), command_id)
@@ -226,13 +226,13 @@ bool BrowserTabStripController::IsCommandEnabledForTab(
 
 void BrowserTabStripController::ExecuteCommandForTab(
     TabStripModel::ContextMenuCommand command_id,
-    Tab* tab) {
+    const Tab* tab) {
   const std::optional<int> model_index = tabstrip_->GetModelIndexOf(tab);
   if (model_index.has_value())
     model_->ExecuteContextMenuCommand(model_index.value(), command_id);
 }
 
-bool BrowserTabStripController::IsTabPinned(Tab* tab) const {
+bool BrowserTabStripController::IsTabPinned(const Tab* tab) const {
   return IsTabPinned(tabstrip_->GetModelIndexOf(tab).value());
 }
 
@@ -423,9 +423,11 @@ void BrowserTabStripController::ToggleTabGroupCollapsedState(
   }
   tabstrip_->ToggleTabGroup(group, !is_currently_collapsed, origin);
 
-  tab_groups::TabGroupVisualData new_data(
-      GetGroupTitle(group), GetGroupColorId(group), !is_currently_collapsed);
-  model_->group_model()->GetTabGroup(group)->SetVisualData(new_data, true);
+  model_->group_model()->GetTabGroup(group)->SetVisualData(
+      tab_groups::TabGroupVisualData(GetGroupTitle(group),
+                                     GetGroupColorId(group),
+                                     !is_currently_collapsed),
+      true);
 }
 
 void BrowserTabStripController::ShowContextMenuForTab(

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

@@ -56,10 +56,10 @@ class BrowserTabStripController : public TabStripController,
   TabStripModel* model() const { return model_; }
 
   bool IsCommandEnabledForTab(TabStripModel::ContextMenuCommand command_id,
-                              Tab* tab) const;
+                              const Tab* tab) const;
   void ExecuteCommandForTab(TabStripModel::ContextMenuCommand command_id,
-                            Tab* tab);
-  bool IsTabPinned(Tab* tab) const;
+                            const Tab* tab);
+  bool IsTabPinned(const Tab* tab) const;
 
   // TabStripController implementation:
   const ui::ListSelectionModel& GetSelectionModel() const override;

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

@@ -567,9 +567,9 @@ class TabStrip::TabDragContextImpl : public TabDragContext,
     // Reset the layout size as we've effectively laid out a different size.
     // This ensures a layout happens after the drag is done.
     tab_strip_->tab_container_->InvalidateIdealBounds();
-    if (views[0]->group().has_value()) {
+    if (views.at(0)->group().has_value()) {
       tab_strip_->tab_container_->UpdateTabGroupVisuals(
-          views[0]->group().value());
+          views.at(0)->group().value());
     }
   }