RC: Plumb the main frame's url to the Page CU.

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

chrome/browser/resource_coordinator/page_signal_receiver.cc

@@ -73,11 +73,11 @@ void PageSignalReceiver::NotifyNonPersistentNotificationCreated(
 
 void PageSignalReceiver::OnLoadTimePerformanceEstimate(
     const CoordinationUnitID& page_cu_id,
-    const std::string& origin,
+    const std::string& url,
     base::TimeDelta cpu_usage_estimate,
     uint64_t private_footprint_kb_estimate) {
   NotifyObserversIfKnownCu(
-      page_cu_id, &PageSignalObserver::OnLoadTimePerformanceEstimate, origin,
+      page_cu_id, &PageSignalObserver::OnLoadTimePerformanceEstimate, url,
       cpu_usage_estimate, private_footprint_kb_estimate);
 }
 

chrome/browser/resource_coordinator/page_signal_receiver.h

@@ -38,7 +38,7 @@ class PageSignalObserver {
   // this notification arrives - hence the |site| parameter.
   virtual void OnLoadTimePerformanceEstimate(
       content::WebContents* web_contents,
-      const std::string& origin,
+      const std::string& url,
       base::TimeDelta cpu_usage_estimate,
       uint64_t private_footprint_kb_estimate) {}
 };
@@ -70,7 +70,7 @@ class PageSignalReceiver : public mojom::PageSignalReceiver {
       const CoordinationUnitID& page_cu_id) override;
   void OnLoadTimePerformanceEstimate(
       const CoordinationUnitID& page_cu_id,
-      const std::string& origin,
+      const std::string& url,
       base::TimeDelta cpu_usage_estimate,
       uint64_t private_footprint_kb_estimate) override;
 

chrome/browser/resource_coordinator/tab_helper.cc

@@ -136,7 +136,8 @@ void ResourceCoordinatorTabHelper::DidFinishNavigation(
     if (navigation_handle->IsInMainFrame()) {
       UpdateUkmRecorder(navigation_handle->GetNavigationId());
       ResetFlag();
-      page_resource_coordinator_->OnMainFrameNavigationCommitted();
+      page_resource_coordinator_->OnMainFrameNavigationCommitted(
+          navigation_handle->GetURL().spec());
     }
   }
 }

chrome/browser/resource_coordinator/tab_manager_resource_coordinator_signal_observer.cc

@@ -56,10 +56,10 @@ void TabManager::ResourceCoordinatorSignalObserver::
 
 void TabManager::ResourceCoordinatorSignalObserver::
     OnLoadTimePerformanceEstimate(content::WebContents* web_contents,
-                                  const std::string& origin,
+                                  const std::string& url,
                                   base::TimeDelta cpu_usage_estimate,
                                   uint64_t private_footprint_kb_estimate) {
-  // TODO(siggi): Persist the measurement associated to |site|.
+  // TODO(siggi): Persist the measurement associated to |url|'s site.
 }
 
 }  // namespace resource_coordinator

chrome/browser/resource_coordinator/tab_manager_resource_coordinator_signal_observer.h

@@ -29,7 +29,7 @@ class TabManager::ResourceCoordinatorSignalObserver
       content::WebContents* web_contents) override;
   void OnLoadTimePerformanceEstimate(
       content::WebContents* web_contents,
-      const std::string& origin,
+      const std::string& url,
       base::TimeDelta cpu_usage_estimate,
       uint64_t private_footprint_kb_estimate) override;
 

services/resource_coordinator/coordination_unit/page_coordination_unit_impl.cc

@@ -64,7 +64,10 @@ void PageCoordinationUnitImpl::OnTitleUpdated() {
   SendEvent(mojom::Event::kTitleUpdated);
 }
 
-void PageCoordinationUnitImpl::OnMainFrameNavigationCommitted() {
+void PageCoordinationUnitImpl::OnMainFrameNavigationCommitted(
+    const std::string& url) {
+  main_frame_url_ = url;
+
   SendEvent(mojom::Event::kNavigationCommitted);
 }
 

services/resource_coordinator/coordination_unit/page_coordination_unit_impl.h

@@ -35,7 +35,7 @@ class PageCoordinationUnitImpl
   void SetUKMSourceId(int64_t ukm_source_id) override;
   void OnFaviconUpdated() override;
   void OnTitleUpdated() override;
-  void OnMainFrameNavigationCommitted() override;
+  void OnMainFrameNavigationCommitted(const std::string& url) override;
 
   // There is no direct relationship between processes and pages. However,
   // frames are accessible by both processes and frames, so we find all of the
@@ -85,6 +85,8 @@ class PageCoordinationUnitImpl
     private_footprint_kb_estimate_ = private_footprint_kb_estimate;
   }
 
+  const std::string& main_frame_url() const { return main_frame_url_; }
+
  private:
   friend class FrameCoordinationUnitImpl;
 
@@ -115,6 +117,9 @@ class PageCoordinationUnitImpl
   // The most current memory footprint estimate.
   uint64_t private_footprint_kb_estimate_ = 0;
 
+  // The URL the main frame last committed a navigation to.
+  std::string main_frame_url_;
+
   DISALLOW_COPY_AND_ASSIGN(PageCoordinationUnitImpl);
 };
 

services/resource_coordinator/coordination_unit/page_coordination_unit_impl_unittest.cc

@@ -173,13 +173,16 @@ TEST_F(PageCoordinationUnitImplTest, TimeSinceLastNavigation) {
   EXPECT_TRUE(cu_graph.page->TimeSinceLastNavigation().is_zero());
 
   // 1st navigation.
-  cu_graph.page->OnMainFrameNavigationCommitted();
+  cu_graph.page->OnMainFrameNavigationCommitted("http://www.example.org");
+  EXPECT_EQ("http://www.example.org", cu_graph.page->main_frame_url());
   AdvanceClock(base::TimeDelta::FromSeconds(11));
   EXPECT_EQ(base::TimeDelta::FromSeconds(11),
             cu_graph.page->TimeSinceLastNavigation());
 
   // 2nd navigation.
-  cu_graph.page->OnMainFrameNavigationCommitted();
+  cu_graph.page->OnMainFrameNavigationCommitted(
+      "http://www.example.org/bobcat");
+  EXPECT_EQ("http://www.example.org/bobcat", cu_graph.page->main_frame_url());
   AdvanceClock(base::TimeDelta::FromSeconds(17));
   EXPECT_EQ(base::TimeDelta::FromSeconds(17),
             cu_graph.page->TimeSinceLastNavigation());

services/resource_coordinator/observers/ipc_volume_reporter_unittest.cc

@@ -55,7 +55,7 @@ TEST_F(IPCVolumeReporterTest, Basic) {
   cu_graph.page->SetUKMSourceId(1);
   cu_graph.page->OnFaviconUpdated();
   cu_graph.page->OnTitleUpdated();
-  cu_graph.page->OnMainFrameNavigationCommitted();
+  cu_graph.page->OnMainFrameNavigationCommitted("http://example.org");
 
   cu_graph.process->SetCPUUsage(1.0);
   cu_graph.process->SetExpectedTaskQueueingDuration(

services/resource_coordinator/observers/metrics_collector_unittest.cc

@@ -48,6 +48,8 @@ class MAYBE_MetricsCollectorTest : public CoordinationUnitTestHarness {
   }
 
  protected:
+  static constexpr char kDummyUrl[] = "http://www.example.org";
+
   void AdvanceClock(base::TimeDelta delta) { clock_.Advance(delta); }
 
   base::HistogramTester histogram_tester_;
@@ -57,12 +59,14 @@ class MAYBE_MetricsCollectorTest : public CoordinationUnitTestHarness {
   DISALLOW_COPY_AND_ASSIGN(MAYBE_MetricsCollectorTest);
 };
 
+constexpr char MAYBE_MetricsCollectorTest::kDummyUrl[];
+
 TEST_F(MAYBE_MetricsCollectorTest, FromBackgroundedToFirstAudioStartsUMA) {
   auto page_cu = CreateCoordinationUnit<PageCoordinationUnitImpl>();
   auto frame_cu = CreateCoordinationUnit<FrameCoordinationUnitImpl>();
   page_cu->AddFrame(frame_cu->id());
 
-  page_cu->OnMainFrameNavigationCommitted();
+  page_cu->OnMainFrameNavigationCommitted(kDummyUrl);
   AdvanceClock(kTestMetricsReportDelayTimeout);
 
   page_cu->SetVisibility(true);
@@ -114,7 +118,7 @@ TEST_F(MAYBE_MetricsCollectorTest,
   page_cu->AddFrame(frame_cu->id());
 
   page_cu->SetVisibility(false);
-  page_cu->OnMainFrameNavigationCommitted();
+  page_cu->OnMainFrameNavigationCommitted(kDummyUrl);
   frame_cu->SetAudibility(true);
   // The page is within 5 minutes after main frame navigation was committed,
   // thus no metrics recorded.
@@ -130,7 +134,7 @@ TEST_F(MAYBE_MetricsCollectorTest,
 TEST_F(MAYBE_MetricsCollectorTest, FromBackgroundedToFirstTitleUpdatedUMA) {
   auto page_cu = CreateCoordinationUnit<PageCoordinationUnitImpl>();
 
-  page_cu->OnMainFrameNavigationCommitted();
+  page_cu->OnMainFrameNavigationCommitted(kDummyUrl);
   AdvanceClock(kTestMetricsReportDelayTimeout);
 
   page_cu->SetVisibility(true);
@@ -162,7 +166,7 @@ TEST_F(MAYBE_MetricsCollectorTest,
        FromBackgroundedToFirstTitleUpdatedUMA5MinutesTimeout) {
   auto page_cu = CreateCoordinationUnit<PageCoordinationUnitImpl>();
 
-  page_cu->OnMainFrameNavigationCommitted();
+  page_cu->OnMainFrameNavigationCommitted(kDummyUrl);
   page_cu->SetVisibility(false);
   page_cu->OnTitleUpdated();
   // The page is within 5 minutes after main frame navigation was committed,
@@ -180,7 +184,7 @@ TEST_F(MAYBE_MetricsCollectorTest, FromBackgroundedToFirstAlertFiredUMA) {
   auto frame_cu = CreateCoordinationUnit<FrameCoordinationUnitImpl>();
   page_cu->AddFrame(frame_cu->id());
 
-  page_cu->OnMainFrameNavigationCommitted();
+  page_cu->OnMainFrameNavigationCommitted(kDummyUrl);
   AdvanceClock(kTestMetricsReportDelayTimeout);
 
   page_cu->SetVisibility(true);
@@ -214,7 +218,7 @@ TEST_F(MAYBE_MetricsCollectorTest,
   auto frame_cu = CreateCoordinationUnit<FrameCoordinationUnitImpl>();
   page_cu->AddFrame(frame_cu->id());
 
-  page_cu->OnMainFrameNavigationCommitted();
+  page_cu->OnMainFrameNavigationCommitted(kDummyUrl);
   page_cu->SetVisibility(false);
   frame_cu->OnAlertFired();
   // The page is within 5 minutes after main frame navigation was committed,
@@ -233,7 +237,7 @@ TEST_F(MAYBE_MetricsCollectorTest,
   auto frame_cu = CreateCoordinationUnit<FrameCoordinationUnitImpl>();
   page_cu->AddFrame(frame_cu->id());
 
-  page_cu->OnMainFrameNavigationCommitted();
+  page_cu->OnMainFrameNavigationCommitted(kDummyUrl);
   AdvanceClock(kTestMetricsReportDelayTimeout);
 
   page_cu->SetVisibility(true);
@@ -268,7 +272,7 @@ TEST_F(
   auto frame_cu = CreateCoordinationUnit<FrameCoordinationUnitImpl>();
   page_cu->AddFrame(frame_cu->id());
 
-  page_cu->OnMainFrameNavigationCommitted();
+  page_cu->OnMainFrameNavigationCommitted(kDummyUrl);
   page_cu->SetVisibility(false);
   frame_cu->OnNonPersistentNotificationCreated();
   // The page is within 5 minutes after main frame navigation was committed,
@@ -284,7 +288,7 @@ TEST_F(
 TEST_F(MAYBE_MetricsCollectorTest, FromBackgroundedToFirstFaviconUpdatedUMA) {
   auto page_cu = CreateCoordinationUnit<PageCoordinationUnitImpl>();
 
-  page_cu->OnMainFrameNavigationCommitted();
+  page_cu->OnMainFrameNavigationCommitted(kDummyUrl);
   AdvanceClock(kTestMetricsReportDelayTimeout);
 
   page_cu->SetVisibility(true);
@@ -316,7 +320,7 @@ TEST_F(MAYBE_MetricsCollectorTest,
        FromBackgroundedToFirstFaviconUpdatedUMA5MinutesTimeout) {
   auto page_cu = CreateCoordinationUnit<PageCoordinationUnitImpl>();
 
-  page_cu->OnMainFrameNavigationCommitted();
+  page_cu->OnMainFrameNavigationCommitted(kDummyUrl);
   page_cu->SetVisibility(false);
   page_cu->OnFaviconUpdated();
   // The page is within 5 minutes after main frame navigation was committed,
@@ -345,7 +349,7 @@ TEST_F(MAYBE_MetricsCollectorTest, ResponsivenessMetric) {
   GURL url = GURL("https://google.com/foobar");
   ukm_recorder.UpdateSourceURL(id, url);
   page_cu->SetUKMSourceId(id);
-  page_cu->OnMainFrameNavigationCommitted();
+  page_cu->OnMainFrameNavigationCommitted(kDummyUrl);
 
   for (int count = 1; count < kDefaultFrequencyUkmEQTReported; ++count) {
     process_cu->SetExpectedTaskQueueingDuration(

services/resource_coordinator/observers/page_signal_generator_impl.cc

@@ -241,11 +241,10 @@ void PageSignalGeneratorImpl::OnSystemEventReceived(
       if (data->GetLoadIdleState() == kLoadedAndIdle &&
           !data->performance_estimate_issued &&
           data->last_state_change < measurement_start) {
-        DISPATCH_PAGE_SIGNAL(
-            receivers_, OnLoadTimePerformanceEstimate, page->id(),
-            std::string(),  // TODO(siggi): Plumb the origin here!
-            page->cumulative_cpu_usage_estimate(),
-            page->private_footprint_kb_estimate());
+        DISPATCH_PAGE_SIGNAL(receivers_, OnLoadTimePerformanceEstimate,
+                             page->id(), page->main_frame_url(),
+                             page->cumulative_cpu_usage_estimate(),
+                             page->private_footprint_kb_estimate());
         data->performance_estimate_issued = true;
       }
     }

services/resource_coordinator/observers/page_signal_generator_impl_unittest.cc

@@ -301,7 +301,7 @@ void PageSignalGeneratorImplTest::TestPageAlmostIdleTransitions(bool timeout) {
   EXPECT_FALSE(page_data->idling_timer.IsRunning());
 
   // Post a navigation. The state should reset.
-  page_cu->OnMainFrameNavigationCommitted();
+  page_cu->OnMainFrameNavigationCommitted("http://www.example.org");
   EXPECT_EQ(LIS::kLoadingNotStarted, page_data->GetLoadIdleState());
   EXPECT_FALSE(page_data->idling_timer.IsRunning());
 }
@@ -419,6 +419,7 @@ TEST_F(PageSignalGeneratorImplTest, OnLoadTimePerformanceEstimate) {
   PageSignalGeneratorImpl::PageData* page_data = psg->GetPageData(page_cu);
   page_data->idling_timer.SetTaskRunner(task_env().GetMainThreadTaskRunner());
 
+  page_cu->OnMainFrameNavigationCommitted("https://www.google.com/");
   DrivePageToLoadedAndIdle(&cu_graph);
 
   base::TimeTicks event_time = ResourceCoordinatorClock::NowTicks();
@@ -438,9 +439,10 @@ TEST_F(PageSignalGeneratorImplTest, OnLoadTimePerformanceEstimate) {
 
   {
     base::RunLoop run_loop;
-    EXPECT_CALL(mock_receiver, OnLoadTimePerformanceEstimate(
-                                   cu_graph.page->id(), std::string(),
-                                   base::TimeDelta::FromMicroseconds(15), 150))
+    EXPECT_CALL(mock_receiver,
+                OnLoadTimePerformanceEstimate(
+                    cu_graph.page->id(), "https://www.google.com/",
+                    base::TimeDelta::FromMicroseconds(15), 150))
         .WillOnce(
             ::testing::InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit));
 
@@ -450,7 +452,7 @@ TEST_F(PageSignalGeneratorImplTest, OnLoadTimePerformanceEstimate) {
   ::testing::Mock::VerifyAndClear(&mock_receiver);
 
   // Make sure a second run around the state machine generates a second event.
-  page_cu->OnMainFrameNavigationCommitted();
+  page_cu->OnMainFrameNavigationCommitted("https://example.org/bobcat");
   task_env().FastForwardUntilNoTasksRemain();
   EXPECT_NE(LIS::kLoadedAndIdle, page_data->GetLoadIdleState());
 
@@ -464,9 +466,10 @@ TEST_F(PageSignalGeneratorImplTest, OnLoadTimePerformanceEstimate) {
 
   {
     base::RunLoop run_loop;
-    EXPECT_CALL(mock_receiver, OnLoadTimePerformanceEstimate(
-                                   cu_graph.page->id(), std::string(),
-                                   base::TimeDelta::FromMicroseconds(25), 250))
+    EXPECT_CALL(mock_receiver,
+                OnLoadTimePerformanceEstimate(
+                    cu_graph.page->id(), "https://example.org/bobcat",
+                    base::TimeDelta::FromMicroseconds(25), 250))
         .WillOnce(
             ::testing::InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit));
 

services/resource_coordinator/public/cpp/page_resource_coordinator.cc

@@ -46,10 +46,11 @@ void PageResourceCoordinator::OnTitleUpdated() {
   service_->OnTitleUpdated();
 }
 
-void PageResourceCoordinator::OnMainFrameNavigationCommitted() {
+void PageResourceCoordinator::OnMainFrameNavigationCommitted(
+    const std::string& url) {
   if (!service_)
     return;
-  service_->OnMainFrameNavigationCommitted();
+  service_->OnMainFrameNavigationCommitted(url);
 }
 
 void PageResourceCoordinator::AddFrame(const FrameResourceCoordinator& frame) {

services/resource_coordinator/public/cpp/page_resource_coordinator.h

@@ -25,7 +25,7 @@ class SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_EXPORT PageResourceCoordinator
   void SetUKMSourceId(int64_t ukm_source_id);
   void OnFaviconUpdated();
   void OnTitleUpdated();
-  void OnMainFrameNavigationCommitted();
+  void OnMainFrameNavigationCommitted(const std::string& url);
 
   void AddFrame(const FrameResourceCoordinator& frame);
   void RemoveFrame(const FrameResourceCoordinator& frame);

services/resource_coordinator/public/mojom/coordination_unit.mojom

@@ -88,7 +88,7 @@ interface PageCoordinationUnit {
   // Event signals.
   OnFaviconUpdated();
   OnTitleUpdated();
-  OnMainFrameNavigationCommitted();
+  OnMainFrameNavigationCommitted(string url);
 };
 
 interface ProcessCoordinationUnit {

services/resource_coordinator/public/mojom/page_signal.mojom

@@ -30,11 +30,11 @@ interface PageSignalReceiver {
   // notification.
   NotifyNonPersistentNotificationCreated(CoordinationUnitID page_cu_id);
 
-  // This notification needs the site that was loaded, as by the time the
+  // This notification needs the url that was loaded, as by the time the
   // notification comes back around, the WebContents may have navigated to
   // another site altogether.
   OnLoadTimePerformanceEstimate(CoordinationUnitID cu_id,
-                                   string origin,
+                                   string url,
                                    mojo_base.mojom.TimeDelta cpu_usage_estimate,
                                    uint64 private_footprint_kb_estimate);
 };