[Sessions] Add detailed logging of SessionRestore events.

chrome/browser/sessions/session_restore_delegate.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/sessions/session_restore_delegate.h"
 
 #include "base/metrics/field_trial.h"
 #include "chrome/browser/sessions/session_restore_stats_collector.h"
 #include "chrome/browser/sessions/tab_loader.h"
 #include "chrome/common/url_constants.h"
@@ skipping 47 matching lines @@
   if (is_app_ != right.is_app_)
     return is_app_;
   // TODO(georgesak): Add criterion based on recency.
   return false;
 }
 
 // static
 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.

       }
     }
   }
 }