[PageLoadMetrics] Keep track of Ad Sizes on Pages

chrome/browser/page_load_metrics/observers/ads_page_load_metrics_observer.h

Index: chrome/browser/page_load_metrics/observers/ads_page_load_metrics_observer.h
diff --git a/chrome/browser/page_load_metrics/observers/ads_page_load_metrics_observer.h b/chrome/browser/page_load_metrics/observers/ads_page_load_metrics_observer.h
new file mode 100644
index 0000000000000000000000000000000000000000..aac07d679ae90216252b383435c7fa4a77443ee3
--- /dev/null
+++ b/chrome/browser/page_load_metrics/observers/ads_page_load_metrics_observer.h
@@ -0,0 +1,58 @@
+// Copyright 2017 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_ADS_PAGE_LOAD_METRICS_OBSERVER_H_
+#define CHROME_BROWSER_PAGE_LOAD_METRICS_OBSERVERS_ADS_PAGE_LOAD_METRICS_OBSERVER_H_
+
+#include <unordered_map>
+#include <unordered_set>

[Charlie Harrison, 2017/04/10 18:39:42]

not used

[jkarlin, 2017/04/11 16:46:28]

Done.

+
+#include "base/macros.h"
+#include "chrome/browser/page_load_metrics/page_load_metrics_observer.h"
+#include "net/http/http_response_info.h"
+
+class AdsPageLoadMetricsObserver
+    : public page_load_metrics::PageLoadMetricsObserver {
+ public:
+  AdsPageLoadMetricsObserver();
+  ~AdsPageLoadMetricsObserver() override;
+
+  // page_load_metrics::PageLoadMetricsObserver implementation:

[Charlie Harrison, 2017/04/10 18:39:41]

just "// page_load_metrics::PageLoadMetricsObserver:" for consistency.

[jkarlin, 2017/04/11 16:46:28]

Done.

+  ObservePolicy OnCommit(content::NavigationHandle* navigation_handle) override;
+  ObservePolicy OnCommitSubFrame(
+      content::NavigationHandle* navigation_handle) override;
+  ObservePolicy FlushMetricsOnAppEnterBackground(
+      const page_load_metrics::PageLoadTiming& timing,
+      const page_load_metrics::PageLoadExtraInfo& extra_info) override;
+  void OnLoadedResource(
+      const page_load_metrics::ExtraRequestInfo& extra_request_info) override;
+  void OnComplete(const page_load_metrics::PageLoadTiming& timing,
+                  const page_load_metrics::PageLoadExtraInfo& info) override;
+
+ private:
+  struct AdFrameData {
+    size_t frame_bytes;
+    size_t frame_bytes_uncached;
+  };
+
+  using FrameTreeNodeId = int;
+  const FrameTreeNodeId kInvalidFrameTreeNodeId = -1;
+
+  friend class AdsPageLoadMetricsObserverTest;
+
+  void RecordHistograms();
+  int FindAdAncestor(content::NavigationHandle* navigation_handle);
+  void ProcessDelayedResources(FrameTreeNodeId frame_tree_node_id);

[Charlie Harrison, 2017/04/10 18:39:41]

Please document this method.

[jkarlin, 2017/04/11 16:46:28]

Done.

+
+  std::unordered_map<FrameTreeNodeId, AdFrameData> ad_frames_;

[Charlie Harrison, 2017/04/10 18:39:42]

Haven't looked deeply but do you really need three maps? Can you consolidate?

[jkarlin, 2017/04/11 16:46:28]

A bit. ad_ancestors_ is now merged into ad_frames_, plus a new list to hold the
data.

+  std::unordered_map<FrameTreeNodeId, FrameTreeNodeId> ad_ancestors_;

[Charlie Harrison, 2017/04/10 18:39:41]

Can you document what the keys and values are?

[jkarlin, 2017/04/11 16:46:28]

Done.

+  std::unordered_map<FrameTreeNodeId, page_load_metrics::ExtraRequestInfo>

[Charlie Harrison, 2017/04/10 18:39:42]

Can you document what a "delayed" resource is?

[jkarlin, 2017/04/11 16:46:28]

Done.

+      delayed_resources_;
+  size_t page_bytes_ = 0;
+  size_t uncached_page_bytes_ = 0;
+
+  DISALLOW_COPY_AND_ASSIGN(AdsPageLoadMetricsObserver);
+};
+
+#endif  // CHROME_BROWSER_PAGE_LOAD_METRICS_OBSERVERS_ADS_PAGE_LOAD_METRICS_OBSERVER_H_