Update SessionRestoreStatsCollector to use TabLoadTracker.

This is necessary in order for it to work when the PageAlmostIdle
feature is enabled.

BUG=829933

Change-Id: Ic3158a984713ceef849788eaa758ecaa255eba5a
Reviewed-on: https://chromium-review.googlesource.com/1097421Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Commit-Queue: Chris Hamilton <chrisha@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567484}

chrome/browser/sessions/session_restore_observer_unittest.cc

@@ -113,6 +113,10 @@ class SessionRestoreObserverTest : public ChromeRenderViewHostTestHarness {
   void LoadWebContents(content::WebContents* contents) {
     WebContentsTester::For(contents)->NavigateAndCommit(GURL(kDefaultUrl));
     WebContentsTester::For(contents)->TestSetIsLoading(false);
+    // Transition through LOADING to LOADED in order to keep the
+    // SessionRestoreStatsCollector state machine happy.
+    TabLoadTracker::Get()->TransitionStateForTesting(contents,
+                                                     TabLoadTracker::LOADING);
     TabLoadTracker::Get()->TransitionStateForTesting(contents,
                                                      TabLoadTracker::LOADED);
     mock_observer_.OnDidRestoreTab(contents);

chrome/browser/sessions/session_restore_stats_collector.h

@@ -12,6 +12,7 @@
 #include "base/callback_list.h"
 #include "base/macros.h"
 #include "base/time/tick_clock.h"
+#include "chrome/browser/resource_coordinator/tab_load_tracker.h"
 #include "chrome/browser/sessions/session_restore.h"
 #include "chrome/browser/sessions/session_restore_delegate.h"
 #include "content/public/browser/notification_observer.h"
@@ -47,8 +48,9 @@ class RenderWidgetHost;
 // presence of an unavailable network, or when tabs are closed during loading.
 // Rethink the collection in these cases.
 class SessionRestoreStatsCollector
-    : public content::NotificationObserver,
-      public base::RefCounted<SessionRestoreStatsCollector> {
+    : public base::RefCounted<SessionRestoreStatsCollector>,
+      public content::NotificationObserver,
+      public resource_coordinator::TabLoadTracker::Observer {
  public:
   // Houses all of the statistics gathered by the SessionRestoreStatsCollector
   // while the underlying TabLoader is active. These statistics are all reported
@@ -160,19 +162,24 @@ class SessionRestoreStatsCollector
 
   ~SessionRestoreStatsCollector() override;
 
-  // NotificationObserver method. This is the workhorse of the class and drives
-  // all state transitions.
+  // NotificationObserver method. Used for detecting first paint.
   void Observe(int type,
                const content::NotificationSource& source,
                const content::NotificationDetails& details) override;
 
-  // Called when a tab is no longer tracked. This is called by the 'Observe'
-  // notification callback. Takes care of unregistering all observers and
-  // removing the tab from all internal data structures.
+  // resource_coordinator::TabLoadTracker::Observer implementation:
+  void OnLoadingStateChange(content::WebContents* contents,
+                            LoadingState old_loading_state,
+                            LoadingState new_loading_state) override;
+  void OnStopTracking(content::WebContents* contents,
+                      LoadingState loading_state) override;
+
+  // Called when a tab is no longer tracked. Takes care of unregistering all
+  // observers and removing the tab from all internal data structures.
   void RemoveTab(content::NavigationController* tab);
 
   // Registers for relevant notifications for a tab and inserts the tab into
-  // to tabs_tracked_ map. Return a pointer to the newly created TabState.
+  // the |tabs_tracked_| map. Return a pointer to the newly created TabState.
   TabState* RegisterForNotifications(content::NavigationController* tab);
 
   // Returns the tab state, nullptr if not found.
@@ -182,6 +189,9 @@ class SessionRestoreStatsCollector
   // Marks a tab as loading.
   void MarkTabAsLoading(TabState* tab_state);
 
+  // Marks a tab as loaded.
+  void MarkTabAsLoaded(TabState* tab_state);
+
   // Checks to see if the SessionRestoreStatsCollector has finished collecting,
   // and if so, releases the self reference to the shared pointer.
   void ReleaseIfDoneTracking();

chrome/browser/sessions/session_restore_stats_collector_unittest.cc

@@ -10,6 +10,7 @@
 
 #include "base/macros.h"
 #include "base/test/simple_test_tick_clock.h"
+#include "chrome/browser/resource_coordinator/tab_helper.h"
 #include "chrome/test/base/testing_profile.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_entry.h"
@@ -24,6 +25,9 @@
 
 namespace {
 
+using resource_coordinator::TabLoadTracker;
+using resource_coordinator::ResourceCoordinatorTabHelper;
+
 using TabLoaderStats = SessionRestoreStatsCollector::TabLoaderStats;
 using StatsReportingDelegate =
     SessionRestoreStatsCollector::StatsReportingDelegate;
@@ -277,36 +281,32 @@ class SessionRestoreStatsCollectorTest : public testing::Test {
     // Create a last active time in the past.
     contents->SetLastActiveTime(base::TimeTicks::Now() -
                                 base::TimeDelta::FromMinutes(1));
+    // TabLoadTracker needs the resource_coordinator WebContentsData to be
+    // initialized.
+    ResourceCoordinatorTabHelper::CreateForWebContents(contents);
     restored_tabs_.push_back(RestoredTab(contents, is_active, false, false));
     if (is_active)
       Show(restored_tabs_.size() - 1);
   }
 
-  // Helper function for various notification generation.
-  void GenerateControllerNotification(size_t tab_index, int type) {
-    content::WebContents* contents = restored_tabs_[tab_index].contents();
-    content::NavigationController* controller = &contents->GetController();
-    stats_collector_->Observe(
-        type, content::Source<content::NavigationController>(controller),
-        content::NotificationService::NoDetails());
-  }
-
   // Generates a load start notification for the given tab.
   void GenerateLoadStart(size_t tab_index) {
-    GenerateControllerNotification(tab_index, content::NOTIFICATION_LOAD_START);
+    content::WebContents* contents = restored_tabs_[tab_index].contents();
+    TabLoadTracker::Get()->TransitionStateForTesting(contents,
+                                                     TabLoadTracker::LOADING);
   }
 
   // Generates a load stop notification for the given tab.
   void GenerateLoadStop(size_t tab_index) {
-    GenerateControllerNotification(tab_index, content::NOTIFICATION_LOAD_STOP);
+    content::WebContents* contents = restored_tabs_[tab_index].contents();
+    TabLoadTracker::Get()->TransitionStateForTesting(contents,
+                                                     TabLoadTracker::LOADED);
   }
 
   // Generates a web contents destroyed notification for the given tab.
   void GenerateWebContentsDestroyed(size_t tab_index) {
     content::WebContents* contents = restored_tabs_[tab_index].contents();
-    stats_collector_->Observe(content::NOTIFICATION_WEB_CONTENTS_DESTROYED,
-                              content::Source<content::WebContents>(contents),
-                              content::NotificationService::NoDetails());
+    test_web_contents_factory_->DestroyWebContents(contents);
   }
 
   // Generates a paint notification for the given tab.

chrome/browser/sessions/tab_loader_unittest.cc

@@ -69,8 +69,13 @@ class TabLoaderTest : public testing::Test {
   }
 
   void SimulateLoaded(size_t tab_index) {
-    TabLoadTracker::Get()->TransitionStateForTesting(
-        restored_tabs_[tab_index].contents(), TabLoadTracker::LOADED);
+    // Transition to a LOADED state. This has to pass through the LOADING state
+    // in order to satisfy the internal logic of SessionRestoreStatsCollector.
+    auto* contents = restored_tabs_[tab_index].contents();
+    auto* tracker = TabLoadTracker::Get();
+    if (tracker->GetLoadingState(contents) != TabLoadTracker::LOADING)
+      tracker->TransitionStateForTesting(contents, TabLoadTracker::LOADING);
+    tracker->TransitionStateForTesting(contents, TabLoadTracker::LOADED);
   }
 
   void SimulateLoadedAll() {

content/public/test/test_web_contents_factory.h

@@ -36,6 +36,9 @@ class TestWebContentsFactory {
   // Ownership remains with the TestWebContentsFactory.
   WebContents* CreateWebContents(BrowserContext* context);
 
+  // Destroys the provided WebContents.
+  void DestroyWebContents(WebContents* contents);
+
  private:
   // The test factory (and friends) for creating test web contents.
   std::unique_ptr<RenderViewHostTestEnabler> rvh_enabler_;

content/test/test_web_contents_factory.cc

@@ -36,4 +36,15 @@ WebContents* TestWebContentsFactory::CreateWebContents(
   return web_contents_.back().get();
 }
 
+void TestWebContentsFactory::DestroyWebContents(WebContents* contents) {
+  auto it = web_contents_.begin();
+  for (; it != web_contents_.end(); ++it) {
+    if (it->get() == contents)
+      break;
+  }
+  if (it == web_contents_.end())
+    return;
+  web_contents_.erase(it);
+}
+
 }  // namespace content

third_party/blink/renderer/platform/scheduler/main_thread/main_thread_metrics_helper.cc

@@ -79,6 +79,7 @@ MainThreadMetricsHelper::MainThreadMetricsHelper(
     bool renderer_backgrounded)
     : MetricsHelper(WebThreadType::kMainThread),
       main_thread_scheduler_(main_thread_scheduler),
+      renderer_shutting_down_(false),
       is_page_almost_idle_signal_enabled_(
           ::resource_coordinator::IsPageAlmostIdleSignalEnabled()),
       main_thread_load_tracker_(
@@ -134,6 +135,7 @@ void MainThreadMetricsHelper::OnRendererBackgrounded(base::TimeTicks now) {
 }
 
 void MainThreadMetricsHelper::OnRendererShutdown(base::TimeTicks now) {
+  renderer_shutting_down_ = true;
   foreground_main_thread_load_tracker_.RecordIdle(now);
   background_main_thread_load_tracker_.RecordIdle(now);
   main_thread_load_tracker_.RecordIdle(now);
@@ -491,6 +493,13 @@ void MainThreadMetricsHelper::ReportLowThreadLoadForPageAlmostIdleSignal(
   if (!is_page_almost_idle_signal_enabled_)
     return;
 
+  // Avoid sending IPCs when the renderer is shutting down as this wreaks havoc
+  // in test harnesses. These messages aren't needed in production code either
+  // as the endpoint receiving them dies shortly after and does nothing with
+  // them.
+  if (renderer_shutting_down_)
+    return;
+
   static const int main_thread_task_load_low_threshold =
       ::resource_coordinator::GetMainThreadTaskLoadLowThreshold();
 

third_party/blink/renderer/platform/scheduler/main_thread/main_thread_metrics_helper.h

@@ -55,6 +55,11 @@ class PLATFORM_EXPORT MainThreadMetricsHelper : public MetricsHelper {
 
   MainThreadSchedulerImpl* main_thread_scheduler_;  // NOT OWNED
 
+  // Set to true when OnRendererShutdown is called. Used to ensure that metrics
+  // that need to cross IPC boundaries aren't sent, as they cause additional
+  // useless tasks to be posted.
+  bool renderer_shutting_down_;
+
   const bool is_page_almost_idle_signal_enabled_;
 
   base::Optional<base::TimeTicks> last_reported_task_;