[Tab Metrics] Measure FP, FCP and FMP for Foreground Tab during Session Restore

chrome/browser/resource_coordinator/tab_manager.cc

 // Copyright (c) 2012 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/resource_coordinator/tab_manager.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <set>
@@ skipping 94 matching lines @@
 TabManager::TabManager()
     : discard_count_(0),
       recent_tab_discard_(false),
       discard_once_(false),
 #if !defined(OS_CHROMEOS)
       minimum_protection_time_(base::TimeDelta::FromMinutes(10)),
 #endif
       browser_tab_strip_tracker_(this, nullptr, nullptr),
       test_tick_clock_(nullptr),
       in_session_restore_(false),
+      initial_active_tab_changed_(false),
       weak_ptr_factory_(this) {
 #if defined(OS_CHROMEOS)
   delegate_.reset(new TabManagerDelegate(weak_ptr_factory_.GetWeakPtr()));
 #endif
   browser_tab_strip_tracker_.Init();
 }
 
 TabManager::~TabManager() {
   Stop();
 }
@@ skipping 661 matching lines @@
   // When ActiveTabChanged, |new_contents| purged state changes to be false.
   GetWebContentsData(new_contents)->set_is_purged(false);
   // If |old_contents| is set, that tab has switched from being active to
   // inactive, so record the time of that transition.
   if (old_contents) {
     GetWebContentsData(old_contents)->SetLastInactiveTime(NowTicks());
     // Re-setting time-to-purge every time a tab becomes inactive.
     GetWebContentsData(old_contents)
         ->set_time_to_purge(GetTimeToPurge(min_time_to_purge_));
   }
+
+  if (old_contents && !initial_active_tab_changed_) {
+    initial_active_tab_changed_ = true;

[chrisha, 2017/06/21 15:00:26]

This logic seems frail. If this is specifically tied to session restore, then it
seems like we should be resetting this when OnSessionRestoreStarted is called,
so that it tracks the initial foreground tab associated with a session restore.

Note that session restore can be invoked at anytime, not just at browser
startup.

The more I think about this, the more I like sky's idea of session restore
itself exposing an observer interface, and then tracking all of this state
directly in the PageLoadMetricObserver, which would also be a
SessionRestoreObserver and a TabStripModelObserver. Why do we need TabManager as
the middle man? Other than for these metrics, TabManager doesn't explicitly care
that the initially visible tab related to a session restore has since been
changed away from...

The session restore observer should be able to share everything about an
individual session restore, including when it starts, the tabs it is restoring
(either as WebContents, or TabStripModels and tab IDs), and when it ends.

There is already something like this: SessionRestoreStatsCollector. We could be
adding these metrics directly to that class, or alternatively, abstracting out a
SessionRestoreObserver helper from that class, and having two separate clients.

[ducbui, 2017/06/27 19:41:24]

Acknowledged. I reset the flag for each session restore.

+  }
 }
 
 void TabManager::TabInsertedAt(TabStripModel* tab_strip_model,
                                content::WebContents* contents,
                                int index,
                                bool foreground) {
   // Only interested in background tabs, as foreground tabs get taken care of by
   // ActiveTabChanged.
   if (foreground)
     return;
@@ skipping 70 matching lines @@
   // platform.
   std::string allow_multiple_discards = variations::GetVariationParamValue(
       features::kAutomaticTabDiscarding.name, "AllowMultipleDiscards");
   return (allow_multiple_discards != "true");
 #else
   return false;
 #endif
 }
 
 }  // namespace resource_coordinator