RC: Add tracking of UI tab count to TabLoadTracker

Tab helpers are attached to prerender web contents, and as a result
TabLoadTracker counts these web contentses as tabs. This CL adds
additional tracking of non-UI tabs and adds Get*TabUiCount() methods
that exclude non-UI tabs from counts.

This CL also modifies CoreTabHelperDelegates to notify TabLoadTracker
when tab contents are swapped. In that case, non-UI tabs are promoted to
UI tabs.

Bug: 861800
Change-Id: I3666b17634efd1445cd24cade8514fee06ad2c92
Reviewed-on: https://chromium-review.googlesource.com/1134504Reviewed-by: Chris Hamilton <chrisha@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Commit-Queue: Scott Haseley <shaseley@google.com>
Cr-Commit-Position: refs/heads/master@{#577886}

chrome/browser/android/tab_android.cc

@@ -33,6 +33,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_android.h"
 #include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/resource_coordinator/tab_load_tracker.h"
 #include "chrome/browser/sessions/session_tab_helper.h"
 #include "chrome/browser/sessions/tab_restore_service_factory.h"
 #include "chrome/browser/sync/glue/synced_tab_delegate_android.h"
@@ -343,6 +344,11 @@ std::unique_ptr<content::WebContents> TabAndroid::SwapTabContents(
     std::unique_ptr<content::WebContents> new_contents,
     bool did_start_load,
     bool did_finish_load) {
+  // TODO(crbug.com/836409): TabLoadTracker should not rely on being notified
+  // directly about tab contents swaps.
+  resource_coordinator::TabLoadTracker::Get()->SwapTabContents(
+      old_contents, new_contents.get());
+
   JNIEnv* env = base::android::AttachCurrentThread();
   Java_Tab_swapWebContents(env, weak_java_tab_.get(env),
                            new_contents->GetJavaWebContents(), did_start_load,

chrome/browser/resource_coordinator/tab_load_tracker.cc

@@ -4,8 +4,11 @@
 
 #include "chrome/browser/resource_coordinator/tab_load_tracker.h"
 
+#include <utility>
+
+#include "base/logging.h"
 #include "base/stl_util.h"
-#include "chrome/browser/ui/browser_tab_strip_tracker.h"
+#include "chrome/browser/prerender/prerender_contents.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/web_contents.h"
@@ -66,6 +69,33 @@ size_t TabLoadTracker::GetLoadedTabCount() const {
   return state_counts_[static_cast<size_t>(LOADED)];
 }
 
+size_t TabLoadTracker::GetUiTabCount() const {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  return ui_tab_state_counts_[static_cast<size_t>(UNLOADED)] +
+         ui_tab_state_counts_[static_cast<size_t>(LOADING)] +
+         ui_tab_state_counts_[static_cast<size_t>(LOADED)];
+}
+
+size_t TabLoadTracker::GetUiTabCount(LoadingState loading_state) const {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  return ui_tab_state_counts_[static_cast<size_t>(loading_state)];
+}
+
+size_t TabLoadTracker::GetUnloadedUiTabCount() const {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  return ui_tab_state_counts_[static_cast<size_t>(UNLOADED)];
+}
+
+size_t TabLoadTracker::GetLoadingUiTabCount() const {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  return ui_tab_state_counts_[static_cast<size_t>(LOADING)];
+}
+
+size_t TabLoadTracker::GetLoadedUiTabCount() const {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  return ui_tab_state_counts_[static_cast<size_t>(LOADED)];
+}
+
 void TabLoadTracker::AddObserver(Observer* observer) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   observers_.AddObserver(observer);
@@ -98,8 +128,11 @@ void TabLoadTracker::StartTracking(content::WebContents* web_contents) {
   data.loading_state = loading_state;
   if (data.loading_state == LOADING)
     data.did_start_loading_seen = true;
+  data.is_ui_tab = IsUiTab(web_contents);
   tabs_.insert(std::make_pair(web_contents, data));
   ++state_counts_[static_cast<size_t>(data.loading_state)];
+  if (data.is_ui_tab)
+    ++ui_tab_state_counts_[static_cast<size_t>(data.loading_state)];
 
   for (Observer& observer : observers_)
     observer.OnStartTracking(web_contents, loading_state);
@@ -113,6 +146,12 @@ void TabLoadTracker::StopTracking(content::WebContents* web_contents) {
   auto loading_state = it->second.loading_state;
   DCHECK_NE(0u, state_counts_[static_cast<size_t>(it->second.loading_state)]);
   --state_counts_[static_cast<size_t>(it->second.loading_state)];
+  if (it->second.is_ui_tab) {
+    DCHECK_NE(
+        0u,
+        ui_tab_state_counts_[static_cast<size_t>(it->second.loading_state)]);
+    --ui_tab_state_counts_[static_cast<size_t>(it->second.loading_state)];
+  }
   tabs_.erase(it);
 
   for (Observer& observer : observers_)
@@ -260,6 +299,11 @@ void TabLoadTracker::TransitionState(TabMap::iterator it,
   --state_counts_[static_cast<size_t>(previous_state)];
   it->second.loading_state = loading_state;
   ++state_counts_[static_cast<size_t>(loading_state)];
+  if (it->second.is_ui_tab) {
+    ++ui_tab_state_counts_[static_cast<size_t>(loading_state)];
+    DCHECK_NE(0u, ui_tab_state_counts_[static_cast<size_t>(previous_state)]);
+    --ui_tab_state_counts_[static_cast<size_t>(previous_state)];
+  }
 
   // If the destination state is LOADED, then also clear the
   // |did_start_loading_seen| state.
@@ -274,6 +318,53 @@ void TabLoadTracker::TransitionState(TabMap::iterator it,
     observer.OnLoadingStateChange(web_contents, previous_state, loading_state);
 }
 
+bool TabLoadTracker::IsUiTab(content::WebContents* web_contents) {
+  // TODO(crbug.com/836409): This should be able to check directly with the
+  // tabstrip UI or use a platform-independent tabstrip observer interface to
+  // learn about |web_contents| associated with the tabstrip, rather than
+  // checking for specific cases where |web_contents| is not a ui tab.
+  if (prerender::PrerenderContents::FromWebContents(web_contents) != nullptr)
+    return false;
+  return true;
+}
+
+void TabLoadTracker::SwapTabContents(content::WebContents* old_contents,
+                                     content::WebContents* new_contents) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  // TODO(crbug.com/836409): This should work by directly tracking tabs that are
+  // attached to UI surfaces instead of relying on being notified directly about
+  // tab contents swaps.
+
+  // Transition |old_contents| to a non-UI tab. If a tab is being swapped out,
+  // then it should exist, we should be tracking it, and it should be a UI tab.
+  DCHECK(old_contents);
+  auto it = tabs_.find(old_contents);
+  DCHECK(it != tabs_.end());
+  DCHECK(it->second.is_ui_tab);
+  it->second.is_ui_tab = false;
+  DCHECK_NE(
+      0u, ui_tab_state_counts_[static_cast<size_t>(it->second.loading_state)]);
+  --ui_tab_state_counts_[static_cast<size_t>(it->second.loading_state)];
+
+  // Transition |new_contents| to a UI tab.
+  DCHECK(IsUiTab(new_contents));
+  it = tabs_.find(new_contents);
+  // |new_contents| will not be tracked if a tab helper wasn't attached yet,
+  // which currently happens for dom distiller. In this case, the tab helper
+  // will be attached and we will start tracking it when it's swapped into the
+  // tab UI, which will happen later in this code path.
+  if (it == tabs_.end())
+    return;
+
+  // |new_contents| shouldn't be considered a UI tab yet. This should catch any
+  // new cases of non-tab web contents that attach tab helpers that we aren't
+  // handling.
+  DCHECK(!it->second.is_ui_tab);
+  // Promote |new_contents| to a UI tab.
+  it->second.is_ui_tab = true;
+  ++ui_tab_state_counts_[static_cast<size_t>(it->second.loading_state)];
+}
+
 TabLoadTracker::Observer::Observer() {}
 
 TabLoadTracker::Observer::~Observer() {}

chrome/browser/resource_coordinator/tab_load_tracker.h

@@ -90,6 +90,18 @@ class TabLoadTracker {
   size_t GetLoadingTabCount() const;
   size_t GetLoadedTabCount() const;
 
+  // Returns the total number of UI tabs that are being tracked by this class.
+  // Some WebContents being tracked by this class may not yet be associated with
+  // a UI tab, e.g. prerender contents. To exclude these tabs from counts, use
+  // the Get*UiTabCount() methods.
+  size_t GetUiTabCount() const;
+
+  // Returns the number of UI tabs in each state.
+  size_t GetUiTabCount(LoadingState loading_state) const;
+  size_t GetUnloadedUiTabCount() const;
+  size_t GetLoadingUiTabCount() const;
+  size_t GetLoadedUiTabCount() const;
+
   // Adds/removes an observer. It is up to the observer to ensure their lifetime
   // exceeds that of the TabLoadTracker, as is removed prior to its destruction.
   void AddObserver(Observer* observer);
@@ -99,6 +111,11 @@ class TabLoadTracker {
   void TransitionStateForTesting(content::WebContents* web_contents,
                                  LoadingState loading_state);
 
+  // Called from CoreTabHelperDelegates when |new_contents| is replacing
+  // |old_contents| in a tab.
+  void SwapTabContents(content::WebContents* old_contents,
+                       content::WebContents* new_contents);
+
  protected:
   // This allows the singleton constructor access to the protected constructor.
   friend class base::NoDestructor<TabLoadTracker>;
@@ -141,6 +158,14 @@ class TabLoadTracker {
   // from the TabManager.
   void OnPageAlmostIdle(content::WebContents* web_contents);
 
+  // Returns true if |web_contents| is a UI tab and false otherwise. This is
+  // used to filter out cases where tab helpers are attached to a non-UI tab
+  // WebContents, e.g prerender contents.
+  //
+  // This is virtual and protected for unittesting to control when web
+  // contentses are considered ui tabs.
+  virtual bool IsUiTab(content::WebContents* web_contents);
+
  private:
   // For unittesting.
   friend class TestTabLoadTracker;
@@ -149,6 +174,7 @@ class TabLoadTracker {
   struct WebContentsData {
     LoadingState loading_state = LoadingState::UNLOADED;
     bool did_start_loading_seen = false;
+    bool is_ui_tab = false;
   };
 
   using TabMap = base::flat_map<content::WebContents*, WebContentsData>;
@@ -168,12 +194,17 @@ class TabLoadTracker {
                        LoadingState loading_state,
                        bool validate_transition);
 
-  // The list of known WebContents and their states.
+  // The list of known WebContents and their states. This includes both UI and
+  // non-UI tabs.
   TabMap tabs_;
 
   // The counts of tabs in each state.
   size_t state_counts_[static_cast<size_t>(LoadingState::kMaxValue) + 1] = {0};
 
+  // The counts of UI tabs in each state.
+  size_t ui_tab_state_counts_[static_cast<size_t>(LoadingState::kMaxValue) +
+                              1] = {0};
+
   base::ObserverList<Observer> observers_;
 
   SEQUENCE_CHECKER(sequence_checker_);

chrome/browser/ui/browser.cc

@@ -78,6 +78,7 @@
 #include "chrome/browser/profiles/profile_metrics.h"
 #include "chrome/browser/profiles/profiles_state.h"
 #include "chrome/browser/repost_form_warning_controller.h"
+#include "chrome/browser/resource_coordinator/tab_load_tracker.h"
 #include "chrome/browser/resource_coordinator/tab_manager_web_contents_data.h"
 #include "chrome/browser/search/search.h"
 #include "chrome/browser/sessions/session_restore.h"
@@ -2003,6 +2004,11 @@ std::unique_ptr<content::WebContents> Browser::SwapTabContents(
       new_view->TakeFallbackContentFrom(old_view);
   }
 
+  // TODO(crbug.com/836409): TabLoadTracker should not rely on being notified
+  // directly about tab contents swaps.
+  resource_coordinator::TabLoadTracker::Get()->SwapTabContents(
+      old_contents, new_contents.get());
+
   int index = tab_strip_model_->GetIndexOfWebContents(old_contents);
   DCHECK_NE(TabStripModel::kNoTab, index);
   return tab_strip_model_->ReplaceWebContentsAt(index, std::move(new_contents));