Revert "Reland "Migrate AdsPageLoadMetricsObserver to ResourceDataUse over OnLoadedResource""

This reverts commit 02979f89.

Reason for revert: Causing failures on linux-xenial-rel bot:

https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/linux-xenial-rel/6075

Original change's description:
> Reland "Migrate AdsPageLoadMetricsObserver to ResourceDataUse over OnLoadedResource"
> 
> This is a reland of 7c6d18ba
> 
> Original change's description:
> > Migrate AdsPageLoadMetricsObserver to ResourceDataUse over OnLoadedResource
> >
> > This CL is strictly a refactor that switches the Ads PLMO to count frame
> > data using ResourceDataUseUpdate instead of OnLoadedResource. This will
> > allow us to include incomplete resources and header bytes in future
> > per-frame metrics. This will not change behavior of existing histograms
> > at all.
> >
> > Bug: 878393
> > Change-Id: Ibb235c11bbaa168a40be51bc99c03a0ba999c251
> > Reviewed-on: https://chromium-review.googlesource.com/c/1372428
> > Commit-Queue: John Delaney <johnidel@chromium.org>
> > Reviewed-by: Robert Sesek <rsesek@chromium.org>
> > Reviewed-by: Josh Karlin <jkarlin@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#619436}
> 
> TBR=rsesek@chromium.org
> 
> Bug: 878393
> Change-Id: I9c8760497d2a5110f4bae31d633d82a405ed812a
> Reviewed-on: https://chromium-review.googlesource.com/c/1393530
> Commit-Queue: John Delaney <johnidel@chromium.org>
> Reviewed-by: Josh Karlin <jkarlin@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#619760}

TBR=jkarlin@chromium.org,rsesek@chromium.org,johnidel@chromium.org

Change-Id: Ia755a547df8982402942b3430e6680ed1e97f3a2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 878393
Reviewed-on: https://chromium-review.googlesource.com/c/1394949Reviewed-by: Tommy Steimel <steimel@chromium.org>
Commit-Queue: Tommy Steimel <steimel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619789}

chrome/browser/page_load_metrics/observers/ads_page_load_metrics_observer.cc

@@ -308,6 +308,11 @@ AdsPageLoadMetricsObserver::FlushMetricsOnAppEnterBackground(
   return STOP_OBSERVING;
 }
 
+void AdsPageLoadMetricsObserver::OnLoadedResource(
+    const page_load_metrics::ExtraRequestCompleteInfo& extra_request_info) {
+  ProcessLoadedResource(extra_request_info);
+}
+
 void AdsPageLoadMetricsObserver::OnComplete(
     const page_load_metrics::mojom::PageLoadTiming& timing,
     const page_load_metrics::PageLoadExtraInfo& info) {
@@ -317,11 +322,10 @@ void AdsPageLoadMetricsObserver::OnComplete(
 }
 
 void AdsPageLoadMetricsObserver::OnResourceDataUseObserved(
-    FrameTreeNodeId frame_tree_node_id,
     const std::vector<page_load_metrics::mojom::ResourceDataUpdatePtr>&
         resources) {
   for (auto const& resource : resources)
-    UpdateResource(frame_tree_node_id, resource);
+    UpdateResource(resource);
 }
 
 void AdsPageLoadMetricsObserver::OnSubframeNavigationEvaluated(
@@ -378,22 +382,29 @@ AdsPageLoadMetricsObserver::AdTypes AdsPageLoadMetricsObserver::DetectAds(
   return ad_types;
 }
 
-void AdsPageLoadMetricsObserver::ProcessResourceForFrame(
-    FrameTreeNodeId frame_tree_node_id,
-    const page_load_metrics::mojom::ResourceDataUpdatePtr& resource) {
-  const auto& id_and_data = ad_frames_data_.find(frame_tree_node_id);
+void AdsPageLoadMetricsObserver::ProcessLoadedResource(
+    const page_load_metrics::ExtraRequestCompleteInfo& extra_request_info) {
+  const auto& id_and_data =
+      ad_frames_data_.find(extra_request_info.frame_tree_node_id);
   if (id_and_data == ad_frames_data_.end()) {
-    if (resource->is_primary_frame_resource) {
-      // Only hold onto primary resources if their load has finished.
-      if (!resource->is_complete)
-        return;
-
+    if (extra_request_info.resource_type == content::RESOURCE_TYPE_MAIN_FRAME ||
+        extra_request_info.resource_type == content::RESOURCE_TYPE_SUB_FRAME) {
       // This resource request is the primary resource load for a frame that
       // hasn't yet finished navigating. Hang onto the request info and replay
       // it once the frame finishes navigating.
       ongoing_navigation_resources_.emplace(
-          std::piecewise_construct, std::forward_as_tuple(frame_tree_node_id),
-          std::forward_as_tuple(resource.Clone()));
+          std::piecewise_construct,
+          std::forward_as_tuple(extra_request_info.frame_tree_node_id),
+          std::forward_as_tuple(
+              extra_request_info.url, extra_request_info.host_port_pair,
+              extra_request_info.frame_tree_node_id,
+              extra_request_info.was_cached, extra_request_info.raw_body_bytes,
+              extra_request_info.original_network_content_length, nullptr,
+              extra_request_info.resource_type, extra_request_info.net_error,
+              extra_request_info.load_timing_info
+                  ? std::make_unique<net::LoadTimingInfo>(
+                        *extra_request_info.load_timing_info)
+                  : nullptr));
     } else {
       // This is unexpected, it could be:
       // 1. a resource from a previous navigation that started its resource
@@ -404,21 +415,18 @@ void AdsPageLoadMetricsObserver::ProcessResourceForFrame(
     return;
   }
 
-  if (resource->is_complete) {
-    page_bytes_ += resource->encoded_body_length;
-    if (!resource->was_fetched_via_cache)
-      uncached_page_bytes_ += resource->encoded_body_length;
-  }
+  page_bytes_ += extra_request_info.raw_body_bytes;
+  if (!extra_request_info.was_cached)
+    uncached_page_bytes_ += extra_request_info.raw_body_bytes;
 
   // Determine if the frame (or its ancestor) is an ad, if so attribute the
   // bytes to the highest ad ancestor.
   AdFrameData* ancestor_data = id_and_data->second;
 
   if (ancestor_data) {
-    if (resource->is_complete) {
-      ancestor_data->frame_bytes += resource->encoded_body_length;
-      if (!resource->was_fetched_via_cache)
-        ancestor_data->frame_bytes_uncached += resource->encoded_body_length;
+    ancestor_data->frame_bytes += extra_request_info.raw_body_bytes;
+    if (!extra_request_info.was_cached) {
+      ancestor_data->frame_bytes_uncached += extra_request_info.raw_body_bytes;
     }
   }
 }
@@ -449,9 +457,7 @@ AdsPageLoadMetricsObserver::GetResourceMimeType(
 }
 
 void AdsPageLoadMetricsObserver::UpdateResource(
-    FrameTreeNodeId frame_tree_node_id,
     const page_load_metrics::mojom::ResourceDataUpdatePtr& resource) {
-  ProcessResourceForFrame(frame_tree_node_id, resource);
   auto it = page_resources_.find(resource->request_id);
   // A new resource has been observed.
   if (it == page_resources_.end())
@@ -684,6 +690,6 @@ void AdsPageLoadMetricsObserver::ProcessOngoingNavigationResource(
   if (frame_id_and_request == ongoing_navigation_resources_.end())
     return;
 
-  ProcessResourceForFrame(frame_tree_node_id, frame_id_and_request->second);
+  ProcessLoadedResource(frame_id_and_request->second);
   ongoing_navigation_resources_.erase(frame_id_and_request);
 }

chrome/browser/page_load_metrics/observers/ads_page_load_metrics_observer.h

@@ -81,10 +81,11 @@ class AdsPageLoadMetricsObserver
   ObservePolicy FlushMetricsOnAppEnterBackground(
       const page_load_metrics::mojom::PageLoadTiming& timing,
       const page_load_metrics::PageLoadExtraInfo& extra_info) override;
+  void OnLoadedResource(const page_load_metrics::ExtraRequestCompleteInfo&
+                            extra_request_info) override;
   void OnComplete(const page_load_metrics::mojom::PageLoadTiming& timing,
                   const page_load_metrics::PageLoadExtraInfo& info) override;
   void OnResourceDataUseObserved(
-      FrameTreeNodeId frame_tree_node_id,
       const std::vector<page_load_metrics::mojom::ResourceDataUpdatePtr>&
           resources) override;
   void OnPageInteractive(
@@ -97,8 +98,6 @@ class AdsPageLoadMetricsObserver
                 AdTypes ad_types,
                 AdOriginStatus origin_status,
                 bool frame_navigated);
-
-    // Total prefilter (body) bytes loaded for complete resources.
     size_t frame_bytes;
     size_t frame_bytes_uncached;
     const FrameTreeNodeId frame_tree_node_id;
@@ -125,9 +124,8 @@ class AdsPageLoadMetricsObserver
   // each call in order to free up memory.
   AdTypes DetectAds(content::NavigationHandle* navigation_handle);
 
-  void ProcessResourceForFrame(
-      FrameTreeNodeId frame_tree_node_id,
-      const page_load_metrics::mojom::ResourceDataUpdatePtr& resource);
+  void ProcessLoadedResource(
+      const page_load_metrics::ExtraRequestCompleteInfo& extra_request_info);
 
   // Get the mime type of a resource. This only returns a subset of mime types,
   // grouped at a higher level. For example, all video mime types return the
@@ -139,7 +137,6 @@ class AdsPageLoadMetricsObserver
   // update. Updates |page_resources_| to reflect the new state of the resource.
   // Called once per ResourceDataUpdate.
   void UpdateResource(
-      FrameTreeNodeId frame_tree_node_id,
       const page_load_metrics::mojom::ResourceDataUpdatePtr& resource);
 
   // Records size of resources by mime type.
@@ -155,7 +152,7 @@ class AdsPageLoadMetricsObserver
 
   // Checks to see if a resource is waiting for a navigation with the given
   // |frame_tree_node_id| to commit before it can be processed. If so, call
-  // OnResourceDataUpdate for the delayed resource.
+  // OnLoadedResource for the delayed resource.
   void ProcessOngoingNavigationResource(FrameTreeNodeId frame_tree_node_id);
 
   // Stores the size data of each ad frame. Pointed to by ad_frames_ so use a
@@ -177,7 +174,7 @@ class AdsPageLoadMetricsObserver
   // When the observer receives report of a document resource loading for a
   // sub-frame before the sub-frame commit occurs, hold onto the resource
   // request info (delay it) until the sub-frame commits.
-  std::map<FrameTreeNodeId, page_load_metrics::mojom::ResourceDataUpdatePtr>
+  std::map<FrameTreeNodeId, page_load_metrics::ExtraRequestCompleteInfo>
       ongoing_navigation_resources_;
 
   // Maps a request_id for a blink resource to the metadata for the resource

chrome/browser/page_load_metrics/observers/ads_page_load_metrics_observer_browsertest.cc

@@ -87,14 +87,6 @@ class AdsPageLoadMetricsObserverBrowserTest
   }
   ~AdsPageLoadMetricsObserverBrowserTest() override {}
 
-  std::unique_ptr<page_load_metrics::PageLoadMetricsTestWaiter>
-  CreatePageLoadMetricsTestWaiter() {
-    content::WebContents* web_contents =
-        browser()->tab_strip_model()->GetActiveWebContents();
-    return std::make_unique<page_load_metrics::PageLoadMetricsTestWaiter>(
-        web_contents);
-  }
-
  private:
   base::test::ScopedFeatureList scoped_feature_list_;
 
@@ -105,12 +97,9 @@ class AdsPageLoadMetricsObserverBrowserTest
 IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverBrowserTest,
                        OriginStatusMetricEmbedded) {
   base::HistogramTester histogram_tester;
-  auto waiter = CreatePageLoadMetricsTestWaiter();
   ui_test_utils::NavigateToURL(
       browser(),
       embedded_test_server()->GetURL("/ads_observer/srcdoc_embedded_ad.html"));
-  waiter->AddMinimumCompleteResourcesExpectation(3);
-  waiter->Wait();
   ui_test_utils::NavigateToURL(browser(), GURL(url::kAboutBlankURL));
   histogram_tester.ExpectUniqueSample(
       kCrossOriginHistogramId,
@@ -132,13 +121,9 @@ IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverBrowserTest,
 IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverBrowserTest,
                        OriginStatusMetricSame) {
   base::HistogramTester histogram_tester;
-  auto waiter = CreatePageLoadMetricsTestWaiter();
   ui_test_utils::NavigateToURL(
       browser(),
       embedded_test_server()->GetURL("/ads_observer/same_origin_ad.html"));
-  waiter->AddMinimumCompleteResourcesExpectation(3);
-  waiter->Wait();
-
   ui_test_utils::NavigateToURL(browser(), GURL(url::kAboutBlankURL));
   histogram_tester.ExpectUniqueSample(
       kCrossOriginHistogramId,
@@ -150,8 +135,6 @@ IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverBrowserTest,
                        OriginStatusMetricCross) {
   // Note: Cannot navigate cross-origin without dynamically generating the URL.
   base::HistogramTester histogram_tester;
-  auto waiter = CreatePageLoadMetricsTestWaiter();
-
   ui_test_utils::NavigateToURL(
       browser(), embedded_test_server()->GetURL("/iframe_blank.html"));
   // Note that the initial iframe is not an ad, so the metric doesn't observe
@@ -161,11 +144,6 @@ IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverBrowserTest,
   NavigateIframeToURL(web_contents(), "test",
                       embedded_test_server()->GetURL(
                           "a.com", "/ads_observer/same_origin_ad.html"));
-
-  // Wait until all resource data updates are sent.
-  waiter->AddPageExpectation(
-      page_load_metrics::PageLoadMetricsTestWaiter::TimingField::kLoadEvent);
-  waiter->Wait();
   ui_test_utils::NavigateToURL(browser(), GURL(url::kAboutBlankURL));
   histogram_tester.ExpectUniqueSample(
       kCrossOriginHistogramId,
@@ -190,24 +168,6 @@ IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverBrowserTest,
   // Navigate away to force the histogram recording.
   ui_test_utils::NavigateToURL(browser(), GURL(url::kAboutBlankURL));
 
-  // TODO(johnidel): Check that the subresources of the new frame are reported
-  // correctly. Resources from a failed provisional load are not reported to
-  // resource data updates, causing this adframe to not be recorded. This is an
-  // uncommon case but should be reported. See crbug.com/914893.
-}
-
-// Test that a blank ad subframe that is docwritten correctly reports metrics.
-IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverBrowserTest,
-                       DocWriteAboutBlankAdframe) {
-  base::HistogramTester histogram_tester;
-  auto waiter = CreatePageLoadMetricsTestWaiter();
-  ui_test_utils::NavigateToURL(browser(),
-                               embedded_test_server()->GetURL(
-                                   "/ads_observer/docwrite_blank_frame.html"));
-  waiter->AddMinimumCompleteResourcesExpectation(4);
-  waiter->Wait();
-  // Navigate away to force the histogram recording.
-  ui_test_utils::NavigateToURL(browser(), GURL(url::kAboutBlankURL));
   histogram_tester.ExpectUniqueSample(
       "PageLoad.Clients.Ads.Google.FrameCounts.AnyParentFrame.AdFrames", 1, 1);
   histogram_tester.ExpectUniqueSample(
@@ -228,13 +188,7 @@ IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverBrowserTest,
   const GURL main_url(embedded_test_server()->GetURL(
       "a.com", "/cross_site_iframe_factory.html?a(b,b,c,d)"));
 
-  auto waiter = CreatePageLoadMetricsTestWaiter();
   ui_test_utils::NavigateToURL(browser(), main_url);
-
-  // One favicon resource and 2 resources for each frame.
-  waiter->AddMinimumCompleteResourcesExpectation(11);
-  waiter->Wait();
-
   // Navigate away to force the histogram recording.
   ui_test_utils::NavigateToURL(browser(), GURL(url::kAboutBlankURL));
 

chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.cc

@@ -716,7 +716,6 @@ void CorePageLoadMetricsObserver::OnUserInput(
 }
 
 void CorePageLoadMetricsObserver::OnResourceDataUseObserved(
-    FrameTreeNodeId frame_tree_node_id,
     const std::vector<page_load_metrics::mojom::ResourceDataUpdatePtr>&
         resources) {
   for (auto const& resource : resources) {

chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.h

@@ -214,7 +214,6 @@ class CorePageLoadMetricsObserver
       const page_load_metrics::mojom::PageLoadTiming& timing,
       const page_load_metrics::PageLoadExtraInfo& extra_info) override;
   void OnResourceDataUseObserved(
-      FrameTreeNodeId frame_tree_node_id,
       const std::vector<page_load_metrics::mojom::ResourceDataUpdatePtr>&
           resources) override;
 

chrome/browser/page_load_metrics/observers/data_reduction_proxy_metrics_observer_base.cc

@@ -354,7 +354,6 @@ void DataReductionProxyMetricsObserverBase::OnLoadedResource(
 }
 
 void DataReductionProxyMetricsObserverBase::OnResourceDataUseObserved(
-    FrameTreeNodeId frame_tree_node_id,
     const std::vector<page_load_metrics::mojom::ResourceDataUpdatePtr>&
         resources) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

chrome/browser/page_load_metrics/observers/data_reduction_proxy_metrics_observer_base.h

@@ -52,7 +52,6 @@ class DataReductionProxyMetricsObserverBase
   void OnLoadedResource(const page_load_metrics::ExtraRequestCompleteInfo&
                             extra_request_compelte_info) override;
   void OnResourceDataUseObserved(
-      FrameTreeNodeId frame_tree_node_id,
       const std::vector<page_load_metrics::mojom::ResourceDataUpdatePtr>&
           resources) override;
   void OnEventOccurred(const void* const event_key) override;

chrome/browser/page_load_metrics/observers/data_saver_site_breakdown_metrics_observer.cc

@@ -42,7 +42,6 @@ DataSaverSiteBreakdownMetricsObserver::OnCommit(
 }
 
 void DataSaverSiteBreakdownMetricsObserver::OnResourceDataUseObserved(
-    FrameTreeNodeId frame_tree_node_id,
     const std::vector<page_load_metrics::mojom::ResourceDataUpdatePtr>&
         resources) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

chrome/browser/page_load_metrics/observers/data_saver_site_breakdown_metrics_observer.h

@@ -31,7 +31,6 @@ class DataSaverSiteBreakdownMetricsObserver
                          ukm::SourceId source_id) override;
 
   void OnResourceDataUseObserved(
-      FrameTreeNodeId frame_tree_node_id,
       const std::vector<page_load_metrics::mojom::ResourceDataUpdatePtr>&
           resources) override;
 

chrome/browser/page_load_metrics/observers/media_page_load_metrics_observer.cc

@@ -24,7 +24,6 @@ MediaPageLoadMetricsObserver::MediaPageLoadMetricsObserver()
 MediaPageLoadMetricsObserver::~MediaPageLoadMetricsObserver() = default;
 
 void MediaPageLoadMetricsObserver::OnResourceDataUseObserved(
-    FrameTreeNodeId frame_tree_node_id,
     const std::vector<page_load_metrics::mojom::ResourceDataUpdatePtr>&
         resources) {
   for (auto const& resource : resources) {

chrome/browser/page_load_metrics/observers/media_page_load_metrics_observer.h

@@ -27,7 +27,6 @@ class MediaPageLoadMetricsObserver
       const page_load_metrics::mojom::PageLoadTiming& timing,
       const page_load_metrics::PageLoadExtraInfo& info) override;
   void OnResourceDataUseObserved(
-      FrameTreeNodeId frame_tree_node_id,
       const std::vector<page_load_metrics::mojom::ResourceDataUpdatePtr>&
           resources) override;
   void MediaStartedPlaying(

chrome/browser/page_load_metrics/observers/page_capping_page_load_metrics_observer.cc

@@ -111,7 +111,6 @@ PageCappingPageLoadMetricsObserver::OnCommit(
 }
 
 void PageCappingPageLoadMetricsObserver::OnResourceDataUseObserved(
-    FrameTreeNodeId frame_tree_node_id,
     const std::vector<page_load_metrics::mojom::ResourceDataUpdatePtr>&
         resources) {
   last_data_use_time_ = clock_->NowTicks();

chrome/browser/page_load_metrics/observers/page_capping_page_load_metrics_observer.h

@@ -75,7 +75,6 @@ class PageCappingPageLoadMetricsObserver
  private:
   // page_load_metrics::PageLoadMetricsObserver:
   void OnResourceDataUseObserved(
-      FrameTreeNodeId frame_tree_node_id,
       const std::vector<page_load_metrics::mojom::ResourceDataUpdatePtr>&
           resources) override;
   ObservePolicy OnCommit(content::NavigationHandle* navigation_handle,

chrome/browser/page_load_metrics/observers/page_load_metrics_observer_test_harness.cc

@@ -70,12 +70,6 @@ void PageLoadMetricsObserverTestHarness::SimulateResourceDataUseUpdate(
   tester_->SimulateResourceDataUseUpdate(resources);
 }
 
-void PageLoadMetricsObserverTestHarness::SimulateResourceDataUseUpdate(
-    const std::vector<mojom::ResourceDataUpdatePtr>& resources,
-    content::RenderFrameHost* render_frame_host) {
-  tester_->SimulateResourceDataUseUpdate(resources, render_frame_host);
-}
-
 void PageLoadMetricsObserverTestHarness::SimulateFeaturesUpdate(
     const mojom::PageLoadFeatures& new_features) {
   tester_->SimulateFeaturesUpdate(new_features);

chrome/browser/page_load_metrics/observers/page_load_metrics_observer_test_harness.h

@@ -83,9 +83,6 @@ class PageLoadMetricsObserverTestHarness
   void SimulateFeaturesUpdate(const mojom::PageLoadFeatures& new_features);
   void SimulateResourceDataUseUpdate(
       const std::vector<mojom::ResourceDataUpdatePtr>& resources);
-  void SimulateResourceDataUseUpdate(
-      const std::vector<mojom::ResourceDataUpdatePtr>& resources,
-      content::RenderFrameHost* render_frame_host);
   void SimulateRenderDataUpdate(const mojom::PageRenderData& render_data);
 
   // Simulates a loaded resource. Main frame resources must specify a

chrome/browser/page_load_metrics/observers/page_load_metrics_observer_tester.cc

@@ -119,19 +119,11 @@ void PageLoadMetricsObserverTester::SimulatePageLoadTimingUpdate(
 
 void PageLoadMetricsObserverTester::SimulateResourceDataUseUpdate(
     const std::vector<mojom::ResourceDataUpdatePtr>& resources) {
-  SimulateResourceDataUseUpdate(resources, web_contents()->GetMainFrame());
-}
-
-void PageLoadMetricsObserverTester::SimulateResourceDataUseUpdate(
-    const std::vector<mojom::ResourceDataUpdatePtr>& resources,
-    content::RenderFrameHost* render_frame_host) {
-  auto timing = mojom::PageLoadTimingPtr(base::in_place);
-  InitPageLoadTimingForTest(timing.get());
-  observer_->OnTimingUpdated(render_frame_host, std::move(timing),
-                             mojom::PageLoadMetadataPtr(base::in_place),
-                             mojom::PageLoadFeaturesPtr(base::in_place),
-                             resources,
-                             mojom::PageRenderDataPtr(base::in_place));
+  observer_->OnTimingUpdated(
+      web_contents()->GetMainFrame(), mojom::PageLoadTimingPtr(base::in_place),
+      mojom::PageLoadMetadataPtr(base::in_place),
+      mojom::PageLoadFeaturesPtr(base::in_place), resources,
+      mojom::PageRenderDataPtr(base::in_place));
 }
 
 void PageLoadMetricsObserverTester::SimulateLoadedResource(

chrome/browser/page_load_metrics/observers/page_load_metrics_observer_tester.h

@@ -54,9 +54,6 @@ class PageLoadMetricsObserverTester : public test::WeakMockTimerProvider {
   void SimulateFeaturesUpdate(const mojom::PageLoadFeatures& new_features);
   void SimulateResourceDataUseUpdate(
       const std::vector<mojom::ResourceDataUpdatePtr>& resources);
-  void SimulateResourceDataUseUpdate(
-      const std::vector<mojom::ResourceDataUpdatePtr>& resources,
-      content::RenderFrameHost* render_frame_host);
   void SimulateRenderDataUpdate(const mojom::PageRenderData& render_data);
 
   // Simulates a loaded resource. Main frame resources must specify a

chrome/browser/page_load_metrics/observers/previews_page_load_metrics_observer.cc

@@ -220,7 +220,6 @@ void PreviewsPageLoadMetricsObserver::RecordTimingMetrics(
 }
 
 void PreviewsPageLoadMetricsObserver::OnResourceDataUseObserved(
-    FrameTreeNodeId frame_tree_node_id,
     const std::vector<page_load_metrics::mojom::ResourceDataUpdatePtr>&
         resources) {
   for (auto const& resource : resources) {

chrome/browser/page_load_metrics/observers/previews_page_load_metrics_observer.h

@@ -40,7 +40,6 @@ class PreviewsPageLoadMetricsObserver
       const page_load_metrics::mojom::PageLoadTiming& timing,
       const page_load_metrics::PageLoadExtraInfo& info) override;
   void OnResourceDataUseObserved(
-      FrameTreeNodeId frame_tree_node_id,
       const std::vector<page_load_metrics::mojom::ResourceDataUpdatePtr>&
           resources) override;
 

chrome/browser/page_load_metrics/observers/tab_restore_page_load_metrics_observer.cc

@@ -40,7 +40,6 @@ TabRestorePageLoadMetricsObserver::OnStart(
 }
 
 void TabRestorePageLoadMetricsObserver::OnResourceDataUseObserved(
-    FrameTreeNodeId frame_tree_node_id,
     const std::vector<page_load_metrics::mojom::ResourceDataUpdatePtr>&
         resources) {
   for (auto const& resource : resources) {

chrome/browser/page_load_metrics/observers/tab_restore_page_load_metrics_observer.h

@@ -30,7 +30,6 @@ class TabRestorePageLoadMetricsObserver
       const GURL& currently_committed_url,
       bool started_in_foreground) override;
   void OnResourceDataUseObserved(
-      FrameTreeNodeId frame_tree_node_id,
       const std::vector<page_load_metrics::mojom::ResourceDataUpdatePtr>&
           resources) override;
   page_load_metrics::PageLoadMetricsObserver::ObservePolicy

chrome/browser/page_load_metrics/page_load_metrics_observer.h

@@ -432,10 +432,9 @@ class PageLoadMetricsObserver {
                                        const PageLoadExtraInfo& extra_info) {}
 
   // Invoked when there is data use for loading a resource on the page
-  // for a given render frame host. This only contains resources that have had
-  // new data use since the last callback.
+  // across all frames. This only contains resources that have had new
+  // data use since the last callback.
   virtual void OnResourceDataUseObserved(
-      FrameTreeNodeId frame_tree_node_id,
       const std::vector<mojom::ResourceDataUpdatePtr>& resources) {}
 
   // Invoked when a media element starts playing.

chrome/browser/page_load_metrics/page_load_metrics_test_waiter.cc

@@ -114,7 +114,6 @@ void PageLoadMetricsTestWaiter::OnLoadedResource(
 }
 
 void PageLoadMetricsTestWaiter::OnResourceDataUseObserved(
-    FrameTreeNodeId frame_tree_node_id,
     const std::vector<page_load_metrics::mojom::ResourceDataUpdatePtr>&
         resources) {
   for (auto const& resource : resources) {
@@ -258,11 +257,10 @@ void PageLoadMetricsTestWaiter::WaiterMetricsObserver::OnLoadedResource(
 
 void PageLoadMetricsTestWaiter::WaiterMetricsObserver::
     OnResourceDataUseObserved(
-        FrameTreeNodeId frame_tree_node_id,
         const std::vector<page_load_metrics::mojom::ResourceDataUpdatePtr>&
             resources) {
   if (waiter_)
-    waiter_->OnResourceDataUseObserved(frame_tree_node_id, resources);
+    waiter_->OnResourceDataUseObserved(resources);
 }
 
 void PageLoadMetricsTestWaiter::WaiterMetricsObserver::OnFeaturesUsageObserved(

chrome/browser/page_load_metrics/page_load_metrics_test_waiter.h

@@ -29,8 +29,6 @@ class PageLoadMetricsTestWaiter
     // kLoadTimingInfo waits for main frame timing info only.
     kLoadTimingInfo = 1 << 6,
   };
-  using FrameTreeNodeId =
-      page_load_metrics::PageLoadMetricsObserver::FrameTreeNodeId;
 
   explicit PageLoadMetricsTestWaiter(content::WebContents* web_contents);
 
@@ -79,8 +77,6 @@ class PageLoadMetricsTestWaiter
   class WaiterMetricsObserver
       : public page_load_metrics::PageLoadMetricsObserver {
    public:
-    using FrameTreeNodeId =
-        page_load_metrics::PageLoadMetricsObserver::FrameTreeNodeId;
     // We use a WeakPtr to the PageLoadMetricsTestWaiter because |waiter| can be
     // destroyed before this WaiterMetricsObserver.
     explicit WaiterMetricsObserver(
@@ -96,7 +92,6 @@ class PageLoadMetricsTestWaiter
                               extra_request_complete_info) override;
 
     void OnResourceDataUseObserved(
-        FrameTreeNodeId frame_tree_node_id,
         const std::vector<page_load_metrics::mojom::ResourceDataUpdatePtr>&
             resources) override;
 
@@ -163,7 +158,6 @@ class PageLoadMetricsTestWaiter
   // from a resource load. Stops waiting if expectations are satisfied after
   // update.
   void OnResourceDataUseObserved(
-      FrameTreeNodeId frame_tree_node_id,
       const std::vector<page_load_metrics::mojom::ResourceDataUpdatePtr>&
           resources);
 

chrome/browser/page_load_metrics/page_load_metrics_update_dispatcher.cc

@@ -444,8 +444,7 @@ void PageLoadMetricsUpdateDispatcher::UpdateMetrics(
 
   // Report data usage before new timing and metadata for messages that have
   // both updates.
-  client_->UpdateResourceDataUse(render_frame_host->GetFrameTreeNodeId(),
-                                 resources);
+  client_->UpdateResourceDataUse(resources);
   if (render_frame_host->GetParent() == nullptr) {
     UpdateMainFrameMetadata(std::move(new_metadata));
     UpdateMainFrameTiming(std::move(new_timing));

chrome/browser/page_load_metrics/page_load_metrics_update_dispatcher.h

@@ -112,7 +112,6 @@ class PageLoadMetricsUpdateDispatcher {
         content::RenderFrameHost* rfh,
         const mojom::PageLoadFeatures& new_features) = 0;
     virtual void UpdateResourceDataUse(
-        int frame_tree_node_id,
         const std::vector<mojom::ResourceDataUpdatePtr>& resources) = 0;
   };
 

chrome/browser/page_load_metrics/page_load_tracker.cc

@@ -651,10 +651,9 @@ void PageLoadTracker::UpdateFeaturesUsage(
 }
 
 void PageLoadTracker::UpdateResourceDataUse(
-    int frame_tree_node_id,
     const std::vector<mojom::ResourceDataUpdatePtr>& resources) {
   for (const auto& observer : observers_) {
-    observer->OnResourceDataUseObserved(frame_tree_node_id, resources);
+    observer->OnResourceDataUseObserved(resources);
   }
 }
 

chrome/browser/page_load_metrics/page_load_tracker.h

@@ -182,7 +182,6 @@ class PageLoadTracker : public PageLoadMetricsUpdateDispatcher::Client {
       content::RenderFrameHost* rfh,
       const mojom::PageLoadFeatures& new_features) override;
   void UpdateResourceDataUse(
-      int frame_tree_node_id,
       const std::vector<mojom::ResourceDataUpdatePtr>& resources) override;
 
   void Redirect(content::NavigationHandle* navigation_handle);

chrome/browser/subresource_filter/ad_tagging_browsertest.cc

@@ -9,7 +9,6 @@
 #include "base/test/metrics/histogram_tester.h"
 #include "chrome/browser/metrics/subprocess_metrics_provider.h"
 #include "chrome/browser/page_load_metrics/observers/ads_page_load_metrics_observer.h"
-#include "chrome/browser/page_load_metrics/page_load_metrics_test_waiter.h"
 #include "chrome/browser/subresource_filter/subresource_filter_browser_test_harness.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
@@ -93,12 +92,6 @@ class AdTaggingBrowserTest : public SubresourceFilterBrowserTest {
     return embedded_test_server()->GetURL("/ad_tagging/" + page);
   }
 
-  std::unique_ptr<page_load_metrics::PageLoadMetricsTestWaiter>
-  CreatePageLoadMetricsTestWaiter() {
-    return std::make_unique<page_load_metrics::PageLoadMetricsTestWaiter>(
-        GetWebContents());
-  }
-
  private:
   content::RenderFrameHost* CreateFrameImpl(
       const content::ToRenderFrameHost& adapter,
@@ -248,9 +241,9 @@ IN_PROC_BROWSER_TEST_F(AdTaggingBrowserTest, VerifyCrossOriginWithoutNavigate) {
 
   // Navigate away and ensure we report cross origin.
   ui_test_utils::NavigateToURL(browser(), GetURL(url::kAboutBlankURL));
-
-  // TODO(johnidel): Check that frame was reported properly. See
-  // crbug.com/914893.
+  histogram_tester.ExpectUniqueSample(
+      kAllOriginStatusHistogram,
+      AdsPageLoadMetricsObserver::AdOriginStatus::kCross, 1);
 }
 
 // Ad script creates a frame and navigates it cross origin.
@@ -258,18 +251,12 @@ IN_PROC_BROWSER_TEST_F(AdTaggingBrowserTest,
                        VerifyCrossOriginWithImmediateNavigate) {
   base::HistogramTester histogram_tester;
 
-  auto waiter = CreatePageLoadMetricsTestWaiter();
   // Create the main frame and cross origin subframe from an ad script.
   // This triggers both subresource_filter and Google ad detection.
   ui_test_utils::NavigateToURL(browser(), GetURL("frame_factory.html"));
   CreateSrcFrameFromAdScript(GetWebContents(),
                              embedded_test_server()->GetURL(
                                  "b.com", "/ads_observer/same_origin_ad.html"));
-  // Wait for all of the subresources to finish loading (includes favicon).
-  // Waiting for the navigation to finish is not sufficient, as it blocks on the
-  // main resource load finishing, not the iframe resource.
-  waiter->AddMinimumCompleteResourcesExpectation(4);
-  waiter->Wait();
 
   // Navigate away and ensure we report cross origin.
   ui_test_utils::NavigateToURL(browser(), GetURL(url::kAboutBlankURL));

chrome/common/page_load_metrics/page_load_metrics.mojom

@@ -199,9 +199,6 @@ struct ResourceDataUpdate {
   // Whether this resource was fetched from the http cache.
   bool was_fetched_via_cache;
 
-  // Whether this resource is the primary resource for a frame.
-  bool is_primary_frame_resource;
-
   // Mime type for the resource found in the network response header.
   string mime_type;
 

chrome/renderer/page_load_metrics/metrics_render_frame_observer.cc

@@ -106,10 +106,10 @@ void MetricsRenderFrameObserver::DidStartResponse(
     // case. There should be a guarantee that DidStartProvisionalLoad be called
     // before DidStartResponse for the frame request.
     provisional_frame_resource_data_use_->DidStartResponse(
-        response_url, request_id, response_head, resource_type);
+        response_url, request_id, response_head);
   } else if (page_timing_metrics_sender_) {
     page_timing_metrics_sender_->DidStartResponse(response_url, request_id,
-                                                  response_head, resource_type);
+                                                  response_head);
     UpdateResourceMetadata(request_id);
   }
 }

chrome/renderer/page_load_metrics/page_resource_data_use.cc

@@ -32,8 +32,7 @@ PageResourceDataUse::~PageResourceDataUse() = default;
 void PageResourceDataUse::DidStartResponse(
     const GURL& response_url,
     int resource_id,
-    const network::ResourceResponseHead& response_head,
-    content::ResourceType resource_type) {
+    const network::ResourceResponseHead& response_head) {
   resource_id_ = resource_id;
   data_reduction_proxy_compression_ratio_estimate_ =
       data_reduction_proxy::EstimateCompressionRatioFromHeaders(&response_head);
@@ -41,9 +40,6 @@ void PageResourceDataUse::DidStartResponse(
   mime_type_ = response_head.mime_type;
   was_fetched_via_cache_ = response_head.was_fetched_via_cache;
   is_secure_scheme_ = response_url.SchemeIsCryptographic();
-  is_primary_frame_resource_ =
-      resource_type == content::RESOURCE_TYPE_MAIN_FRAME ||
-      resource_type == content::RESOURCE_TYPE_SUB_FRAME;
 }
 
 void PageResourceDataUse::DidReceiveTransferSizeUpdate(
@@ -104,7 +100,6 @@ mojom::ResourceDataUpdatePtr PageResourceDataUse::GetResourceDataUpdate() {
   resource_data_update->was_fetched_via_cache = was_fetched_via_cache_;
   resource_data_update->is_secure_scheme = is_secure_scheme_;
   resource_data_update->proxy_used = proxy_used_;
-  resource_data_update->is_primary_frame_resource = is_primary_frame_resource_;
   return resource_data_update;
 }
 }  // namespace page_load_metrics

chrome/renderer/page_load_metrics/page_resource_data_use.h

@@ -7,7 +7,6 @@
 
 #include "base/macros.h"
 #include "chrome/common/page_load_metrics/page_load_metrics.mojom.h"
-#include "content/public/common/resource_type.h"
 
 class GURL;
 
@@ -28,8 +27,7 @@ class PageResourceDataUse {
 
   void DidStartResponse(const GURL& response_url,
                         int resource_id,
-                        const network::ResourceResponseHead& response_head,
-                        content::ResourceType resource_type);
+                        const network::ResourceResponseHead& response_head);
 
   // Updates received bytes.
   void DidReceiveTransferSizeUpdate(int received_data_length);
@@ -76,7 +74,6 @@ class PageResourceDataUse {
   bool was_fetched_via_cache_;
   bool is_secure_scheme_;
   bool proxy_used_;
-  bool is_primary_frame_resource_ = false;
 
   std::string mime_type_;
 

chrome/renderer/page_load_metrics/page_timing_metrics_sender.cc

@@ -100,15 +100,14 @@ void PageTimingMetricsSender::DidObserveLayoutJank(double jank_fraction) {
 void PageTimingMetricsSender::DidStartResponse(
     const GURL& response_url,
     int resource_id,
-    const network::ResourceResponseHead& response_head,
-    content::ResourceType resource_type) {
+    const network::ResourceResponseHead& response_head) {
   DCHECK(!base::ContainsKey(page_resource_data_use_, resource_id));
 
   auto resource_it = page_resource_data_use_.emplace(
       std::piecewise_construct, std::forward_as_tuple(resource_id),
       std::forward_as_tuple(std::make_unique<PageResourceDataUse>()));
   resource_it.first->second->DidStartResponse(response_url, resource_id,
-                                              response_head, resource_type);
+                                              response_head);
 }
 
 void PageTimingMetricsSender::DidReceiveTransferSizeUpdate(

chrome/renderer/page_load_metrics/page_timing_metrics_sender.h

@@ -49,8 +49,7 @@ class PageTimingMetricsSender {
   void DidObserveLayoutJank(double jank_fraction);
   void DidStartResponse(const GURL& response_url,
                         int resource_id,
-                        const network::ResourceResponseHead& response_head,
-                        content::ResourceType resource_type);
+                        const network::ResourceResponseHead& response_head);
   void DidReceiveTransferSizeUpdate(int resource_id, int received_data_length);
   void DidCompleteResponse(int resource_id,
                            const network::URLLoaderCompletionStatus& status);

chrome/test/data/ads_observer/docwrite_blank_frame.html

-<html>
-<iframe id="blank_frame" name="google_ads_iframe"></iframe>
-
-<script>
-  let iframe = document.getElementById("blank_frame");
-  let doc = iframe.contentWindow.document;
-  doc.open();
-  doc.write("<html>Rewritten. <img src=pixel.png> <img src=pixel2.png> <img src=pixel3.png></html>");
-  doc.close();
-</script>
-
-</html>