[subresource_filter] Convert DocumentLoadStatistics message to mojo

This CL keeps most usage of the DocumentLoadStatistics struct as a
normal struct and not a *Ptr, since we don't really need nullability.

Bug: 820612
Change-Id: Ib00002d6b5d320fa7bf4341812f3aca26da2e244
Reviewed-on: https://chromium-review.googlesource.com/1214506
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Josh Karlin <jkarlin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595192}

components/subresource_filter/content/browser/content_subresource_filter_throttle_manager.cc

@@ -196,18 +196,6 @@ void ContentSubresourceFilterThrottleManager::DidFinishLoad(
   statistics_->OnDidFinishLoad();
 }
 
-bool ContentSubresourceFilterThrottleManager::OnMessageReceived(
-    const IPC::Message& message,
-    content::RenderFrameHost* render_frame_host) {
-  bool handled = true;
-  IPC_BEGIN_MESSAGE_MAP(ContentSubresourceFilterThrottleManager, message)
-    IPC_MESSAGE_HANDLER(SubresourceFilterHostMsg_DocumentLoadStatistics,
-                        OnDocumentLoadStatistics)
-    IPC_MESSAGE_UNHANDLED(handled = false)
-  IPC_END_MESSAGE_MAP()
-  return handled;
-}
-
 // Sets the desired page-level |activation_state| for the currently ongoing
 // page load, identified by its main-frame |navigation_handle|. If this method
 // is not called for a main-frame navigation, the default behavior is no
@@ -382,12 +370,6 @@ void ContentSubresourceFilterThrottleManager::
   }
 }
 
-void ContentSubresourceFilterThrottleManager::OnDocumentLoadStatistics(
-    const DocumentLoadStatistics& statistics) {
-  if (statistics_)
-    statistics_->OnDocumentLoadStatistics(statistics);
-}
-
 void ContentSubresourceFilterThrottleManager::OnFrameIsAdSubframe(
     content::RenderFrameHost* render_frame_host) {
   DCHECK(render_frame_host);
@@ -405,6 +387,12 @@ void ContentSubresourceFilterThrottleManager::FrameIsAdSubframe() {
   OnFrameIsAdSubframe(binding_.GetCurrentTargetFrame());
 }
 
+void ContentSubresourceFilterThrottleManager::SetDocumentLoadStatistics(
+    mojom::DocumentLoadStatisticsPtr statistics) {
+  if (statistics_)
+    statistics_->OnDocumentLoadStatistics(*statistics);
+}
+
 void ContentSubresourceFilterThrottleManager::MaybeActivateSubframeSpecialUrls(
     content::NavigationHandle* navigation_handle) {
   if (navigation_handle->IsInMainFrame())

components/subresource_filter/content/browser/content_subresource_filter_throttle_manager.h

@@ -28,10 +28,6 @@ class NavigationThrottle;
 class RenderFrameHost;
 }  // namespace content
 
-namespace IPC {
-class Message;
-}  // namespace IPC
-
 namespace subresource_filter {
 
 class AsyncDocumentSubresourceFilter;
@@ -39,7 +35,6 @@ class ActivationStateComputingNavigationThrottle;
 class PageLoadStatistics;
 class SubresourceFilterObserverManager;
 class SubresourceFilterClient;
-struct DocumentLoadStatistics;
 
 // The ContentSubresourceFilterThrottleManager manages NavigationThrottles in
 // order to calculate frame activation states and subframe navigation filtering,
@@ -96,8 +91,6 @@ class ContentSubresourceFilterThrottleManager
       content::NavigationHandle* navigation_handle) override;
   void DidFinishLoad(content::RenderFrameHost* render_frame_host,
                      const GURL& validated_url) override;
-  bool OnMessageReceived(const IPC::Message& message,
-                         content::RenderFrameHost* render_frame_host) override;
 
   // SubresourceFilterObserver:
   void OnSubresourceFilterGoingAway() override;
@@ -137,8 +130,6 @@ class ContentSubresourceFilterThrottleManager
   VerifiedRuleset::Handle* EnsureRulesetHandle();
   void DestroyRulesetHandleIfNoLongerUsed();
 
-  void OnDocumentLoadStatistics(const DocumentLoadStatistics& statistics);
-
   // Registers |render_frame_host| as an ad frame. If the frame later moves to
   // a new process its RenderHost will be told that it's an ad.
   void OnFrameIsAdSubframe(content::RenderFrameHost* render_frame_host);
@@ -146,6 +137,8 @@ class ContentSubresourceFilterThrottleManager
   // mojom::SubresourceFilterHost:
   void DidDisallowFirstSubresource() override;
   void FrameIsAdSubframe() override;
+  void SetDocumentLoadStatistics(
+      mojom::DocumentLoadStatisticsPtr statistics) override;
 
   // Adds the navigation's RenderFrameHost to activated_frame_hosts_ if it is a
   // special navigation which did not go through navigation throttles and its

components/subresource_filter/content/browser/page_load_statistics.cc

@@ -16,7 +16,7 @@ PageLoadStatistics::PageLoadStatistics(const mojom::ActivationState& state)
 PageLoadStatistics::~PageLoadStatistics() {}
 
 void PageLoadStatistics::OnDocumentLoadStatistics(
-    const DocumentLoadStatistics& statistics) {
+    const mojom::DocumentLoadStatistics& statistics) {
   // Note: Chances of overflow are negligible.
   aggregated_document_statistics_.num_loads_total += statistics.num_loads_total;
   aggregated_document_statistics_.num_loads_evaluated +=

components/subresource_filter/content/browser/page_load_statistics.h

@@ -6,7 +6,6 @@
 #define COMPONENTS_SUBRESOURCE_FILTER_CONTENT_BROWSER_PAGE_LOAD_STATISTICS_H_
 
 #include "base/macros.h"
-#include "components/subresource_filter/core/common/document_load_statistics.h"
 #include "components/subresource_filter/mojom/subresource_filter.mojom.h"
 
 namespace subresource_filter {
@@ -19,7 +18,8 @@ class PageLoadStatistics {
   PageLoadStatistics(const mojom::ActivationState& state);
   ~PageLoadStatistics();
 
-  void OnDocumentLoadStatistics(const DocumentLoadStatistics& statistics);
+  void OnDocumentLoadStatistics(
+      const mojom::DocumentLoadStatistics& statistics);
   void OnDidFinishLoad();
 
  private:
@@ -27,7 +27,7 @@ class PageLoadStatistics {
 
   // Statistics about subresource loads, aggregated across all frames of the
   // current page.
-  DocumentLoadStatistics aggregated_document_statistics_;
+  mojom::DocumentLoadStatistics aggregated_document_statistics_;
 
   DISALLOW_COPY_AND_ASSIGN(PageLoadStatistics);
 };

components/subresource_filter/content/common/subresource_filter_messages.h

@@ -6,7 +6,6 @@
 // no-include-guard-because-multiply-included
 
 #include "base/time/time.h"
-#include "components/subresource_filter/core/common/document_load_statistics.h"
 #include "components/subresource_filter/mojom/subresource_filter.mojom.h"
 #include "content/public/common/common_param_traits_macros.h"
 #include "ipc/ipc_message.h"
@@ -27,15 +26,6 @@ IPC_STRUCT_TRAITS_BEGIN(subresource_filter::mojom::ActivationState)
   IPC_STRUCT_TRAITS_MEMBER(enable_logging)
 IPC_STRUCT_TRAITS_END()
 
-IPC_STRUCT_TRAITS_BEGIN(subresource_filter::DocumentLoadStatistics)
-  IPC_STRUCT_TRAITS_MEMBER(num_loads_total)
-  IPC_STRUCT_TRAITS_MEMBER(num_loads_evaluated)
-  IPC_STRUCT_TRAITS_MEMBER(num_loads_matching_rules)
-  IPC_STRUCT_TRAITS_MEMBER(num_loads_disallowed)
-  IPC_STRUCT_TRAITS_MEMBER(evaluation_total_wall_duration)
-  IPC_STRUCT_TRAITS_MEMBER(evaluation_total_cpu_duration)
-IPC_STRUCT_TRAITS_END()
-
 // ----------------------------------------------------------------------------
 // Messages sent from the browser to the renderer.
 // ----------------------------------------------------------------------------
@@ -63,16 +53,3 @@ IPC_MESSAGE_ROUTED2(
     SubresourceFilterMsg_ActivateForNextCommittedLoad,
     subresource_filter::mojom::ActivationState /* activation_state */,
     bool /* is_ad_subframe */)
-
-// ----------------------------------------------------------------------------
-// Messages sent from the renderer to the browser.
-// ----------------------------------------------------------------------------
-
-// This is sent to a RenderFrameHost in the browser when a document load is
-// finished, just before the DidFinishLoad message, and contains statistics
-// collected by the DocumentSubresourceFilter up until that point: the number of
-// subresources evaluated/disallowed/etc, and total time spent on evaluating
-// subresource loads in its allowLoad method. The time metrics are equal to zero
-// if performance measurements were disabled for the load.
-IPC_MESSAGE_ROUTED1(SubresourceFilterHostMsg_DocumentLoadStatistics,
-                    subresource_filter::DocumentLoadStatistics /* statistics */)

components/subresource_filter/content/renderer/subresource_filter_agent.cc

@@ -16,7 +16,6 @@
 #include "components/subresource_filter/content/common/subresource_filter_utils.h"
 #include "components/subresource_filter/content/renderer/unverified_ruleset_dealer.h"
 #include "components/subresource_filter/content/renderer/web_document_subresource_filter_impl.h"
-#include "components/subresource_filter/core/common/document_load_statistics.h"
 #include "components/subresource_filter/core/common/document_subresource_filter.h"
 #include "components/subresource_filter/core/common/memory_mapped_ruleset.h"
 #include "components/subresource_filter/core/common/scoped_timers.h"
@@ -71,9 +70,8 @@ void SubresourceFilterAgent::
 }
 
 void SubresourceFilterAgent::SendDocumentLoadStatistics(
-    const DocumentLoadStatistics& statistics) {
-  render_frame()->Send(new SubresourceFilterHostMsg_DocumentLoadStatistics(
-      render_frame()->GetRoutingID(), statistics));
+    const mojom::DocumentLoadStatistics& statistics) {
+  GetSubresourceFilterHost()->SetDocumentLoadStatistics(statistics.Clone());
 }
 
 void SubresourceFilterAgent::SendFrameIsAdSubframe() {

components/subresource_filter/content/renderer/subresource_filter_agent.h

@@ -21,7 +21,6 @@ class WebDocumentSubresourceFilter;
 
 namespace subresource_filter {
 
-struct DocumentLoadStatistics;
 class UnverifiedRulesetDealer;
 class WebDocumentSubresourceFilterImpl;
 
@@ -62,7 +61,7 @@ class SubresourceFilterAgent
 
   // Sends statistics about the DocumentSubresourceFilter's work to the browser.
   virtual void SendDocumentLoadStatistics(
-      const DocumentLoadStatistics& statistics);
+      const mojom::DocumentLoadStatistics& statistics);
 
   // Tells the browser that the frame is an ad subframe.
   virtual void SendFrameIsAdSubframe();

components/subresource_filter/content/renderer/subresource_filter_agent_unittest.cc

@@ -14,7 +14,6 @@
 #include "base/time/time.h"
 #include "components/subresource_filter/content/common/subresource_filter_messages.h"
 #include "components/subresource_filter/content/renderer/unverified_ruleset_dealer.h"
-#include "components/subresource_filter/core/common/document_load_statistics.h"
 #include "components/subresource_filter/core/common/memory_mapped_ruleset.h"
 #include "components/subresource_filter/core/common/scoped_timers.h"
 #include "components/subresource_filter/core/common/test_ruleset_creator.h"
@@ -52,7 +51,8 @@ class SubresourceFilterAgentUnderTest : public SubresourceFilterAgent {
   MOCK_METHOD0(GetDocumentURL, GURL());
   MOCK_METHOD0(OnSetSubresourceFilterForCommittedLoadCalled, void());
   MOCK_METHOD0(SignalFirstSubresourceDisallowedForCommittedLoad, void());
-  MOCK_METHOD1(SendDocumentLoadStatistics, void(const DocumentLoadStatistics&));
+  MOCK_METHOD1(SendDocumentLoadStatistics,
+               void(const mojom::DocumentLoadStatistics&));
   MOCK_METHOD0(SendFrameIsAdSubframe, void());
 
   bool IsMainFrame() override { return true; }

components/subresource_filter/core/common/BUILD.gn

@@ -13,7 +13,6 @@ static_library("common") {
     "activation_scope.h",
     "common_features.cc",
     "common_features.h",
-    "document_load_statistics.h",
     "document_subresource_filter.cc",
     "document_subresource_filter.h",
     "first_party_origin.cc",

components/subresource_filter/core/common/document_load_statistics.h

-// Copyright 2016 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 COMPONENTS_SUBRESOURCE_FILTER_CORE_COMMON_DOCUMENT_LOAD_STATISTICS_H_
-#define COMPONENTS_SUBRESOURCE_FILTER_CORE_COMMON_DOCUMENT_LOAD_STATISTICS_H_
-
-#include <stdint.h>
-
-#include "base/time/time.h"
-
-namespace subresource_filter {
-
-// Contains statistics and performance metrics collected by the
-// DocumentSubresourceFilter.
-struct DocumentLoadStatistics {
-  // The number of subresource loads that went through the
-  // DocumentSubresouceFilter filtering methods.
-  int32_t num_loads_total = 0;
-
-  // Statistics on the number of subresource loads that were evaluated, were
-  // matched by filtering rules, and were disallowed, respectively, during the
-  // lifetime of a DocumentSubresourceFilter.
-  int32_t num_loads_evaluated = 0;
-  int32_t num_loads_matching_rules = 0;
-  int32_t num_loads_disallowed = 0;
-
-  // Total time spent in GetLoadPolicy() calls evaluating subresource loads.
-  base::TimeDelta evaluation_total_wall_duration;
-  base::TimeDelta evaluation_total_cpu_duration;
-};
-
-}  // namespace subresource_filter
-
-#endif  // COMPONENTS_SUBRESOURCE_FILTER_CORE_COMMON_DOCUMENT_LOAD_STATISTICS_H_

components/subresource_filter/core/common/document_subresource_filter.h

@@ -12,7 +12,6 @@
 
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
-#include "components/subresource_filter/core/common/document_load_statistics.h"
 #include "components/subresource_filter/core/common/indexed_ruleset.h"
 #include "components/subresource_filter/core/common/load_policy.h"
 #include "components/subresource_filter/mojom/subresource_filter.mojom.h"
@@ -46,12 +45,14 @@ class DocumentSubresourceFilter {
   const mojom::ActivationState& activation_state() const {
     return activation_state_;
   }
-  const DocumentLoadStatistics& statistics() const { return statistics_; }
+  const mojom::DocumentLoadStatistics& statistics() const {
+    return statistics_;
+  }
 
   // WARNING: This is only to allow DocumentSubresourceFilter's wrappers to
   // modify the |statistics|.
   // TODO(pkalinnikov): Find a better way to achieve this.
-  DocumentLoadStatistics& statistics() { return statistics_; }
+  mojom::DocumentLoadStatistics& statistics() { return statistics_; }
 
   LoadPolicy GetLoadPolicy(
       const GURL& subresource_url,
@@ -77,7 +78,7 @@ class DocumentSubresourceFilter {
   // Equals nullptr iff |activation_state_.filtering_disabled_for_document|.
   std::unique_ptr<FirstPartyOrigin> document_origin_;
 
-  DocumentLoadStatistics statistics_;
+  mojom::DocumentLoadStatistics statistics_;
 
   DISALLOW_COPY_AND_ASSIGN(DocumentSubresourceFilter);
 };

components/subresource_filter/mojom/subresource_filter.mojom

@@ -4,6 +4,8 @@
 
 module subresource_filter.mojom;
 
+import "mojo/public/mojom/base/time.mojom";
+
 enum ActivationLevel {
   kDisabled,
   kDryRun,
@@ -37,6 +39,25 @@ struct ActivationState {
   bool enable_logging = false;
 };
 
+// Contains statistics and performance metrics collected by the
+// DocumentSubresourceFilter.
+struct DocumentLoadStatistics {
+  // The number of subresource loads that went through the
+  // DocumentSubresouceFilter filtering methods.
+  int32 num_loads_total = 0;
+
+  // Statistics on the number of subresource loads that were evaluated, were
+  // matched by filtering rules, and were disallowed, respectively, during the
+  // lifetime of a DocumentSubresourceFilter.
+  int32 num_loads_evaluated = 0;
+  int32 num_loads_matching_rules = 0;
+  int32 num_loads_disallowed = 0;
+
+  // Total time spent in GetLoadPolicy() calls evaluating subresource loads.
+  mojo_base.mojom.TimeDelta evaluation_total_wall_duration;
+  mojo_base.mojom.TimeDelta evaluation_total_cpu_duration;
+};
+
 interface SubresourceFilterHost {
   // Called the first time a subresource load is disallowed for the most
   // recently committed document load in a frame. It is used to trigger a UI
@@ -49,5 +70,13 @@ interface SubresourceFilterHost {
   // an ad via SubresourceFilterMsg_ActivateForNextCommittedLoad, at
   // ReadyToCommitNavigation time.
   FrameIsAdSubframe();
+
+  // This is sent to a RenderFrameHost in the browser when a document load is
+  // finished, just before the DidFinishLoad message, and contains statistics
+  // collected by the DocumentSubresourceFilter up until that point: the number
+  // of subresources evaluated/disallowed/etc, and total time spent on
+  // evaluating subresource loads in its allowLoad method. The time metrics are
+  // equal to zero if performance measurements were disabled for the load.
+  SetDocumentLoadStatistics(DocumentLoadStatistics statistics);
 };