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 <[email protected]>
Reviewed-by: Peter Kasting <[email protected]>
Reviewed-by: Bret Sepulveda <[email protected]>
Commit-Queue: Dana Fried <[email protected]>
Cr-Commit-Position: refs/heads/master@{#657513}
diff --git a/chrome/browser/ui/browser_command_controller.cc b/chrome/browser/ui/browser_command_controller.cc
index 517c4fd..970c19e 100644
--- a/chrome/browser/ui/browser_command_controller.cc
+++ b/chrome/browser/ui/browser_command_controller.cc
@@ -758,28 +758,32 @@
TabStripModel* tab_strip_model,
const TabStripModelChange& change,
const TabStripSelectionChange& selection) {
- if (change.type() != TabStripModelChange::kInserted &&
- change.type() != TabStripModelChange::kReplaced &&
- change.type() != TabStripModelChange::kRemoved)
- return;
+ std::vector<content::WebContents*> new_contents;
+ std::vector<content::WebContents*> old_contents;
- for (const auto& delta : change.deltas()) {
- content::WebContents* new_contents = nullptr;
- content::WebContents* old_contents = nullptr;
- if (change.type() == TabStripModelChange::kInserted) {
- new_contents = delta.insert.contents;
- } else if (change.type() == TabStripModelChange::kReplaced) {
- new_contents = delta.replace.new_contents;
- old_contents = delta.replace.old_contents;
- } else {
- old_contents = delta.remove.contents;
+ switch (change.type()) {
+ case TabStripModelChange::kInserted:
+ for (const auto& contents : change.GetInsert()->contents)
+ new_contents.push_back(contents.contents);
+ break;
+ case TabStripModelChange::kReplaced: {
+ auto* replace = change.GetReplace();
+ new_contents.push_back(replace->new_contents);
+ old_contents.push_back(replace->old_contents);
+ break;
}
-
- if (old_contents)
- RemoveInterstitialObservers(old_contents);
- if (new_contents)
- AddInterstitialObservers(new_contents);
+ case TabStripModelChange::kRemoved:
+ for (const auto& contents : change.GetRemove()->contents)
+ old_contents.push_back(contents.contents);
+ break;
+ default:
+ break;
}
+
+ for (auto* contents : old_contents)
+ RemoveInterstitialObservers(contents);
+ for (auto* contents : new_contents)
+ AddInterstitialObservers(contents);
}
void BrowserCommandController::TabBlockedStateChanged(
