Notify selection changes made by tab removal

When selected tabs were removed, we must notify that even
selection model is unchanged. This is because the old selection model
and new one would be same when the newly activated tab was right after
prviously active tab.

Bug: 842194, 903438
Change-Id: I23fd9d8ba9f1ac10b3daf03088a2c2d974f435d8
Reviewed-on: https://chromium-review.googlesource.com/c/1331098
Commit-Queue: Sang Woo Ko <sangwoo108@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607485}

chrome/browser/ui/tabs/tab_strip_model.cc

@@ -453,7 +453,6 @@ std::unique_ptr<content::WebContents> TabStripModel::DetachWebContentsImpl(
 
 void TabStripModel::SendDetachWebContentsNotifications(
     DetachNotifications* notifications) {
-  bool was_any_tab_selected = false;
   std::vector<TabStripModelChange::Delta> deltas;
 
   // Sort the DetachedWebContents in decreasing order of
@@ -479,17 +478,18 @@ void TabStripModel::SendDetachWebContentsNotifications(
   selection.old_model = notifications->selection_model;
   selection.new_model = selection_model_;
   selection.reason = TabStripModelObserver::CHANGE_REASON_NONE;
+  selection.selected_tabs_were_removed = std::any_of(
+      notifications->detached_web_contents.begin(),
+      notifications->detached_web_contents.end(), [&notifications](auto& dwc) {
+        return notifications->selection_model.IsSelected(
+            dwc->index_before_any_removals);
+      });
 
   TabStripModelChange change(TabStripModelChange::kRemoved, deltas);
   for (auto& observer : observers_)
     observer.OnTabStripModelChanged(this, change, selection);
 
   for (auto& dwc : notifications->detached_web_contents) {
-    if (notifications->selection_model.IsSelected(
-            dwc->index_before_any_removals)) {
-      was_any_tab_selected = true;
-    }
-
     if (notifications->initially_active_web_contents &&
         dwc->contents.get() == notifications->initially_active_web_contents) {
       if (!empty())

chrome/browser/ui/tabs/tab_strip_model_observer.cc

@@ -6,6 +6,9 @@
 
 using content::WebContents;
 
+////////////////////////////////////////////////////////////////////////////////
+// TabStripModelChange
+//
 // static
 TabStripModelChange::Delta TabStripModelChange::CreateInsertDelta(
     content::WebContents* contents,
@@ -55,10 +58,13 @@ TabStripModelChange::TabStripModelChange(
     const std::vector<TabStripModelChange::Delta>& deltas)
     : type_(type), deltas_(deltas) {}
 
-TabStripModelChange::~TabStripModelChange() = default;
-
 TabStripModelChange::TabStripModelChange(TabStripModelChange&& other) = default;
 
+TabStripModelChange::~TabStripModelChange() = default;
+
+////////////////////////////////////////////////////////////////////////////////
+// TabStripSelectionChange
+//
 TabStripSelectionChange::TabStripSelectionChange() = default;
 
 TabStripSelectionChange::TabStripSelectionChange(
@@ -70,6 +76,17 @@ TabStripSelectionChange::TabStripSelectionChange(
       new_model(selection_model),
       reason(0) {}
 
+TabStripSelectionChange::~TabStripSelectionChange() = default;
+
+TabStripSelectionChange::TabStripSelectionChange(
+    const TabStripSelectionChange& other) = default;
+
+TabStripSelectionChange& TabStripSelectionChange::operator=(
+    const TabStripSelectionChange& other) = default;
+
+////////////////////////////////////////////////////////////////////////////////
+// TabStripModelObserver
+//
 TabStripModelObserver::TabStripModelObserver() {
 }
 

chrome/browser/ui/tabs/tab_strip_model_observer.h

@@ -111,6 +111,10 @@ class TabStripModelChange {
 // Struct to carry changes on selection/activation.
 struct TabStripSelectionChange {
   TabStripSelectionChange();
+  TabStripSelectionChange(const TabStripSelectionChange& other);
+  ~TabStripSelectionChange();
+
+  TabStripSelectionChange& operator=(const TabStripSelectionChange& other);
 
   // Fill TabStripSelectionChange with given |contents| and |selection_model|.
   // note that |new_contents| and |new_model| will be filled too so that
@@ -122,7 +126,9 @@ struct TabStripSelectionChange {
 
   // TODO(sangwoo.ko) Do we need something to indicate that the change
   // was made implicitly?
-  bool selection_changed() const { return old_model != new_model; }
+  bool selection_changed() const {
+    return selected_tabs_were_removed || old_model != new_model;
+  }
 
   content::WebContents* old_contents = nullptr;
   content::WebContents* new_contents = nullptr;
@@ -130,6 +136,8 @@ struct TabStripSelectionChange {
   ui::ListSelectionModel old_model;
   ui::ListSelectionModel new_model;
 
+  bool selected_tabs_were_removed = false;
+
   int reason = 0;
 };