Refactor TabStripModelChange.

This CL does several positive things:
 - allows us to have changes include non-basic types by removing the
   use of a C union in the Delta class
 - in lieu of std::variant, makes access to each type of change runtime
   type-safe via DCHECK()
 - make all changes single items rather than lists of (implicitly)
   homogeneous deltas

Each delta may affect multiple tabs, but only (currently) in the case
of multiple insert and multiple remove; we also make the fact that
there can be more than one page inserted or removed explicit.

Bug: 958103
Change-Id: If6b4291ab05857830b94d596d664653f128d1368
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1576276
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Commit-Queue: Dana Fried <dfried@chromium.org>
Cr-Commit-Position: refs/heads/master@{#657513}

chrome/browser/devtools/devtools_auto_opener.cc

@@ -22,8 +22,7 @@ void DevToolsAutoOpener::OnTabStripModelChanged(
   if (change.type() != TabStripModelChange::kInserted)
     return;
 
-  for (const auto& delta : change.deltas()) {
-    if (!DevToolsWindow::IsDevToolsWindow(delta.insert.contents))
-      DevToolsWindow::OpenDevToolsWindow(delta.insert.contents);
-  }
+  for (const auto& contents : change.GetInsert()->contents)
+    if (!DevToolsWindow::IsDevToolsWindow(contents.contents))
+      DevToolsWindow::OpenDevToolsWindow(contents.contents);
 }

chrome/browser/devtools/global_confirm_info_bar.cc

@@ -184,9 +184,8 @@ void GlobalConfirmInfoBar::OnTabStripModelChanged(
     const TabStripSelectionChange& selection) {
   if (change.type() != TabStripModelChange::kInserted)
     return;
-
-  for (const auto& delta : change.deltas())
-    MaybeAddInfoBar(delta.insert.contents);
+  for (const auto& contents : change.GetInsert()->contents)
+    MaybeAddInfoBar(contents.contents);
 }
 
 void GlobalConfirmInfoBar::TabChangedAt(content::WebContents* web_contents,

chrome/browser/extensions/api/tabs/tabs_event_router.cc

@@ -187,36 +187,35 @@ void TabsEventRouter::OnTabStripModelChanged(
     const TabStripSelectionChange& selection) {
   switch (change.type()) {
     case TabStripModelChange::kInserted: {
-      for (const auto& delta : change.deltas()) {
-        DispatchTabInsertedAt(tab_strip_model, delta.insert.contents,
-                              delta.insert.index,
-                              selection.new_contents == delta.insert.contents);
+      for (const auto& contents : change.GetInsert()->contents) {
+        DispatchTabInsertedAt(tab_strip_model, contents.contents,
+                              contents.index,
+                              selection.new_contents == contents.contents);
       }
       break;
     }
     case TabStripModelChange::kRemoved: {
-      for (const auto& delta : change.deltas()) {
-        if (delta.remove.will_be_deleted)
-          DispatchTabClosingAt(tab_strip_model, delta.remove.contents,
-                               delta.remove.index);
-
-        DispatchTabDetachedAt(delta.remove.contents, delta.remove.index,
-                              selection.old_contents == delta.remove.contents);
+      const bool will_be_deleted = change.GetRemove()->will_be_deleted;
+      for (const auto& contents : change.GetRemove()->contents) {
+        if (will_be_deleted) {
+          DispatchTabClosingAt(tab_strip_model, contents.contents,
+                               contents.index);
+        }
+
+        DispatchTabDetachedAt(contents.contents, contents.index,
+                              selection.old_contents == contents.contents);
       }
       break;
     }
     case TabStripModelChange::kMoved: {
-      for (const auto& delta : change.deltas()) {
-        DispatchTabMoved(delta.move.contents, delta.move.from_index,
-                         delta.move.to_index);
-      }
+      auto* move = change.GetMove();
+      DispatchTabMoved(move->contents, move->from_index, move->to_index);
       break;
     }
     case TabStripModelChange::kReplaced: {
-      for (const auto& delta : change.deltas()) {
-        DispatchTabReplacedAt(delta.replace.old_contents,
-                              delta.replace.new_contents, delta.replace.index);
-      }
+      auto* replace = change.GetReplace();
+      DispatchTabReplacedAt(replace->old_contents, replace->new_contents,
+                            replace->index);
       break;
     }
     case TabStripModelChange::kGroupChanged:

chrome/browser/extensions/api/web_navigation/web_navigation_api.cc

@@ -98,25 +98,22 @@ void WebNavigationEventRouter::OnTabStripModelChanged(
   if (change.type() != TabStripModelChange::kReplaced)
     return;
 
-  for (const auto& delta : change.deltas()) {
-    content::WebContents* old_contents = delta.replace.old_contents;
-    content::WebContents* new_contents = delta.replace.new_contents;
-
-    WebNavigationTabObserver* tab_observer =
-        WebNavigationTabObserver::Get(old_contents);
-    if (!tab_observer) {
-      // If you hit this DCHECK(), please add reproduction steps to
-      // http://crbug.com/109464.
-      DCHECK(GetViewType(old_contents) != VIEW_TYPE_TAB_CONTENTS);
-      continue;
-    }
-    if (!FrameNavigationState::IsValidUrl(old_contents->GetURL()) ||
-        !FrameNavigationState::IsValidUrl(new_contents->GetURL()))
-      continue;
+  auto* replace = change.GetReplace();
+  WebNavigationTabObserver* tab_observer =
+      WebNavigationTabObserver::Get(replace->old_contents);
 
-    web_navigation_api_helpers::DispatchOnTabReplaced(old_contents, profile_,
-                                                      new_contents);
+  if (!tab_observer) {
+    // If you hit this DCHECK(), please add reproduction steps to
+    // http://crbug.com/109464.
+    DCHECK(GetViewType(replace->old_contents) != VIEW_TYPE_TAB_CONTENTS);
+    return;
   }
+  if (!FrameNavigationState::IsValidUrl(replace->old_contents->GetURL()) ||
+      !FrameNavigationState::IsValidUrl(replace->new_contents->GetURL()))
+    return;
+
+  web_navigation_api_helpers::DispatchOnTabReplaced(
+      replace->old_contents, profile_, replace->new_contents);
 }
 
 void WebNavigationEventRouter::Observe(

chrome/browser/metrics/desktop_session_duration/audible_contents_tracker.cc

@@ -36,28 +36,21 @@ void AudibleContentsTracker::OnTabStripModelChanged(
     TabStripModel* tab_strip_model,
     const TabStripModelChange& change,
     const TabStripSelectionChange& selection) {
-  if (change.type() != TabStripModelChange::kRemoved &&
-      change.type() != TabStripModelChange::kReplaced)
-    return;
-
-  for (const auto& delta : change.deltas()) {
-    content::WebContents* removed_contents = nullptr;
-    content::WebContents* added_contents = nullptr;
-    if (change.type() == TabStripModelChange::kReplaced) {
-      removed_contents = delta.replace.old_contents;
-      added_contents = delta.replace.new_contents;
-    } else if (delta.remove.will_be_deleted) {
-      removed_contents = delta.remove.contents;
+  if (change.type() == TabStripModelChange::kRemoved) {
+    auto* remove = change.GetRemove();
+    if (remove->will_be_deleted) {
+      for (const auto& contents : remove->contents)
+        RemoveAudibleWebContents(contents.contents);
     }
-
-    if (removed_contents)
-      RemoveAudibleWebContents(removed_contents);
-
-    if (added_contents) {
+  } else if (change.type() == TabStripModelChange::kReplaced) {
+    auto* replace = change.GetReplace();
+    if (replace->old_contents)
+      RemoveAudibleWebContents(replace->old_contents);
+    if (replace->new_contents) {
       auto* audible_helper =
-          RecentlyAudibleHelper::FromWebContents(added_contents);
+          RecentlyAudibleHelper::FromWebContents(replace->new_contents);
       if (audible_helper->WasRecentlyAudible())
-        AddAudibleWebContents(added_contents);
+        AddAudibleWebContents(replace->new_contents);
     }
   }
 }

chrome/browser/metrics/tab_reactivation_tracker.cc

@@ -80,9 +80,10 @@ void TabReactivationTracker::OnTabStripModelChanged(
     const TabStripModelChange& change,
     const TabStripSelectionChange& selection) {
   if (change.type() == TabStripModelChange::kRemoved) {
-    for (const auto& delta : change.deltas()) {
-      if (delta.remove.will_be_deleted)
-        GetHelper(delta.remove.contents)->OnTabClosing();
+    auto* remove = change.GetRemove();
+    if (remove->will_be_deleted) {
+      for (const auto& contents : remove->contents)
+        GetHelper(contents.contents)->OnTabClosing();
     }
   }
 

chrome/browser/metrics/tab_stats_tracker.cc

@@ -313,8 +313,8 @@ void TabStatsTracker::OnTabStripModelChanged(
     const TabStripSelectionChange& selection) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   if (change.type() == TabStripModelChange::kInserted) {
-    for (const auto& delta : change.deltas())
-      OnInitialOrInsertedTab(delta.insert.contents);
+    for (const auto& contents : change.GetInsert()->contents)
+      OnInitialOrInsertedTab(contents.contents);
 
     tab_stats_data_store_->UpdateMaxTabsPerWindowIfNeeded(
         static_cast<size_t>(tab_strip_model->count()));
@@ -323,15 +323,13 @@ void TabStatsTracker::OnTabStripModelChanged(
   }
 
   if (change.type() == TabStripModelChange::kReplaced) {
-    for (const auto& delta : change.deltas()) {
-      content::WebContents* old_contents = delta.replace.old_contents;
-      content::WebContents* new_contents = delta.replace.new_contents;
-      tab_stats_data_store_->OnTabReplaced(old_contents, new_contents);
-      web_contents_usage_observers_.insert(std::make_pair(
-          new_contents,
-          std::make_unique<WebContentsUsageObserver>(new_contents, this)));
-      web_contents_usage_observers_.erase(old_contents);
-    }
+    auto* replace = change.GetReplace();
+    tab_stats_data_store_->OnTabReplaced(replace->old_contents,
+                                         replace->new_contents);
+    web_contents_usage_observers_.insert(std::make_pair(
+        replace->new_contents, std::make_unique<WebContentsUsageObserver>(
+                                   replace->new_contents, this)));
+    web_contents_usage_observers_.erase(replace->old_contents);
   }
 }
 

chrome/browser/metrics/tab_usage_recorder.cc

@@ -112,11 +112,9 @@ void TabUsageRecorder::OnTabStripModelChanged(
   if (change.type() != TabStripModelChange::kInserted)
     return;
 
-  for (const auto& delta : change.deltas()) {
-    // Set the initial pin state.
-    TabPinnedStateChanged(tab_strip_model, delta.insert.contents,
-                          delta.insert.index);
-  }
+  // Set the initial pin state.
+  for (const auto& contents : change.GetInsert()->contents)
+    TabPinnedStateChanged(tab_strip_model, contents.contents, contents.index);
 }
 
 void TabUsageRecorder::TabPinnedStateChanged(TabStripModel* tab_strip_model,

chrome/browser/prerender/prerender_browsertest.cc

@@ -422,20 +422,19 @@ class NavigationOrSwapObserver : public WebContentsObserver,
     if (change.type() != TabStripModelChange::kReplaced)
       return;
 
-    for (const auto& delta : change.deltas()) {
-      if (delta.replace.old_contents != web_contents())
-        continue;
-
-      // Switch to observing the new WebContents.
-      Observe(delta.replace.new_contents);
-      if (delta.replace.new_contents->IsLoading()) {
-        // If the new WebContents is still loading, wait for it to complete.
-        // Only one load post-swap is supported.
-        did_start_loading_ = true;
-        number_of_loads_ = 1;
-      } else {
-        loop_.Quit();
-      }
+    auto* replace = change.GetReplace();
+    if (replace->old_contents != web_contents())
+      return;
+
+    // Switch to observing the new WebContents.
+    Observe(replace->new_contents);
+    if (replace->new_contents->IsLoading()) {
+      // If the new WebContents is still loading, wait for it to complete.
+      // Only one load post-swap is supported.
+      did_start_loading_ = true;
+      number_of_loads_ = 1;
+    } else {
+      loop_.Quit();
     }
   }
 

chrome/browser/resource_coordinator/tab_activity_watcher.cc

@@ -565,33 +565,32 @@ void TabActivityWatcher::OnTabStripModelChanged(
     const TabStripSelectionChange& selection) {
   switch (change.type()) {
     case TabStripModelChange::kInserted: {
-      for (const auto& delta : change.deltas()) {
+      for (const auto& contents : change.GetInsert()->contents) {
         // Ensure the WebContentsData is created to observe this WebContents
         // since it may represent a newly created tab.
-        WebContentsData::CreateForWebContents(delta.insert.contents);
-        WebContentsData::FromWebContents(delta.insert.contents)
-            ->TabInserted(selection.new_contents == delta.insert.contents);
+        WebContentsData::CreateForWebContents(contents.contents);
+        WebContentsData::FromWebContents(contents.contents)
+            ->TabInserted(selection.new_contents == contents.contents);
       }
       break;
     }
     case TabStripModelChange::kRemoved: {
-      for (const auto& delta : change.deltas())
-        WebContentsData::FromWebContents(delta.remove.contents)->TabDetached();
+      for (const auto& contents : change.GetRemove()->contents)
+        WebContentsData::FromWebContents(contents.contents)->TabDetached();
       break;
     }
     case TabStripModelChange::kReplaced: {
-      for (const auto& delta : change.deltas()) {
-        WebContentsData* old_web_contents_data =
-            WebContentsData::FromWebContents(delta.replace.old_contents);
-        old_web_contents_data->WasReplaced();
+      auto* replace = change.GetReplace();
+      WebContentsData* old_web_contents_data =
+          WebContentsData::FromWebContents(replace->old_contents);
+      old_web_contents_data->WasReplaced();
 
-        // Ensure the WebContentsData is created to observe this WebContents
-        // since it likely hasn't been inserted into a tabstrip before.
-        WebContentsData::CreateForWebContents(delta.replace.new_contents);
+      // Ensure the WebContentsData is created to observe this WebContents
+      // since it likely hasn't been inserted into a tabstrip before.
+      WebContentsData::CreateForWebContents(replace->new_contents);
 
-        WebContentsData::FromWebContents(delta.replace.new_contents)
-            ->DidReplace(*old_web_contents_data);
-      }
+      WebContentsData::FromWebContents(replace->new_contents)
+          ->DidReplace(*old_web_contents_data);
       break;
     }
     case TabStripModelChange::kMoved:

chrome/browser/resource_coordinator/tab_lifecycle_unit_source.cc

@@ -282,20 +282,20 @@ void TabLifecycleUnitSource::OnTabStripModelChanged(
     const TabStripSelectionChange& selection) {
   switch (change.type()) {
     case TabStripModelChange::kInserted: {
-      for (const auto& delta : change.deltas()) {
-        OnTabInserted(tab_strip_model, delta.insert.contents,
-                      selection.new_contents == delta.insert.contents);
+      for (const auto& contents : change.GetInsert()->contents) {
+        OnTabInserted(tab_strip_model, contents.contents,
+                      selection.new_contents == contents.contents);
       }
       break;
     }
     case TabStripModelChange::kRemoved: {
-      for (const auto& delta : change.deltas())
-        OnTabDetached(delta.remove.contents);
+      for (const auto& contents : change.GetRemove()->contents)
+        OnTabDetached(contents.contents);
       break;
     }
     case TabStripModelChange::kReplaced: {
-      for (const auto& delta : change.deltas())
-        OnTabReplaced(delta.replace.old_contents, delta.replace.new_contents);
+      auto* replace = change.GetReplace();
+      OnTabReplaced(replace->old_contents, replace->new_contents);
       break;
     }
     case TabStripModelChange::kMoved:

chrome/browser/resource_coordinator/tab_manager.cc

@@ -453,10 +453,8 @@ void TabManager::OnTabStripModelChanged(
     const TabStripModelChange& change,
     const TabStripSelectionChange& selection) {
   if (change.type() == TabStripModelChange::kReplaced) {
-    for (const auto& delta : change.deltas()) {
-      WebContentsData::CopyState(delta.replace.old_contents,
-                                 delta.replace.new_contents);
-    }
+    auto* replace = change.GetReplace();
+    WebContentsData::CopyState(replace->old_contents, replace->new_contents);
   }
 
   if (selection.active_tab_changed() && !tab_strip_model->empty())

chrome/browser/supervised_user/supervised_user_browsertest.cc

@@ -269,11 +269,14 @@ class TabClosingObserver : public TabStripModelObserver {
     if (change.type() != TabStripModelChange::kRemoved)
       return;
 
-    for (const auto& delta : change.deltas()) {
-      if (delta.remove.will_be_deleted && contents_ == delta.remove.contents) {
+    auto* remove = change.GetRemove();
+    if (!remove->will_be_deleted)
+      return;
+
+    for (const auto& contents : remove->contents) {
+      if (contents_ == contents.contents) {
         if (run_loop_.running())
           run_loop_.Quit();
-
         contents_ = nullptr;
         return;
       }

chrome/browser/sync/sessions/browser_list_router_helper.cc

@@ -52,18 +52,20 @@ void BrowserListRouterHelper::OnTabStripModelChanged(
     TabStripModel* tab_strip_model,
     const TabStripModelChange& change,
     const TabStripSelectionChange& selection) {
-  if (change.type() != TabStripModelChange::kInserted &&
-      change.type() != TabStripModelChange::kReplaced)
+  std::vector<content::WebContents*> web_contents;
+  if (change.type() == TabStripModelChange::kInserted) {
+    for (const auto& contents : change.GetInsert()->contents)
+      web_contents.push_back(contents.contents);
+  } else if (change.type() == TabStripModelChange::kReplaced) {
+    web_contents.push_back(change.GetReplace()->new_contents);
+  } else {
     return;
+  }
 
-  for (const auto& delta : change.deltas()) {
-    content::WebContents* web_contents =
-        change.type() == TabStripModelChange::kInserted
-            ? delta.insert.contents
-            : delta.replace.new_contents;
-    if (Profile::FromBrowserContext(web_contents->GetBrowserContext()) ==
+  for (auto* contents : web_contents) {
+    if (Profile::FromBrowserContext(contents->GetBrowserContext()) ==
         profile_) {
-      router_->NotifyTabModified(web_contents, false);
+      router_->NotifyTabModified(contents, false);
     }
   }
 }