Remove FrameData class in the AdsPageLoadMetricsObserver.

This CL removes the FrameData class entirely, favoring small subclasses
AggregateFrameData and FrameTreeData that do the various types of
computations and repeating some of the high-level functionality to call
those subclasses correctly in each class.

It organizes all of the data into a FrameData namespace for easily
containing/referencing it and not having it pollute the namespace and
most/many of the enums are moved out of the classes to sit in that
namespace for general referencing.  It also removes main_frame_data
from the APLMO, in favor of storing the portions of it we wish to
know about in aggregate_frame_data as computational components using
the subclasses.  It also moves the collection of current memory usage
for individual frames to sit under APLMO, much like the resource
tracking information already does, to make memory collection more
generic and so we don't store multiple maps, some of which contain
only one entry.

After this, there are still a few more minor clean-ups that could
potentially be done:
1) Uniformity around naming accessors and updaters, as right now we
use some combination of GetValueType/value_type and UpdateValueType/
set_value_type.  I've done some fixing of this in this CL, but more
could be done.
2) Migration of Heavy Ads logic out of FrameData.  This should really
be performed as part of APLMO, but it sits in FrameData right now.
3) Migration of UKM reporting out of FrameData.  We report all UMA in
APLMO but report UKM as part of FrameData, and they should both be
done at the same level, presumably in APLMO.
4) Remove kAnyVisibility in favor of accessors that just add the two
visibilities together, as this would simplify the logic in many areas
where we loop over two visibilities ("any" and the appropriate one)
setting them both, rather than just setting one.

Bug: 1136068
Change-Id: Id65950726064b5a52ff08db21bf4e988339427a6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2496201
Commit-Queue: Eric Robinson <ericrobinson@chromium.org>
Reviewed-by: John Delaney <johnidel@chromium.org>
Reviewed-by: Josh Karlin <jkarlin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#825002}

chrome/browser/page_load_metrics/observers/ad_metrics/ads_page_load_metrics_observer.h

@@ -47,20 +47,9 @@ class AdsPageLoadMetricsObserver
       public performance_manager::v8_memory::V8DetailedMemoryObserverAnySeq,
       public subresource_filter::SubresourceFilterObserver {
  public:
-  // Returns a new AdsPageLoadMetricsObserver. If the feature is disabled it
-  // returns nullptr.
-  static std::unique_ptr<AdsPageLoadMetricsObserver> CreateIfNeeded(
-      content::WebContents* web_contents);
-
-  // For a given subframe, returns whether or not the subframe's url would be
-  // considering same origin to the main frame's url. |use_parent_origin|
-  // indicates that the subframe's parent frames's origin should be used when
-  // performing the comparison.
-  static bool IsSubframeSameOriginToMainFrame(
-      content::RenderFrameHost* sub_host,
-      bool use_parent_origin);
-
-  using ResourceMimeType = FrameData::ResourceMimeType;
+  using AggregateFrameData = ad_metrics::AggregateFrameData;
+  using FrameTreeData = ad_metrics::FrameTreeData;
+  using ResourceMimeType = ad_metrics::ResourceMimeType;
 
   // Helper class that generates a random amount of noise to apply to thresholds
   // for heavy ads. A different noise should be generated for each frame.
@@ -86,6 +75,19 @@ class AdsPageLoadMetricsObserver
     const bool use_noise_;
   };
 
+  // Returns a new AdsPageLoadMetricsObserver. If the feature is disabled it
+  // returns nullptr.
+  static std::unique_ptr<AdsPageLoadMetricsObserver> CreateIfNeeded(
+      content::WebContents* web_contents);
+
+  // For a given subframe, returns whether or not the subframe's url would be
+  // considering same origin to the main frame's url. |use_parent_origin|
+  // indicates that the subframe's parent frames's origin should be used when
+  // performing the comparison.
+  static bool IsSubframeSameOriginToMainFrame(
+      content::RenderFrameHost* sub_host,
+      bool use_parent_origin);
+
   explicit AdsPageLoadMetricsObserver(base::TickClock* clock = nullptr,
                                       HeavyAdBlocklist* blocklist = nullptr);
   ~AdsPageLoadMetricsObserver() override;
@@ -141,7 +143,7 @@ class AdsPageLoadMetricsObserver
       const V8DetailedMemoryObserverAnySeq::FrameDataMap& frame_data) override;
 
   void UpdateAggregateMemoryUsage(int64_t bytes,
-                                  FrameData::FrameVisibility visibility);
+                                  ad_metrics::FrameVisibility visibility);
 
   void CleanupDeletedFrame(FrameTreeNodeId id,
                            FrameTreeData* frame_data,
@@ -205,6 +207,14 @@ class AdsPageLoadMetricsObserver
       int process_id,
       const page_load_metrics::mojom::ResourceDataUpdatePtr& resource) const;
 
+  // Looks up the |frame_node_id| in the current memory usage map, updates it
+  // with the current bytes (or removes it), and returns the
+  // difference between the new value (0 if removed) and the old one, or the
+  // delta in the amount of memory the frame is using.
+  int64_t UpdateMemoryUsageForFrame(FrameTreeNodeId frame_node_id,
+                                    uint64_t current_bytes);
+  int64_t RemoveMemoryUsageForFrame(FrameTreeNodeId frame_node_id);
+
   // Updates page level counters for resource loads.
   void ProcessResourceForPage(
       int process_id,
@@ -217,7 +227,7 @@ class AdsPageLoadMetricsObserver
   void RecordHistograms(ukm::SourceId source_id);
   void RecordAggregateHistogramsForCpuUsage();
   void RecordAggregateHistogramsForAdTagging(
-      FrameData::FrameVisibility visibility);
+      ad_metrics::FrameVisibility visibility);
   void RecordAggregateHistogramsForHeavyAds();
 
   // Should be called on all frames prior to recording any aggregate histograms.
@@ -234,12 +244,12 @@ class AdsPageLoadMetricsObserver
 
   // Records whether an ad frame was ignored by the Restricted Navigation
   // AdTagging feature. For frames that are ignored, this is recorded when a
-  // FrameData object would have been created for them, or when their FrameData
-  // is deleted. For non-ignored frames, this is recorded when it is logged to
-  // metrics.
+  // FrameTreeData object would have been created for them, or when their
+  // FrameTreeData is deleted. For non-ignored frames, this is recorded when it
+  // is logged to metrics.
   void RecordAdFrameIgnoredByRestrictedAdTagging(bool ignored);
 
-  // Find the FrameData object associated with a given FrameTreeNodeId in
+  // Find the FrameTreeData object associated with a given FrameTreeNodeId in
   // |ad_frames_data_storage_|.
   FrameTreeData* FindFrameData(FrameTreeNodeId id);
 
@@ -255,8 +265,8 @@ class AdsPageLoadMetricsObserver
   HeavyAdBlocklist* GetHeavyAdBlocklist();
 
   // Maps a frame (by id) to the corresponding FrameInstance. Multiple frame ids
-  // can point to the same underlying FrameData. The responsible frame is the
-  // top-most frame labeled as an ad in the frame's ancestry, which may be
+  // can point to the same underlying FrameTreeData. The responsible frame is
+  // the top-most frame labeled as an ad in the frame's ancestry, which may be
   // itself. If the frame is not an ad, the id will point to a FrameInstance
   // where FrameInstance::Get() returns nullptr..
   std::map<FrameTreeNodeId, FrameInstance> ad_frames_data_;
@@ -270,8 +280,9 @@ class AdsPageLoadMetricsObserver
   std::map<FrameTreeNodeId, page_load_metrics::mojom::ResourceDataUpdatePtr>
       ongoing_navigation_resources_;
 
-  // Tracks byte counts only for resources loaded in the main frame.
-  std::unique_ptr<FrameTreeData> main_frame_data_;
+  // Per-frame memory usage by V8 in bytes. Memory data is stored for each frame
+  // on the page during the navigation.
+  std::unordered_map<FrameTreeNodeId, uint64_t> v8_current_memory_usage_map_;
 
   // Tracks page-level information for the navigation.
   std::unique_ptr<AggregateFrameData> aggregate_frame_data_;

chrome/browser/page_load_metrics/observers/ad_metrics/ads_page_load_metrics_observer_browsertest.cc

@@ -55,8 +55,8 @@
 
 namespace {
 
-using OriginStatus = FrameData::OriginStatus;
-using OriginStatusWithThrottling = FrameData::OriginStatusWithThrottling;
+using OriginStatus = ad_metrics::OriginStatus;
+using OriginStatusWithThrottling = ad_metrics::OriginStatusWithThrottling;
 
 using FrameTreeNodeId = int;
 
@@ -239,7 +239,7 @@ IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverBrowserTest,
   waiter->Wait();
   ui_test_utils::NavigateToURL(browser(), GURL(url::kAboutBlankURL));
   histogram_tester.ExpectUniqueSample(kCrossOriginHistogramId,
-                                      FrameData::OriginStatus::kSame, 1);
+                                      ad_metrics::OriginStatus::kSame, 1);
 }
 
 // Test that an empty embedded ad isn't reported at all.
@@ -270,13 +270,13 @@ IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverBrowserTest,
 
   ui_test_utils::NavigateToURL(browser(), GURL(url::kAboutBlankURL));
   histogram_tester.ExpectUniqueSample(kCrossOriginHistogramId,
-                                      FrameData::OriginStatus::kSame, 1);
+                                      ad_metrics::OriginStatus::kSame, 1);
   auto entries =
       ukm_recorder.GetEntriesByName(ukm::builders::AdFrameLoad::kEntryName);
   EXPECT_EQ(1u, entries.size());
   ukm_recorder.ExpectEntryMetric(
       entries.front(), ukm::builders::AdFrameLoad::kStatus_CrossOriginName,
-      static_cast<int>(FrameData::OriginStatus::kSame));
+      static_cast<int>(ad_metrics::OriginStatus::kSame));
 }
 
 // Test that an ad with a different origin as the main page is cross origin.
@@ -303,13 +303,13 @@ IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverBrowserTest,
   waiter->Wait();
   ui_test_utils::NavigateToURL(browser(), GURL(url::kAboutBlankURL));
   histogram_tester.ExpectUniqueSample(kCrossOriginHistogramId,
-                                      FrameData::OriginStatus::kCross, 1);
+                                      ad_metrics::OriginStatus::kCross, 1);
   auto entries =
       ukm_recorder.GetEntriesByName(ukm::builders::AdFrameLoad::kEntryName);
   EXPECT_EQ(1u, entries.size());
   ukm_recorder.ExpectEntryMetric(
       entries.front(), ukm::builders::AdFrameLoad::kStatus_CrossOriginName,
-      static_cast<int>(FrameData::OriginStatus::kCross));
+      static_cast<int>(ad_metrics::OriginStatus::kCross));
 }
 
 // Verifies that the page ad density records the maximum value during
@@ -675,8 +675,8 @@ class CreativeOriginAdsPageLoadMetricsObserverBrowserTest
 
   void TestCreativeOriginStatus(
       std::unique_ptr<Frame> main_frame,
-      FrameData::OriginStatus expected_status,
-      base::Optional<FrameData::OriginStatusWithThrottling>
+      ad_metrics::OriginStatus expected_status,
+      base::Optional<ad_metrics::OriginStatusWithThrottling>
           expected_status_with_throttling) {
     base::HistogramTester histogram_tester;
     bool subframe_exists = main_frame->HasChild();
@@ -742,8 +742,8 @@ IN_PROC_BROWSER_TEST_F(CreativeOriginAdsPageLoadMetricsObserverBrowserTest,
   TestCreativeOriginStatus(
       MakeFrame("a",
                 MakeFrame("a", MakeFrame("b", MakeFrame("c", nullptr), true))),
-      FrameData::OriginStatus::kCross,
-      FrameData::OriginStatusWithThrottling::kCrossAndUnthrottled);
+      ad_metrics::OriginStatus::kCross,
+      ad_metrics::OriginStatusWithThrottling::kCrossAndUnthrottled);
 }
 
 // Test that an ad creative with a different origin as the main page,
@@ -854,10 +854,10 @@ IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverBrowserTest,
   ui_test_utils::NavigateToURL(browser(), GURL(url::kAboutBlankURL));
   histogram_tester.ExpectBucketCount(
       kAdUserActivationHistogramId,
-      FrameData::UserActivationStatus::kReceivedActivation, 1);
+      ad_metrics::UserActivationStatus::kReceivedActivation, 1);
   histogram_tester.ExpectBucketCount(
       kAdUserActivationHistogramId,
-      FrameData::UserActivationStatus::kNoActivation, 1);
+      ad_metrics::UserActivationStatus::kNoActivation, 1);
   auto entries =
       ukm_recorder.GetEntriesByName(ukm::builders::AdFrameLoad::kEntryName);
   EXPECT_EQ(2u, entries.size());
@@ -1181,7 +1181,7 @@ IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverBrowserTest,
   EXPECT_EQ(1u, entries.size());
   ukm_recorder.ExpectEntryMetric(
       entries.front(), ukm::builders::AdFrameLoad::kStatus_MediaName,
-      static_cast<int>(FrameData::MediaStatus::kNotPlayed));
+      static_cast<int>(ad_metrics::MediaStatus::kNotPlayed));
 }
 
 // Flaky on all platforms, http://crbug.com/972822.
@@ -1221,7 +1221,7 @@ IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverBrowserTest,
   EXPECT_EQ(1u, entries.size());
   ukm_recorder.ExpectEntryMetric(
       entries.front(), ukm::builders::AdFrameLoad::kStatus_MediaName,
-      static_cast<int>(FrameData::MediaStatus::kPlayed));
+      static_cast<int>(ad_metrics::MediaStatus::kPlayed));
 }
 
 IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverBrowserTest,
@@ -1728,13 +1728,13 @@ IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverResourceBrowserTest,
   error_observer.WaitForNavigationFinished();
 
   histogram_tester.ExpectUniqueSample(kHeavyAdInterventionTypeHistogramId,
-                                      FrameData::HeavyAdStatus::kNetwork, 1);
+                                      ad_metrics::HeavyAdStatus::kNetwork, 1);
 
   // Check that the ad frame was navigated to the intervention page.
   EXPECT_FALSE(error_observer.last_navigation_succeeded());
 
   histogram_tester.ExpectUniqueSample(kHeavyAdInterventionTypeHistogramId,
-                                      FrameData::HeavyAdStatus::kNetwork, 1);
+                                      ad_metrics::HeavyAdStatus::kNetwork, 1);
   histogram_tester.ExpectBucketCount(
       "Blink.UseCounter.Features",
       blink::mojom::WebFeature::kHeavyAdIntervention, 1);
@@ -1859,13 +1859,13 @@ IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverResourceBrowserTest,
   error_observer.WaitForNavigationFinished();
 
   histogram_tester.ExpectUniqueSample(kHeavyAdInterventionTypeHistogramId,
-                                      FrameData::HeavyAdStatus::kNetwork, 1);
+                                      ad_metrics::HeavyAdStatus::kNetwork, 1);
 
   // Check that the ad frame was navigated to the intervention page.
   EXPECT_FALSE(error_observer.last_navigation_succeeded());
 
   histogram_tester.ExpectUniqueSample(kHeavyAdInterventionTypeHistogramId,
-                                      FrameData::HeavyAdStatus::kNetwork, 1);
+                                      ad_metrics::HeavyAdStatus::kNetwork, 1);
 
   EXPECT_TRUE(ExecJs(
       web_contents,
@@ -1883,7 +1883,7 @@ IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverResourceBrowserTest,
 
   // Check that the intervention did not trigger on this frame.
   histogram_tester.ExpectUniqueSample(kHeavyAdInterventionTypeHistogramId,
-                                      FrameData::HeavyAdStatus::kNetwork, 1);
+                                      ad_metrics::HeavyAdStatus::kNetwork, 1);
 }
 
 // Verifies that the blocklist is setup correctly and the intervention triggers
@@ -2009,7 +2009,8 @@ IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverBrowserTest,
   EXPECT_GE(elapsed_time.InMilliseconds(), 300);
 
   // Ensure that there is a single entry that is at least the percent specified.
-  int min_percent = 100 * 300 / FrameData::kCpuWindowSize.InMilliseconds();
+  int min_percent =
+      100 * 300 / ad_metrics::PeakCpuAggregator::kWindowSize.InMilliseconds();
   auto samples = histogram_tester.GetAllSamples(kPeakWindowdPercentHistogramId);
   EXPECT_EQ(1u, samples.size());
   EXPECT_EQ(1, samples.front().count);
@@ -2045,7 +2046,8 @@ IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverBrowserTest,
 
   // Ensure that there is a single entry that is at least the peak windowed
   // percent of 400ms.
-  int min_percent = 100 * 400 / FrameData::kCpuWindowSize.InMilliseconds();
+  int min_percent =
+      100 * 400 / ad_metrics::PeakCpuAggregator::kWindowSize.InMilliseconds();
   auto samples = histogram_tester.GetAllSamples(kPeakWindowdPercentHistogramId);
   EXPECT_EQ(1u, samples.size());
   EXPECT_EQ(1, samples.front().count);
@@ -2127,7 +2129,8 @@ IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverBrowserTest,
 
   // Ensure that there is a single entry that is at least the peak windowed
   // percent of 400ms.
-  int min_percent = 100 * 300 / FrameData::kCpuWindowSize.InMilliseconds();
+  int min_percent =
+      100 * 300 / ad_metrics::PeakCpuAggregator::kWindowSize.InMilliseconds();
   auto samples = histogram_tester.GetAllSamples(kPeakWindowdPercentHistogramId);
   EXPECT_EQ(1u, samples.size());
   EXPECT_EQ(1, samples.front().count);

chrome/browser/page_load_metrics/observers/resource_metrics_observer.cc

@@ -58,7 +58,8 @@ void ResourceMetricsObserver::RecordResourceMimeHistograms(
       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 = FrameData::GetResourceMimeType(resource);
+  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) {

chrome/browser/page_load_metrics/observers/resource_metrics_observer.h

@@ -17,7 +17,7 @@
 class ResourceMetricsObserver
     : public page_load_metrics::PageLoadMetricsObserver {
  public:
-  using ResourceMimeType = FrameData::ResourceMimeType;
+  using ResourceMimeType = ad_metrics::ResourceMimeType;
   ResourceMetricsObserver();
   ~ResourceMetricsObserver() override;
 

chrome/browser/subresource_filter/ad_tagging_browsertest.cc

@@ -414,7 +414,7 @@ IN_PROC_BROWSER_TEST_F(AdTaggingBrowserTest, VerifySameOriginWithoutNavigate) {
   // Navigate away and ensure we report same origin.
   ui_test_utils::NavigateToURL(browser(), GURL(url::kAboutBlankURL));
   histogram_tester.ExpectUniqueSample(kSubresourceFilterOriginStatusHistogram,
-                                      FrameData::OriginStatus::kSame, 1);
+                                      ad_metrics::OriginStatus::kSame, 1);
 }
 
 IN_PROC_BROWSER_TEST_F(AdTaggingBrowserTest, VerifyCrossOriginWithoutNavigate) {
@@ -457,7 +457,7 @@ IN_PROC_BROWSER_TEST_F(AdTaggingBrowserTest,
   // Navigate away and ensure we report cross origin.
   ui_test_utils::NavigateToURL(browser(), GURL(url::kAboutBlankURL));
   histogram_tester.ExpectUniqueSample(kSubresourceFilterOriginStatusHistogram,
-                                      FrameData::OriginStatus::kCross, 1);
+                                      ad_metrics::OriginStatus::kCross, 1);
 }
 
 // Ad script creates a frame and navigates it same origin.
@@ -479,7 +479,7 @@ IN_PROC_BROWSER_TEST_F(AdTaggingBrowserTest,
   // Navigate away and ensure we report same origin.
   ui_test_utils::NavigateToURL(browser(), GURL(url::kAboutBlankURL));
   histogram_tester.ExpectUniqueSample(kSubresourceFilterOriginStatusHistogram,
-                                      FrameData::OriginStatus::kSame, 1);
+                                      ad_metrics::OriginStatus::kSame, 1);
 }
 
 // Test that a subframe with a non-ad url but loaded by ad script is an ad.