Remove ResourceMetricsObserver and all histograms

These histograms are no longer used and account for some of the larger
histograms recorded by Chrome due to being per-resource.

Bug: 1135295
Change-Id: I2a19101d049209f2894625c03c851034cff16a7a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2521367
Commit-Queue: John Delaney <johnidel@chromium.org>
Reviewed-by: Josh Karlin <jkarlin@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Cr-Commit-Position: refs/heads/master@{#825639}

chrome/browser/BUILD.gn

@@ -1032,8 +1032,6 @@ static_library("browser") {
     "page_load_metrics/observers/previews_ukm_observer.h",
     "page_load_metrics/observers/protocol_page_load_metrics_observer.cc",
     "page_load_metrics/observers/protocol_page_load_metrics_observer.h",
-    "page_load_metrics/observers/resource_metrics_observer.cc",
-    "page_load_metrics/observers/resource_metrics_observer.h",
     "page_load_metrics/observers/scheme_page_load_metrics_observer.cc",
     "page_load_metrics/observers/scheme_page_load_metrics_observer.h",
     "page_load_metrics/observers/security_state_page_load_metrics_observer.cc",

chrome/browser/page_load_metrics/observers/resource_metrics_observer.cc

-// Copyright 2019 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/page_load_metrics/observers/resource_metrics_observer.h"
-
-#include "components/page_load_metrics/browser/page_load_metrics_util.h"
-#include "components/page_load_metrics/browser/resource_tracker.h"
-
-namespace {
-
-#define RESOURCE_BYTES_HISTOGRAM(suffix, was_cached, value)                \
-  if (was_cached) {                                                        \
-    PAGE_BYTES_HISTOGRAM("Ads.ResourceUsage.Size.Cache2." suffix, value);  \
-  } else {                                                                 \
-    PAGE_BYTES_HISTOGRAM("Ads.ResourceUsage.Size.Network." suffix, value); \
-  }
-
-}  // namespace
-
-ResourceMetricsObserver::ResourceMetricsObserver() {}
-
-ResourceMetricsObserver::~ResourceMetricsObserver() {}
-
-void ResourceMetricsObserver::OnResourceDataUseObserved(
-    content::RenderFrameHost* content,
-    const std::vector<page_load_metrics::mojom::ResourceDataUpdatePtr>&
-        resources) {
-  for (auto const& resource : resources) {
-    if (resource->is_complete)
-      RecordResourceHistograms(resource);
-  }
-}
-
-page_load_metrics::PageLoadMetricsObserver::ObservePolicy
-ResourceMetricsObserver::FlushMetricsOnAppEnterBackground(
-    const page_load_metrics::mojom::PageLoadTiming& timing) {
-  // TODO(johnidel): This logic was maintained when resource metrics were moved
-  // out of the AdsPageLoadMetricsObserver. These metrics don't need to stop
-  // being reported when backgrounded.
-  if (GetDelegate().DidCommit()) {
-    OnComplete(timing);
-  }
-
-  return STOP_OBSERVING;
-}
-
-void ResourceMetricsObserver::OnComplete(
-    const page_load_metrics::mojom::PageLoadTiming& timing) {
-  for (auto const& kv :
-       GetDelegate().GetResourceTracker().unfinished_resources())
-    RecordResourceHistograms(kv.second);
-}
-
-void ResourceMetricsObserver::RecordResourceMimeHistograms(
-    const page_load_metrics::mojom::ResourceDataUpdatePtr& resource) {
-  bool was_cached =
-      resource->cache_type != page_load_metrics::mojom::CacheType::kNotCached;
-  int64_t data_length = was_cached ? resource->encoded_body_length
-                                   : resource->received_data_length;
-  ResourceMimeType mime_type =
-      ad_metrics::ResourceLoadAggregator::GetResourceMimeType(resource);
-  if (mime_type == ResourceMimeType::kImage) {
-    RESOURCE_BYTES_HISTOGRAM("Mime.Image", was_cached, data_length);
-  } else if (mime_type == ResourceMimeType::kJavascript) {
-    RESOURCE_BYTES_HISTOGRAM("Mime.JS", was_cached, data_length);
-  } else if (mime_type == ResourceMimeType::kVideo) {
-    RESOURCE_BYTES_HISTOGRAM("Mime.Video", was_cached, data_length);
-  } else if (mime_type == ResourceMimeType::kCss) {
-    RESOURCE_BYTES_HISTOGRAM("Mime.CSS", was_cached, data_length);
-  } else if (mime_type == ResourceMimeType::kHtml) {
-    RESOURCE_BYTES_HISTOGRAM("Mime.HTML", was_cached, data_length);
-  } else if (mime_type == ResourceMimeType::kOther) {
-    RESOURCE_BYTES_HISTOGRAM("Mime.Other", was_cached, data_length);
-  }
-}
-
-void ResourceMetricsObserver::RecordResourceHistograms(
-    const page_load_metrics::mojom::ResourceDataUpdatePtr& resource) {
-  bool was_cached =
-      resource->cache_type != page_load_metrics::mojom::CacheType::kNotCached;
-  int64_t data_length = was_cached ? resource->encoded_body_length
-                                   : resource->received_data_length;
-  if (resource->is_main_frame_resource && resource->reported_as_ad_resource) {
-    RESOURCE_BYTES_HISTOGRAM("Mainframe.AdResource", was_cached, data_length);
-  } else if (resource->is_main_frame_resource) {
-    RESOURCE_BYTES_HISTOGRAM("Mainframe.VanillaResource", was_cached,
-                             data_length);
-  } else if (resource->reported_as_ad_resource) {
-    RESOURCE_BYTES_HISTOGRAM("Subframe.AdResource", was_cached, data_length);
-  } else {
-    RESOURCE_BYTES_HISTOGRAM("Subframe.VanillaResource", was_cached,
-                             data_length);
-  }
-
-  // Only report sizes by mime type for ad resources.
-  if (resource->reported_as_ad_resource)
-    RecordResourceMimeHistograms(resource);
-}

chrome/browser/page_load_metrics/observers/resource_metrics_observer.h

-// Copyright 2019 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.
-
-#ifndef CHROME_BROWSER_PAGE_LOAD_METRICS_OBSERVERS_RESOURCE_METRICS_OBSERVER_H_
-#define CHROME_BROWSER_PAGE_LOAD_METRICS_OBSERVERS_RESOURCE_METRICS_OBSERVER_H_
-
-#include <map>
-#include <memory>
-
-#include "base/macros.h"
-#include "chrome/browser/page_load_metrics/observers/ad_metrics/frame_data.h"
-#include "components/page_load_metrics/browser/page_load_metrics_observer.h"
-#include "components/page_load_metrics/common/page_load_metrics.mojom-forward.h"
-
-// Observer that records histograms for individual resources loaded by the page.
-class ResourceMetricsObserver
-    : public page_load_metrics::PageLoadMetricsObserver {
- public:
-  using ResourceMimeType = ad_metrics::ResourceMimeType;
-  ResourceMetricsObserver();
-  ~ResourceMetricsObserver() override;
-
-  // page_load_metrics::PageLoadMetricsObserver
-  void OnResourceDataUseObserved(
-      content::RenderFrameHost* content,
-      const std::vector<page_load_metrics::mojom::ResourceDataUpdatePtr>&
-          resources) override;
-  ObservePolicy FlushMetricsOnAppEnterBackground(
-      const page_load_metrics::mojom::PageLoadTiming& timing) override;
-  void OnComplete(
-      const page_load_metrics::mojom::PageLoadTiming& timing) override;
-
- private:
-  // Records per-resource histograms.
-  void RecordResourceHistograms(
-      const page_load_metrics::mojom::ResourceDataUpdatePtr& resource);
-
-  // Records size of resources by mime type.
-  void RecordResourceMimeHistograms(
-      const page_load_metrics::mojom::ResourceDataUpdatePtr& resource);
-
-  DISALLOW_COPY_AND_ASSIGN(ResourceMetricsObserver);
-};
-
-#endif  // CHROME_BROWSER_PAGE_LOAD_METRICS_OBSERVERS_RESOURCE_METRICS_OBSERVER_H_

chrome/browser/page_load_metrics/observers/resource_metrics_observer_browsertest.cc

-// Copyright 2019 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 "base/test/metrics/histogram_tester.h"
-#include "chrome/browser/browser_process.h"
-#include "chrome/browser/page_load_metrics/observers/resource_metrics_observer.h"
-#include "chrome/browser/subresource_filter/subresource_filter_browser_test_harness.h"
-#include "chrome/browser/ui/browser.h"
-#include "chrome/test/base/in_process_browser_test.h"
-#include "chrome/test/base/ui_test_utils.h"
-#include "components/page_load_metrics/browser/page_load_metrics_test_waiter.h"
-#include "components/subresource_filter/content/browser/ruleset_service.h"
-#include "components/subresource_filter/core/browser/subresource_filter_features.h"
-#include "components/subresource_filter/core/common/activation_scope.h"
-#include "components/subresource_filter/core/common/common_features.h"
-#include "components/subresource_filter/core/common/test_ruleset_utils.h"
-#include "components/subresource_filter/core/mojom/subresource_filter.mojom.h"
-#include "content/public/test/browser_test.h"
-#include "content/public/test/browser_test_utils.h"
-#include "net/dns/mock_host_resolver.h"
-#include "net/test/embedded_test_server/controllable_http_response.h"
-#include "net/test/embedded_test_server/embedded_test_server.h"
-
-class ResourceMetricsObserverBrowserTest
-    : public subresource_filter::SubresourceFilterBrowserTest {
- public:
-  ResourceMetricsObserverBrowserTest() {}
-
-  ~ResourceMetricsObserverBrowserTest() override {}
-  void SetUpOnMainThread() override {
-    host_resolver()->AddRule("*", "127.0.0.1");
-    SetRulesetWithRules(
-        {subresource_filter::testing::CreateSuffixRule("ad_script.js"),
-         subresource_filter::testing::CreateSuffixRule("ad_script_2.js")});
-    embedded_test_server()->ServeFilesFromSourceDirectory(
-        "chrome/test/data/ad_tagging");
-  }
-
-  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);
-  }
-};
-
-IN_PROC_BROWSER_TEST_F(ResourceMetricsObserverBrowserTest,
-                       RecordUnfinishedResourceToMetrics) {
-  base::HistogramTester histogram_tester;
-  auto waiter = CreatePageLoadMetricsTestWaiter();
-  SetRulesetWithRules(
-      {subresource_filter::testing::CreateSuffixRule("ad_iframe_writer.js")});
-  embedded_test_server()->ServeFilesFromSourceDirectory(
-      "chrome/test/data/ads_observer");
-  content::SetupCrossSiteRedirector(embedded_test_server());
-
-  const char kHttpResponseHeader[] =
-      "HTTP/1.1 200 OK\r\n"
-      "Content-Type: text/html; charset=utf-8\r\n"
-      "\r\n";
-  auto incomplete_resource_response =
-      std::make_unique<net::test_server::ControllableHttpResponse>(
-          embedded_test_server(), "/incomplete_resource.js",
-          true /*relative_url_is_prefix*/);
-  ASSERT_TRUE(embedded_test_server()->Start());
-
-  GURL url = embedded_test_server()->GetURL(
-      "foo.com", "/ad_with_incomplete_resource.html");
-  ui_test_utils::NavigateToURL(browser(), url);
-
-  waiter->AddMinimumCompleteResourcesExpectation(3);
-  waiter->Wait();
-
-  incomplete_resource_response->WaitForRequest();
-  incomplete_resource_response->Send(kHttpResponseHeader);
-  incomplete_resource_response->Send(std::string(2048, ' '));
-
-  // Wait for the resource update to be received for the incomplete response.
-  waiter->AddMinimumNetworkBytesExpectation(2048);
-  waiter->Wait();
-
-  // Verify correct numbers of resources are recorded. A favicon is
-  // automatically loaded.
-  histogram_tester.ExpectTotalCount(
-      "Ads.ResourceUsage.Size.Network.Mainframe.VanillaResource", 2);
-  histogram_tester.ExpectTotalCount(
-      "Ads.ResourceUsage.Size.Network.Mainframe.AdResource", 1);
-
-  // Unfinished resource not yet recorded.
-  histogram_tester.ExpectTotalCount(
-      "Ads.ResourceUsage.Size.Network.Subframe.AdResource", 0);
-  histogram_tester.ExpectTotalCount(
-      "Ads.ResourceUsage.Size.Network.Subframe.VanillaResource", 0);
-
-  // Close all tabs instead of navigating as the embedded_test_server will
-  // hang waiting for loads to finish when we have an unfinished
-  // ControllableHttpResponse.
-  browser()->tab_strip_model()->CloseAllTabs();
-
-  // Verify unfinished resource recorded when page is destroyed.
-  histogram_tester.ExpectUniqueSample(
-      "Ads.ResourceUsage.Size.Network.Subframe.AdResource", 2, 1);
-}
-
-// Verify that per-resource metrics are reported for cached resources and
-// resources loaded by the network.
-IN_PROC_BROWSER_TEST_F(ResourceMetricsObserverBrowserTest,
-                       RecordedCacheResourceMetrics) {
-  base::HistogramTester histogram_tester;
-  content::SetupCrossSiteRedirector(embedded_test_server());
-  ASSERT_TRUE(embedded_test_server()->Start());
-  auto waiter = CreatePageLoadMetricsTestWaiter();
-  ui_test_utils::NavigateToURL(
-      browser(), embedded_test_server()->GetURL("foo.com", "/cachetime"));
-
-  // Wait for the favicon to be fetched.
-  waiter->AddMinimumCompleteResourcesExpectation(2);
-  waiter->Wait();
-
-  // All resources should have been loaded by network.
-  histogram_tester.ExpectTotalCount(
-      "Ads.ResourceUsage.Size.Network.Mainframe.VanillaResource", 2);
-
-  // Open a new tab and navigate so that resources are fetched via the disk
-  // cache. Navigating to the same URL in the same tab triggers a refresh which
-  // will not check the disk cache.
-  ui_test_utils::NavigateToURLWithDisposition(
-      browser(), GURL("about:blank"), WindowOpenDisposition::NEW_FOREGROUND_TAB,
-      ui_test_utils::BROWSER_TEST_WAIT_FOR_TAB |
-          ui_test_utils::BROWSER_TEST_WAIT_FOR_LOAD_STOP);
-  waiter = CreatePageLoadMetricsTestWaiter();
-  ui_test_utils::NavigateToURL(
-      browser(), embedded_test_server()->GetURL("foo.com", "/cachetime"));
-
-  // Wait for the resource to be fetched.
-  waiter->AddMinimumCompleteResourcesExpectation(1);
-  waiter->Wait();
-
-  // Resource should be recorded as loaded from the cache. Favicon not
-  // fetched this time.
-  histogram_tester.ExpectTotalCount(
-      "Ads.ResourceUsage.Size.Cache2.Mainframe.VanillaResource", 1);
-}
-
-// Verify that Mime type metrics are recorded correctly.
-IN_PROC_BROWSER_TEST_F(ResourceMetricsObserverBrowserTest,
-                       RecordedMimeMetrics) {
-  base::HistogramTester histogram_tester;
-  content::SetupCrossSiteRedirector(embedded_test_server());
-  ASSERT_TRUE(embedded_test_server()->Start());
-
-  auto waiter = CreatePageLoadMetricsTestWaiter();
-
-  content::WebContents* contents =
-      browser()->tab_strip_model()->GetActiveWebContents();
-  GURL url = embedded_test_server()->GetURL("foo.com", "/frame_factory.html");
-  ui_test_utils::NavigateToURL(browser(), url);
-  EXPECT_TRUE(
-      ExecJs(contents, "createAdFrame('multiple_mimes.html', 'test');"));
-  waiter->AddMinimumCompleteResourcesExpectation(11);
-  waiter->Wait();
-
-  // Close all tabs to log metrics, as the video resource request is incomplete.
-  browser()->tab_strip_model()->CloseAllTabs();
-
-  histogram_tester.ExpectTotalCount("Ads.ResourceUsage.Size.Network.Mime.HTML",
-                                    1);
-  histogram_tester.ExpectTotalCount("Ads.ResourceUsage.Size.Network.Mime.CSS",
-                                    1);
-  histogram_tester.ExpectTotalCount("Ads.ResourceUsage.Size.Network.Mime.JS",
-                                    3);
-
-  // Note: png and video/webm mime types are not set explicitly by the
-  // embedded_test_server.
-  histogram_tester.ExpectTotalCount("Ads.ResourceUsage.Size.Network.Mime.Image",
-                                    1);
-  histogram_tester.ExpectTotalCount("Ads.ResourceUsage.Size.Network.Mime.Video",
-                                    1);
-  histogram_tester.ExpectTotalCount("Ads.ResourceUsage.Size.Network.Mime.Other",
-                                    1);
-}

chrome/browser/page_load_metrics/page_load_metrics_initialize.cc

@@ -32,7 +32,6 @@
 #include "chrome/browser/page_load_metrics/observers/previews_page_load_metrics_observer.h"
 #include "chrome/browser/page_load_metrics/observers/previews_ukm_observer.h"
 #include "chrome/browser/page_load_metrics/observers/protocol_page_load_metrics_observer.h"
-#include "chrome/browser/page_load_metrics/observers/resource_metrics_observer.h"
 #include "chrome/browser/page_load_metrics/observers/scheme_page_load_metrics_observer.h"
 #include "chrome/browser/page_load_metrics/observers/security_state_page_load_metrics_observer.h"
 #include "chrome/browser/page_load_metrics/observers/service_worker_page_load_metrics_observer.h"
@@ -116,7 +115,6 @@ void PageLoadMetricsEmbedder::RegisterEmbedderObservers(
     tracker->AddObserver(
         std::make_unique<previews::PreviewsPageLoadMetricsObserver>());
     tracker->AddObserver(std::make_unique<previews::PreviewsUKMObserver>());
-    tracker->AddObserver(std::make_unique<ResourceMetricsObserver>());
     tracker->AddObserver(
         std::make_unique<ServiceWorkerPageLoadMetricsObserver>());
     tracker->AddObserver(

chrome/test/BUILD.gn

@@ -1186,7 +1186,6 @@ if (!is_android) {
       "../browser/page_load_metrics/observers/live_tab_count_page_load_metrics_observer_browsertest.cc",
       "../browser/page_load_metrics/observers/multi_tab_loading_page_load_metrics_observer_browsertest.cc",
       "../browser/page_load_metrics/observers/prefetch_proxy_page_load_metrics_observer_browsertest.cc",
-      "../browser/page_load_metrics/observers/resource_metrics_observer_browsertest.cc",
       "../browser/page_load_metrics/observers/security_state_page_load_metrics_observer_browsertest.cc",
       "../browser/page_load_metrics/observers/signed_exchange_page_load_metrics_browsertest.cc",
       "../browser/page_load_metrics/observers/third_party_metrics_observer_browsertest.cc",

tools/metrics/histograms/histograms_xml/others/histograms.xml

@@ -315,6 +315,9 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
 
 <histogram base="true" name="Ads.ResourceUsage.Size.Cache2" units="KB"
     expires_after="2020-10-02">
+  <obsolete>
+    Removed 11/2020.
+  </obsolete>
   <owner>johnidel@chromium.org</owner>
   <owner>jkarlin@chromium.org</owner>
   <summary>
@@ -330,6 +333,9 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
 
 <histogram base="true" name="Ads.ResourceUsage.Size.Network" units="KB"
     expires_after="2020-10-02">
+  <obsolete>
+    Removed 11/2020.
+  </obsolete>
   <owner>johnidel@chromium.org</owner>
   <owner>jkarlin@chromium.org</owner>
   <summary>