Record the number of tabs in a session restore in SampledProfile

The OnSessionRestore callback has been changed to accept the number
of tabs as an argument.

BUG=chromium:455482
TEST=Add logging to make sure the correct number of tabs is being
passed to perf provider during session restore
Signed-off-by: Simon Que <sque@chromium.org>

Review URL: https://codereview.chromium.org/902613003

Cr-Commit-Position: refs/heads/master@{#318168}

chrome/browser/chrome_browser_main_posix.cc

@@ -96,7 +96,7 @@ class ExitHandler {
   ~ExitHandler();
 
   // Called when a session restore has finished.
-  void OnSessionRestoreDone();
+  void OnSessionRestoreDone(int num_tabs_restored);
 
   // Does the appropriate call to Exit.
   static void Exit();
@@ -133,7 +133,7 @@ ExitHandler::ExitHandler() {
 ExitHandler::~ExitHandler() {
 }
 
-void ExitHandler::OnSessionRestoreDone() {
+void ExitHandler::OnSessionRestoreDone(int /* num_tabs */) {
   if (!SessionRestore::IsRestoringSynchronously()) {
     // At this point the message loop may not be running (meaning we haven't
     // gotten through browser startup, but are close). Post the task to at which

chrome/browser/extensions/state_store_notification_observer.cc

@@ -23,7 +23,8 @@ StateStoreNotificationObserver::StateStoreNotificationObserver(
 StateStoreNotificationObserver::~StateStoreNotificationObserver() {
 }
 
-void StateStoreNotificationObserver::OnSessionRestoreDone() {
+void StateStoreNotificationObserver::OnSessionRestoreDone(
+    int /* num_tabs_restored */) {
   on_session_restored_callback_subscription_.reset();
   state_store_->RequestInitAfterDelay();
 }

chrome/browser/extensions/state_store_notification_observer.h

@@ -22,7 +22,7 @@ class StateStoreNotificationObserver {
 
  private:
   // Called when a session restore has finished.
-  void OnSessionRestoreDone();
+  void OnSessionRestoreDone(int num_tabs_restored);
 
   StateStore* state_store_;  // Not owned.
 

chrome/browser/metrics/perf_provider_chromeos.cc

@@ -210,7 +210,7 @@ void PerfProvider::SuspendDone(const base::TimeDelta& sleep_duration) {
                           collection_delay));
 }
 
-void PerfProvider::OnSessionRestoreDone() {
+void PerfProvider::OnSessionRestoreDone(int num_tabs_restored) {
   // Do not collect a profile unless logged in as a normal user.
   if (!IsNormalUserLoggedIn())
     return;
@@ -245,7 +245,8 @@ void PerfProvider::OnSessionRestoreDone() {
       collection_delay,
       base::Bind(&PerfProvider::CollectPerfDataAfterSessionRestore,
                  weak_factory_.GetWeakPtr(),
-                 collection_delay));
+                 collection_delay,
+                 num_tabs_restored));
 }
 
 void PerfProvider::OnUserLoggedIn() {
@@ -347,11 +348,13 @@ void PerfProvider::CollectPerfDataAfterResume(
 }
 
 void PerfProvider::CollectPerfDataAfterSessionRestore(
-    const base::TimeDelta& time_after_restore) {
+    const base::TimeDelta& time_after_restore,
+    int num_tabs_restored) {
   // Fill out a SampledProfile protobuf that will contain the collected data.
   scoped_ptr<SampledProfile> sampled_profile(new SampledProfile);
   sampled_profile->set_trigger_event(SampledProfile::RESTORE_SESSION);
   sampled_profile->set_ms_after_restore(time_after_restore.InMilliseconds());
+  sampled_profile->set_num_tabs_restored(num_tabs_restored);
 
   CollectIfNecessary(sampled_profile.Pass());
   last_session_restore_collection_time_ = base::TimeTicks::Now();

chrome/browser/metrics/perf_provider_chromeos.h

@@ -61,7 +61,7 @@ class PerfProvider : public base::NonThreadSafe,
   void OnUserLoggedIn();
 
   // Called when a session restore has finished.
-  void OnSessionRestoreDone();
+  void OnSessionRestoreDone(int num_tabs_restored);
 
   // Turns off perf collection. Does not delete any data that was already
   // collected and stored in |cached_perf_data_|.
@@ -87,9 +87,11 @@ class PerfProvider : public base::NonThreadSafe,
                                   const base::TimeDelta& time_after_resume);
 
   // Collects perf data after a session restore. |time_after_restore| is how
-  // long ago the session restore started.
+  // long ago the session restore started. |num_tabs_restored| is the total
+  // number of tabs being restored.
   void CollectPerfDataAfterSessionRestore(
-      const base::TimeDelta& time_after_restore);
+      const base::TimeDelta& time_after_restore,
+      int num_tabs_restored);
 
   // Parses a perf data protobuf from the |data| passed in only if the
   // |incognito_observer| indicates that no incognito window had been opened

chrome/browser/sessions/session_restore.cc

@@ -335,7 +335,7 @@ void TabLoader::LoadNextTab() {
   // When the session restore is done synchronously, notification is sent from
   // SessionRestoreImpl::Restore .
   if (tabs_to_load_.empty() && !SessionRestore::IsRestoringSynchronously()) {
-    on_session_restored_callbacks_->Notify();
+    on_session_restored_callbacks_->Notify(tab_count_);
   }
 }
 
@@ -641,8 +641,13 @@ class SessionRestoreImpl : public content::NotificationObserver {
         loop.Run();
         quit_closure_for_sync_restore_ = base::Closure();
       }
+      // Count the total number of tabs in |windows_|.
+      int total_num_tabs = 0;
+      for (int i = 0; i < static_cast<int>(windows_.size()); ++i)
+        total_num_tabs += windows_[i]->tabs.size();
+
       Browser* browser = ProcessSessionWindows(&windows_, active_window_id_);
-      on_session_restored_callbacks_->Notify();
+      on_session_restored_callbacks_->Notify(total_num_tabs);
       delete this;
       return browser;
     }
@@ -1354,10 +1359,10 @@ bool SessionRestore::IsRestoringSynchronously() {
 // static
 SessionRestore::CallbackSubscription
     SessionRestore::RegisterOnSessionRestoredCallback(
-        const base::Closure& callback) {
+        const base::Callback<void(int)>& callback) {
   return on_session_restored_callbacks()->Add(callback);
 }
 
 // static
-base::CallbackList<void(void)>*
+base::CallbackList<void(int)>*
     SessionRestore::on_session_restored_callbacks_ = nullptr;

chrome/browser/sessions/session_restore.h

@@ -40,12 +40,12 @@ class SessionRestore {
   };
 
   // Notification callback list.
-  using CallbackList = base::CallbackList<void(void)>;
+  using CallbackList = base::CallbackList<void(int)>;
 
   // Used by objects calling RegisterOnSessionRestoredCallback() to de-register
   // themselves when they are destroyed.
   using CallbackSubscription =
-      scoped_ptr<base::CallbackList<void(void)>::Subscription>;
+      scoped_ptr<base::CallbackList<void(int)>::Subscription>;
 
   // Restores the last session. |behavior| is a bitmask of Behaviors, see it
   // for details. If |browser| is non-null the tabs for the first window are
@@ -92,9 +92,20 @@ class SessionRestore {
   static bool IsRestoringSynchronously();
 
   // Register callbacks for session restore events. These callbacks are stored
-  // in on_session_restored_callbacks_.
+  // in |on_session_restored_callbacks_|.
+  // The callback is supplied an integer arg representing a tab count. The exact
+  // meaning and timing depend upon the restore type:
+  // - SessionRestore::SYNCHRONOUS: the parameter is the number of tabs that
+  // were created. Additionally the callback is invoked immediately after the
+  // tabs have been created. That is, the tabs are not necessarily loading.
+  // - For all other restore types the parameter is the number of tabs that were
+  // restored and is sent after all tabs have started loading. Additionally if a
+  // request to restore tabs comes in while a previous request to restore tabs
+  // has not yet completed (loading tabs is throttled), then the callback is
+  // only notified once both sets of tabs have started loading and with the
+  // total number of tabs for both restores.
   static CallbackSubscription RegisterOnSessionRestoredCallback(
-      const base::Closure& callback);
+      const base::Callback<void(int)>& callback);
 
   // The max number of non-selected tabs SessionRestore loads when restoring
   // a session. A value of 0 indicates all tabs are loaded at once.

chrome/browser/sessions/session_restore_test_helper.cc

@@ -29,7 +29,8 @@ void SessionRestoreTestHelper::Wait() {
   EXPECT_TRUE(restore_notification_seen_);
 }
 
-void SessionRestoreTestHelper::OnSessionRestoreDone() {
+void SessionRestoreTestHelper::OnSessionRestoreDone(
+    int /* num_tabs_restored */) {
   restore_notification_seen_ = true;
   if (!loop_is_running_)
     return;

chrome/browser/sessions/session_restore_test_helper.h

@@ -27,7 +27,7 @@ class SessionRestoreTestHelper {
 
  private:
   // Callback for session restore notifications.
-  void OnSessionRestoreDone();
+  void OnSessionRestoreDone(int /* num_tabs_restored */);
 
   // Indicates whether a session restore notification has been received.
   bool restore_notification_seen_;