[Sessions] Add detailed logging of SessionRestore events.

chrome/browser/sessions/session_restore_delegate.cc

Index: chrome/browser/sessions/session_restore_delegate.cc
diff --git a/chrome/browser/sessions/session_restore_delegate.cc b/chrome/browser/sessions/session_restore_delegate.cc
index 9d5c3122709459dff1f353967f51ca45e44af392..beb4e256ecfa77c1b1376047aece64c44ed3b904 100644
--- a/chrome/browser/sessions/session_restore_delegate.cc
+++ b/chrome/browser/sessions/session_restore_delegate.cc
@@ -65,22 +65,32 @@ bool SessionRestoreDelegate::RestoredTab::operator<(
 void SessionRestoreDelegate::RestoreTabs(
     const std::vector<RestoredTab>& tabs,
     const base::TimeTicks& restore_started) {
+  SessionRestoreStatsCollector::TrackTabs(tabs, restore_started);
+
+  // Restore the favicon for all tabs. Any tab may end up being deferred due
+  // to memory pressure so it's best to have some visual indication of its
+  // contents.
+  for (auto& restored_tab : tabs) {
+    // Restore the favicon for deferred tabs.
+    favicon::ContentFaviconDriver* favicon_driver =
+        favicon::ContentFaviconDriver::FromWebContents(
+            restored_tab.contents());
+    favicon_driver->FetchFavicon(favicon_driver->GetActiveURL());
+  }
+
   // This experiment allows us to have comparative numbers for session restore
   // metrics. It will be removed once those numbers are obtained.
   // TODO(georgesak): Remove this experiment when stats are collected.
   base::FieldTrial* trial =
       base::FieldTrialList::Find("IntelligentSessionRestore");
   if (!trial || trial->group_name() != "DontRestoreBackgroundTabs") {
-    SessionRestoreStatsCollector::TrackTabs(tabs, restore_started);
     TabLoader::RestoreTabs(tabs, restore_started);
   } else {
-    SessionRestoreStatsCollector::TrackActiveTabs(tabs, restore_started);
     for (auto& restored_tab : tabs) {
       if (!restored_tab.is_active()) {
-        favicon::ContentFaviconDriver* favicon_driver =
-            favicon::ContentFaviconDriver::FromWebContents(
-                restored_tab.contents());
-        favicon_driver->FetchFavicon(favicon_driver->GetActiveURL());
+        // Non-active tabs aren't being loaded, so mark them as deferred.
+        auto tab_controller = &restored_tab.contents()->GetController();
+        SessionRestoreStatsCollector::DeferTab(tab_controller);

[sky, 2015/05/14 15:47:04]

I would rather not expose a bunch of static functions on
SessionRestoreStatsController. It starts getting harder to reason and know about
state. Could you pass in the set of deferred tabs to TrackTabs? Just one
function that way.

[chrisha, 2015/05/14 21:27:02]

This is an edge case. When memory pressure fires the tabs are already being
tracked (already having been submitted to the collector via TrackTabs). We need
a mechanism to inform the stats collector that they are deliberately *not* being
loaded, and an additional static function seems the easiest way to do this.

The above code is only for the experiment, and simply reuses that function. This
is an edge case in that TrackTabs and DeferTab are being called back-to-back,
but in intended real usage of this, DeferTab will be called with a time delay
after TrackTabs.

We *could* augment RestoredTab with an 'is_deferred' state, and simply resubmit
the list to TrackTabs. Any preference?

[sky, 2015/05/14 21:51:50]

I don't think deferred belongs in RestoredTab as it makes no sense at the call
site creating it. I prefer having TrackTabs take a separate vector of deferred
tabs.

       }
     }
   }