Use TabStripModelObserver's new API: browser_tab_strip_controller

Replace old API with new API. This CL is a refactor
and has no intended behavior change.

To keep TabStrip UI consistent with model, make tabstrip UI the
first observer.

Bug: 842194
Change-Id: Icba9481d41ba32a7910593198cebd313ad531e5d
Reviewed-on: https://chromium-review.googlesource.com/c/1249461
Commit-Queue: Sang Woo Ko <sangwoo108@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604497}

chrome/browser/ui/tabs/tab_strip_model.cc

@@ -260,6 +260,21 @@ TabStripModel::~TabStripModel() {
   order_controller_.reset();
 }
 
+void TabStripModel::SetTabStripUI(TabStripModelObserver* observer) {
+  DCHECK(!tab_strip_ui_was_set_);
+
+  std::vector<TabStripModelObserver*> new_observers{observer};
+  for (auto& old_observer : observers_)
+    new_observers.push_back(&old_observer);
+
+  observers_.Clear();
+
+  for (auto* new_observer : new_observers)
+    observers_.AddObserver(new_observer);
+
+  tab_strip_ui_was_set_ = true;
+}
+
 void TabStripModel::AddObserver(TabStripModelObserver* observer) {
   observers_.AddObserver(observer);
 }

chrome/browser/ui/tabs/tab_strip_model.h

@@ -119,6 +119,10 @@ class TabStripModel {
   // Retrieves the TabStripModelDelegate associated with this TabStripModel.
   TabStripModelDelegate* delegate() const { return delegate_; }
 
+  // Sets the TabStripModelObserver used by the UI showing the tabs. As other
+  // observers may query the UI for state, the UI's observer must be first.
+  void SetTabStripUI(TabStripModelObserver* observer);
+
   // Add and remove observers to changes within this TabStripModel.
   void AddObserver(TabStripModelObserver* observer);
   void RemoveObserver(TabStripModelObserver* observer);
@@ -525,6 +529,9 @@ class TabStripModel {
   std::vector<std::unique_ptr<WebContentsData>> contents_data_;
 
   TabStripModelDelegate* delegate_;
+
+  bool tab_strip_ui_was_set_ = false;
+
   base::ObserverList<TabStripModelObserver>::Unchecked observers_;
 
   // A profile associated with this TabStripModel.

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

@@ -167,7 +167,7 @@ BrowserTabStripController::BrowserTabStripController(TabStripModel* model,
       tabstrip_(NULL),
       browser_view_(browser_view),
       hover_tab_selector_(model) {
-  model_->AddObserver(this);
+  model_->SetTabStripUI(this);
 
   local_pref_registrar_.Init(g_browser_process->local_state());
   local_pref_registrar_.Add(
@@ -436,54 +436,70 @@ Profile* BrowserTabStripController::GetProfile() const {
 ////////////////////////////////////////////////////////////////////////////////
 // BrowserTabStripController, TabStripModelObserver implementation:
 
-void BrowserTabStripController::TabInsertedAt(TabStripModel* tab_strip_model,
-                                              WebContents* contents,
-                                              int model_index,
-                                              bool is_active) {
-  DCHECK(contents);
-  DCHECK(model_->ContainsIndex(model_index));
-  AddTab(contents, model_index, is_active);
-}
-
-void BrowserTabStripController::TabDetachedAt(WebContents* contents,
-                                              int model_index,
-                                              bool was_active) {
-  // Cancel any pending tab transition.
-  hover_tab_selector_.CancelTabTransition();
-
-  tabstrip_->RemoveTabAt(contents, model_index, was_active);
-}
+void BrowserTabStripController::OnTabStripModelChanged(
+    TabStripModel* tab_strip_model,
+    const TabStripModelChange& change,
+    const TabStripSelectionChange& selection) {
+  switch (change.type()) {
+    case TabStripModelChange::kInserted: {
+      for (const auto& delta : change.deltas()) {
+        DCHECK(delta.insert.contents);
+        DCHECK(model_->ContainsIndex(delta.insert.index));
+        AddTab(delta.insert.contents, delta.insert.index,
+               selection.new_contents == delta.insert.contents);
+      }
+      break;
+    }
+    case TabStripModelChange::kRemoved: {
+      for (const auto& delta : change.deltas()) {
+        // Cancel any pending tab transition.
+        hover_tab_selector_.CancelTabTransition();
 
-void BrowserTabStripController::ActiveTabChanged(
-    content::WebContents* old_contents,
-    content::WebContents* new_contents,
-    int index,
-    int reason) {
-  // It's possible for |new_contents| to be null when the final tab in a tab
-  // strip is closed.
-  if (new_contents && index != TabStripModel::kNoTab) {
-    TabUIHelper::FromWebContents(new_contents)->set_was_active_at_least_once();
-    SetTabDataAt(new_contents, index);
+        tabstrip_->RemoveTabAt(delta.remove.contents, delta.remove.index,
+                               delta.remove.contents == selection.old_contents);
+      }
+      break;
+    }
+    case TabStripModelChange::kMoved: {
+      for (const auto& delta : change.deltas()) {
+        // Cancel any pending tab transition.
+        hover_tab_selector_.CancelTabTransition();
+
+        // A move may have resulted in the pinned state changing, so pass in a
+        // TabRendererData.
+        tabstrip_->MoveTab(
+            delta.move.from_index, delta.move.to_index,
+            TabRendererDataFromModel(delta.move.contents, delta.move.to_index,
+                                     EXISTING_TAB));
+      }
+      break;
+    }
+    case TabStripModelChange::kReplaced: {
+      for (const auto& delta : change.deltas())
+        SetTabDataAt(delta.replace.new_contents, delta.replace.index);
+      break;
+    }
+    case TabStripModelChange::kSelectionOnly:
+      break;
   }
-}
 
-void BrowserTabStripController::TabSelectionChanged(
-    TabStripModel* tab_strip_model,
-    const ui::ListSelectionModel& old_model) {
-  tabstrip_->SetSelection(model_->selection_model());
-}
+  if (tab_strip_model->empty())
+    return;
 
-void BrowserTabStripController::TabMoved(WebContents* contents,
-                                         int from_model_index,
-                                         int to_model_index) {
-  // Cancel any pending tab transition.
-  hover_tab_selector_.CancelTabTransition();
+  if (selection.active_tab_changed()) {
+    // It's possible for |new_contents| to be null when the final tab in a tab
+    // strip is closed.
+    content::WebContents* new_contents = selection.new_contents;
+    int index = selection.new_model.active();
+    if (new_contents && index != TabStripModel::kNoTab) {
+      TabUIHelper::FromWebContents(new_contents)
+          ->set_was_active_at_least_once();
+      SetTabDataAt(new_contents, index);
+    }
+  }
 
-  // A move may have resulted in the pinned state changing, so pass in a
-  // TabRendererData.
-  tabstrip_->MoveTab(
-      from_model_index, to_model_index,
-      TabRendererDataFromModel(contents, to_model_index, EXISTING_TAB));
+  if (selection.selection_changed())
+    tabstrip_->SetSelection(selection.new_model);
 }
 
 void BrowserTabStripController::TabChangedAt(WebContents* contents,
@@ -492,13 +508,6 @@ void BrowserTabStripController::TabChangedAt(WebContents* contents,
   SetTabDataAt(contents, model_index);
 }
 
-void BrowserTabStripController::TabReplacedAt(TabStripModel* tab_strip_model,
-                                              WebContents* old_contents,
-                                              WebContents* new_contents,
-                                              int model_index) {
-  SetTabDataAt(new_contents, model_index);
-}
-
 void BrowserTabStripController::TabPinnedStateChanged(
     TabStripModel* tab_strip_model,
     WebContents* contents,

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

@@ -88,29 +88,13 @@ class BrowserTabStripController : public TabStripController,
   Profile* GetProfile() const override;
 
   // TabStripModelObserver implementation:
-  void TabInsertedAt(TabStripModel* tab_strip_model,
-                     content::WebContents* contents,
-                     int model_index,
-                     bool is_active) override;
-  void TabDetachedAt(content::WebContents* contents,
-                     int model_index,
-                     bool was_active) override;
-  void ActiveTabChanged(content::WebContents* old_contents,
-                        content::WebContents* new_contents,
-                        int index,
-                        int reason) override;
-  void TabSelectionChanged(TabStripModel* tab_strip_model,
-                           const ui::ListSelectionModel& old_model) override;
-  void TabMoved(content::WebContents* contents,
-                int from_model_index,
-                int to_model_index) override;
+  void OnTabStripModelChanged(
+      TabStripModel* tab_strip_model,
+      const TabStripModelChange& change,
+      const TabStripSelectionChange& selection) override;
   void TabChangedAt(content::WebContents* contents,
                     int model_index,
                     TabChangeType change_type) override;
-  void TabReplacedAt(TabStripModel* tab_strip_model,
-                     content::WebContents* old_contents,
-                     content::WebContents* new_contents,
-                     int model_index) override;
   void TabPinnedStateChanged(TabStripModel* tab_strip_model,
                              content::WebContents* contents,
                              int model_index) override;