Introduce TabStripModelObserver::OnTabStripChanged()

Previously, selection and content changes were sent as separate callbacks.
This caused consistency errors in observers, as the two must be processed
simultaneously.

TabStripModelObserver::OnTabStripChanged() carries selection and contents
data at the same time. And it'll be called after tab strip model finishes
it's work. So we don't have to care about model's state that much.

Thus, we can replace following with OnTabStripChanged():
  void TabInsertedAt(...);
  void TabReplacedAt(...);
  void TabMoved(...);
  void TabClosingAt(...);
  void TabDetachedAt(...);
  void TabDeactivated(...);
  void ActiveTabChanged(...);
  void TabSelectionChanged(...);

Also this callback allows us to handle multiple changes in a single
callback. e.g. multiple tab closing, insertion, etc.

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

chrome/browser/ui/tabs/tab_strip_model.h

@@ -431,14 +431,6 @@ class TabStripModel {
   struct DetachedWebContents;
   struct DetachNotifications;
 
-  // Used when making selection notifications.
-  enum class Notify {
-    kDefault,
-
-    // The selection is changing from a user gesture.
-    kUserGesture,
-  };
-
   // Performs all the work to detach a WebContents instance but avoids sending
   // most notifications. TabClosingAt() and TabDetachedAt() are sent because
   // observers are reliant on the selection model being accurate at the time
@@ -521,22 +513,26 @@ class TabStripModel {
       const std::vector<int>& indices);
 
   // Notifies the observers if the active tab has changed.
-  void NotifyIfActiveTabChanged(content::WebContents* old_contents,
-                                Notify notify_types);
+  void NotifyIfActiveTabChanged(const TabStripSelectionChange& selection);
 
   // Notifies the observers if the active tab or the tab selection has changed.
   // |old_model| is a snapshot of |selection_model_| before the change.
   // Note: This function might end up sending 0 to 2 notifications in the
   // following order: ActiveTabChanged, TabSelectionChanged.
   void NotifyIfActiveOrSelectionChanged(
-      content::WebContents* old_contents,
-      Notify notify_types,
-      const ui::ListSelectionModel& old_model);
+      const TabStripSelectionChange& selection);
 
   // Sets the selection to |new_model| and notifies any observers.
   // Note: This function might end up sending 0 to 3 notifications in the
   // following order: TabDeactivated, ActiveTabChanged, TabSelectionChanged.
-  void SetSelection(ui::ListSelectionModel new_model, Notify notify_types);
+  // |selection| will be filled with information corresponding to 3 notification
+  // above. When it's |triggered_by_other_operation|, This won't notify
+  // observers that selection was changed. Callers should notify it by
+  // themselves.
+  TabStripSelectionChange SetSelection(
+      ui::ListSelectionModel new_model,
+      TabStripModelObserver::ChangeReason reason,
+      bool triggered_by_other_operation);
 
   // Selects either the next tab (|forward| is true), or the previous tab
   // (|forward| is false).

chrome/browser/ui/tabs/tab_strip_model_observer.cc

@@ -6,9 +6,66 @@
 
 using content::WebContents;
 
+// static
+TabStripModelChange::Delta TabStripModelChange::CreateInsertDelta(
+    content::WebContents* contents,
+    int index) {
+  TabStripModelChange::Delta delta;
+  delta.insert = {contents, index};
+  return delta;
+}
+
+// static
+TabStripModelChange::Delta TabStripModelChange::CreateRemoveDelta(
+    content::WebContents* contents,
+    int index,
+    bool will_be_deleted) {
+  TabStripModelChange::Delta delta;
+  delta.remove = {contents, index, will_be_deleted};
+  return delta;
+}
+
+// static
+TabStripModelChange::Delta TabStripModelChange::CreateMoveDelta(
+    content::WebContents* contents,
+    int from_index,
+    int to_index) {
+  TabStripModelChange::Delta delta;
+  delta.move = {contents, from_index, to_index};
+  return delta;
+}
+
+// static
+TabStripModelChange::Delta TabStripModelChange::CreateReplaceDelta(
+    content::WebContents* old_contents,
+    content::WebContents* new_contents,
+    int index) {
+  TabStripModelChange::Delta delta;
+  delta.replace = {old_contents, new_contents, index};
+  return delta;
+}
+
+TabStripModelChange::TabStripModelChange(Type type, const Delta& delta)
+    : type_(type), deltas_({delta}) {}
+
+TabStripModelChange::TabStripModelChange(
+    TabStripModelChange::Type type,
+    const std::vector<TabStripModelChange::Delta>& deltas)
+    : type_(type), deltas_(deltas) {}
+
+TabStripModelChange::~TabStripModelChange() = default;
+
+TabStripModelChange::TabStripModelChange(TabStripModelChange&& other) = default;
+
+TabStripSelectionChange::TabStripSelectionChange() = default;
+
 TabStripModelObserver::TabStripModelObserver() {
 }
 
+void TabStripModelObserver::OnTabStripModelChanged(
+    const base::Optional<TabStripModelChange>& change,
+    const TabStripSelectionChange& selection) {}
+
 void TabStripModelObserver::TabInsertedAt(TabStripModel* tab_strip_model,
                                           WebContents* contents,
                                           int index,

chrome/browser/ui/tabs/tab_strip_model_observer.h

@@ -5,8 +5,12 @@
 #ifndef CHROME_BROWSER_UI_TABS_TAB_STRIP_MODEL_OBSERVER_H_
 #define CHROME_BROWSER_UI_TABS_TAB_STRIP_MODEL_OBSERVER_H_
 
+#include <vector>
+
 #include "base/macros.h"
+#include "base/optional.h"
 #include "chrome/browser/ui/tabs/tab_change_type.h"
+#include "ui/base/models/list_selection_model.h"
 
 class TabStripModel;
 
@@ -14,9 +18,119 @@ namespace content {
 class WebContents;
 }
 
-namespace ui {
-class ListSelectionModel;
-}
+////////////////////////////////////////////////////////////////////////////////
+//
+// TabStripModelChange / TabStripSelectionChange
+//
+// The following class and structures are used to inform TabStripModelObservers
+// of changes to:
+// 1) selection model
+// 2) activated tab
+// 3) inserted/removed/moved tabs.
+// These changes must be bundled together because (1) and (2) consist of indices
+// into a list of tabs [determined by (3)]. All three must be kept synchronized.
+//
+////////////////////////////////////////////////////////////////////////////////
+class TabStripModelChange {
+ public:
+  enum Type {
+    kInserted,
+    kRemoved,
+    kMoved,
+    kReplaced,
+  };
+
+  // A WebContents was inserted at |index|. This implicitly changes the existing
+  // selection model by calling IncrementFrom(index).
+  struct Insert {
+    content::WebContents* contents;
+    int index;
+  };
+
+  // A WebContents was removed at |index|. This implicitly changes the existing
+  // selection model by calling DecrementFrom(index).
+  struct Remove {
+    content::WebContents* contents;
+    int index;
+
+    // The specified WebContents at |index| is being closed (and eventually
+    // destroyed). |tab_strip_model| is the TabStripModel that contained the
+    // tab.
+    bool will_be_deleted;
+  };
+
+  // A WebContents was moved from |from_index| to |to_index|. This implicitly
+  // changes the existing selection model by calling
+  // Move(from_index, to_index, 1).
+  struct Move {
+    content::WebContents* contents;
+    int from_index;
+    int to_index;
+  };
+
+  // The WebContents was replaced at the specified index. This is invoked when
+  // prerendering swaps in a prerendered WebContents.
+  struct Replace {
+    content::WebContents* old_contents;
+    content::WebContents* new_contents;
+    int index;
+  };
+
+  struct Delta {
+    union {
+      Insert insert;
+      Remove remove;
+      Move move;
+      Replace replace;
+    };
+  };
+
+  // Convenient factory methods to create |Delta| with each member.
+  static Delta CreateInsertDelta(content::WebContents* contents, int index);
+  static Delta CreateRemoveDelta(content::WebContents* contents,
+                                 int index,
+                                 bool will_be_deleted);
+  static Delta CreateMoveDelta(content::WebContents* contents,
+                               int from_index,
+                               int to_index);
+  static Delta CreateReplaceDelta(content::WebContents* old_contents,
+                                  content::WebContents* new_contents,
+                                  int index);
+
+  TabStripModelChange(Type type, const Delta& delta);
+  TabStripModelChange(Type type, const std::vector<Delta>& deltas);
+  ~TabStripModelChange();
+
+  TabStripModelChange(TabStripModelChange&& other);
+
+  Type type() const { return type_; }
+  const std::vector<Delta>& deltas() const { return deltas_; }
+
+ private:
+  const Type type_;
+  const std::vector<Delta> deltas_;
+
+  DISALLOW_COPY_AND_ASSIGN(TabStripModelChange);
+};
+
+// Struct to carry changes on selection/activation.
+struct TabStripSelectionChange {
+  TabStripSelectionChange();
+
+  bool active_tab_changed() const { return old_contents != new_contents; }
+
+  // TODO(sangwoo.ko) Do we need something to indicate that the change
+  // was made implicitly?
+  bool selection_changed() const { return old_model != new_model; }
+
+  content::WebContents* old_contents = nullptr;
+  content::WebContents* new_contents = nullptr;
+
+  ui::ListSelectionModel old_model;
+  ui::ListSelectionModel new_model;
+
+  int reason = 0;
+};
 
 ////////////////////////////////////////////////////////////////////////////////
 //
@@ -52,9 +166,21 @@ class TabStripModelObserver {
     kCloseAllCompleted = 1,
   };
 
+  // |change| is a series of changes in tabtrip model. |change| consists
+  // of changes with same type and those changes may have caused selection or
+  // activation changes. |selection| is determined by comparing the state of
+  // TabStripModel before the |change| and after the |change| are applied.
+  // When only selection/activation was changed without any change about
+  // WebContents, |change| can be empty.
+  virtual void OnTabStripModelChanged(
+      const base::Optional<TabStripModelChange>& change,
+      const TabStripSelectionChange& selection);
+
   // A new WebContents was inserted into the TabStripModel at the
   // specified index. |foreground| is whether or not it was opened in the
   // foreground (selected).
+  // DEPRECATED, use OnTabStripChanged() above.
+  // TODO(crbug.com/842194): Delete this and migrate callsites.
   virtual void TabInsertedAt(TabStripModel* tab_strip_model,
                              content::WebContents* contents,
                              int index,
@@ -62,6 +188,7 @@ class TabStripModelObserver {
 
   // The specified WebContents at |index| is being closed (and eventually
   // destroyed). |tab_strip_model| is the TabStripModel that contained the tab.
+  // DEPRECATED, use OnTabStripChanged() above.
   // TODO(erikchen): |index| is not used outside of tests. Do not use it. It
   // will be removed soon. https://crbug.com/842194.
   virtual void TabClosingAt(TabStripModel* tab_strip_model,
@@ -73,6 +200,8 @@ class TabStripModelObserver {
   // action is necessary to deal with the WebContents no longer being present.
   // |previous_index| cannot be used to index into the current state of the
   // TabStripModel.
+  // DEPRECATED, use OnTabStripChanged() above.
+  // TODO(crbug.com/842194): Delete this and migrate callsites.
   virtual void TabDetachedAt(content::WebContents* contents,
                              int previous_index,
                              bool was_active);
@@ -80,6 +209,8 @@ class TabStripModelObserver {
   // The active WebContents is about to change from |old_contents|.
   // This gives observers a chance to prepare for an impending switch before it
   // happens.
+  // DEPRECATED, use OnTabStripChanged() above.
+  // TODO(crbug.com/842194): Delete this and migrate callsites.
   virtual void TabDeactivated(content::WebContents* contents);
 
   // Sent when the active tab changes. The previously active tab is identified
@@ -95,6 +226,8 @@ class TabStripModelObserver {
   // TabSelectionChanged.
   // Note: |old_contents| will be NULL if there was no contents previously
   // active.
+  // DEPRECATED, use OnTabStripChanged() above.
+  // TODO(crbug.com/842194): Delete this and migrate callsites.
   virtual void ActiveTabChanged(content::WebContents* old_contents,
                                 content::WebContents* new_contents,
                                 int index,
@@ -106,10 +239,14 @@ class TabStripModelObserver {
   // details.
   // TODO(erikchen): |old_model| is not used outside of tests. Do not use it. It
   // will be removed soon. https://crbug.com/842194.
+  // DEPRECATED, use OnTabStripChanged() above.
+  // TODO(crbug.com/842194): Delete this and migrate callsites.
   virtual void TabSelectionChanged(TabStripModel* tab_strip_model,
                                    const ui::ListSelectionModel& old_model);
 
   // The specified WebContents at |from_index| was moved to |to_index|.
+  // DEPRECATED, use OnTabStripChanged() above.
+  // TODO(crbug.com/842194): Delete this and migrate callsites.
   virtual void TabMoved(content::WebContents* contents,
                         int from_index,
                         int to_index);
@@ -125,6 +262,8 @@ class TabStripModelObserver {
 
   // The WebContents was replaced at the specified index. This is invoked when
   // prerendering swaps in a prerendered WebContents.
+  // DEPRECATED, use OnTabStripChanged() above.
+  // TODO(crbug.com/842194): Delete this and migrate callsites.
   virtual void TabReplacedAt(TabStripModel* tab_strip_model,
                              content::WebContents* old_contents,
                              content::WebContents* new_contents,