RC: Start measurement on PageAlmostIdle.

This way there's a consistent measurement point, irrespecive the
setting of how TabLoadTracker assesses the loaded state.

Bug: 755840
Change-Id: I42790bff863e1787db2ac804eecd117b185b5afb
Reviewed-on: https://chromium-review.googlesource.com/1119062Reviewed-by: François Doray <fdoray@chromium.org>
Commit-Queue: Sigurður Ásgeirsson <siggi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572218}

chrome/browser/resource_coordinator/performance_measurement_manager.cc

@@ -12,27 +12,23 @@ namespace resource_coordinator {
 using LoadingState = TabLoadTracker::LoadingState;
 
 PerformanceMeasurementManager::PerformanceMeasurementManager(
-    TabLoadTracker* tab_load_tracker,
+    PageSignalReceiver* page_signal_receiver,
     RenderProcessProbe* render_process_probe)
-    : scoped_observer_(this), render_process_probe_(render_process_probe) {
-  scoped_observer_.Add(tab_load_tracker);
+    : scoped_observer_(this),
+      page_signal_receiver_(page_signal_receiver),
+      render_process_probe_(render_process_probe) {
+  scoped_observer_.Add(page_signal_receiver);
 }
 
 PerformanceMeasurementManager::~PerformanceMeasurementManager() = default;
 
-void PerformanceMeasurementManager::OnStartTracking(
+void PerformanceMeasurementManager::OnPageAlmostIdle(
     content::WebContents* web_contents,
-    LoadingState loading_state) {
-  if (loading_state == LoadingState::LOADED)
-    render_process_probe_->StartSingleGather();
-}
-
-void PerformanceMeasurementManager::OnLoadingStateChange(
-    content::WebContents* web_contents,
-    LoadingState old_loading_state,
-    LoadingState new_loading_state) {
-  if (new_loading_state == LoadingState::LOADED)
+    const PageNavigationIdentity& page_navigation_id) {
+  if (page_signal_receiver_->GetNavigationIDForWebContents(web_contents) ==
+      page_navigation_id.navigation_id) {
     render_process_probe_->StartSingleGather();
+  }
 }
 
 }  // namespace resource_coordinator

chrome/browser/resource_coordinator/performance_measurement_manager.h

@@ -8,34 +8,30 @@
 #include "base/macros.h"
 #include "base/scoped_observer.h"
 #include "base/sequence_checker.h"
-#include "chrome/browser/resource_coordinator/tab_load_tracker.h"
+#include "chrome/browser/resource_coordinator/page_signal_receiver.h"
 
 namespace resource_coordinator {
 
 class RenderProcessProbe;
-class TabLoadTracker;
-
-// This class observes tab state change notifications issued by the
-// TabLoadTracker and uses them to drive performance measurement requests
-// to the RenderProcessProbe. Results then funnel through the resource
-// coordinator service, back to this class, which stores them in the feature
-// database.
-class PerformanceMeasurementManager : public TabLoadTracker::Observer {
+
+// This class observes the PageAlmostIdle signal from the PageSignalGenerator
+// and uses it to fire off performance measurements for tabs that have reached
+// idle state.
+class PerformanceMeasurementManager : public PageSignalObserver {
  public:
-  PerformanceMeasurementManager(TabLoadTracker* tab_load_tracker,
+  PerformanceMeasurementManager(PageSignalReceiver* receiver,
                                 RenderProcessProbe* render_process_probe);
   ~PerformanceMeasurementManager() override;
 
-  // TabLoadTracker::Observer implementation.
-  void OnStartTracking(content::WebContents* web_contents,
-                       LoadingState loading_state) override;
-  void OnLoadingStateChange(content::WebContents* web_contents,
-                            LoadingState old_loading_state,
-                            LoadingState new_loading_state) override;
+  // PageSignalObserver overrides.
+  void OnPageAlmostIdle(
+      content::WebContents* web_contents,
+      const PageNavigationIdentity& page_navigation_id) override;
 
  private:
-  ScopedObserver<TabLoadTracker, PerformanceMeasurementManager>
+  ScopedObserver<PageSignalReceiver, PerformanceMeasurementManager>
       scoped_observer_;
+  PageSignalReceiver* page_signal_receiver_;
   RenderProcessProbe* render_process_probe_;
 
   DISALLOW_COPY_AND_ASSIGN(PerformanceMeasurementManager);

chrome/browser/resource_coordinator/performance_measurement_manager_unittest.cc

@@ -3,10 +3,14 @@
 // found in the LICENSE file.
 
 #include "chrome/browser/resource_coordinator/performance_measurement_manager.h"
+
+#include "base/test/scoped_feature_list.h"
+#include "chrome/browser/resource_coordinator/page_signal_receiver.h"
 #include "chrome/browser/resource_coordinator/render_process_probe.h"
 #include "chrome/browser/resource_coordinator/tab_helper.h"
-#include "chrome/browser/resource_coordinator/tab_load_tracker.h"
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
+#include "services/resource_coordinator/public/cpp/coordination_unit_id.h"
+#include "services/resource_coordinator/public/cpp/resource_coordinator_features.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -37,22 +41,32 @@ class PerformanceMeasurementManagerTest
   MockRenderProcessProbe mock_render_process_probe_;
 };
 
-TEST_F(PerformanceMeasurementManagerTest, NoMeasurementOnCreation) {
+TEST_F(PerformanceMeasurementManagerTest, StartMeasurementOnPageAlmostIdle) {
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitAndEnableFeature(features::kPageAlmostIdle);
+  ASSERT_TRUE(resource_coordinator::IsPageAlmostIdleSignalEnabled());
+
+  PageSignalReceiver psr;
   PerformanceMeasurementManager performance_measurement_manager(
-      TabLoadTracker::Get(), &mock_render_process_probe());
+      &psr, &mock_render_process_probe());
 
   auto contents = CreateWebContents();
-}
+  const CoordinationUnitID contents_id(CoordinationUnitType::kPage,
+                                       CoordinationUnitID::RANDOM_ID);
+  psr.AssociateCoordinationUnitIDWithWebContents(contents_id, contents.get());
+  psr.SetNavigationID(contents.get(), 100);
 
-TEST_F(PerformanceMeasurementManagerTest, StartMeasurementOnLoaded) {
-  PerformanceMeasurementManager performance_measurement_manager(
-      TabLoadTracker::Get(), &mock_render_process_probe());
+  const CoordinationUnitID dummy_id(CoordinationUnitType::kPage,
+                                    CoordinationUnitID::RANDOM_ID);
+  // There should be no measurement when signaling an unknown CU ID.
+  psr.NotifyPageAlmostIdle(PageNavigationIdentity{dummy_id, 100, ""});
 
-  auto contents = CreateWebContents();
+  // There should be no measurement when signaling the "wrong" navigation ID.
+  psr.NotifyPageAlmostIdle(PageNavigationIdentity{contents_id, 99, ""});
 
+  // A measurement should happen on signaling the right navigation ID.
   EXPECT_CALL(mock_render_process_probe(), StartSingleGather());
-  TabLoadTracker::Get()->TransitionStateForTesting(
-      contents.get(), TabLoadTracker::LoadingState::LOADED);
+  psr.NotifyPageAlmostIdle(PageNavigationIdentity{contents_id, 100, ""});
 }
 
 }  // namespace resource_coordinator