[subresource_filter] Convert trivial routed messages to mojo

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

components/subresource_filter/content/DEPS

@@ -3,6 +3,7 @@ include_rules = [
   "+content/public/common",
   "+ipc",
   "+services/network/public/cpp",
+  "+third_party/blink/public/common",
 ]
 
 specific_include_rules = {

components/subresource_filter/content/browser/content_subresource_filter_throttle_manager.cc

@@ -38,6 +38,7 @@ ContentSubresourceFilterThrottleManager::
         VerifiedRulesetDealer::Handle* dealer_handle,
         content::WebContents* web_contents)
     : content::WebContentsObserver(web_contents),
+      binding_(web_contents, this),
       scoped_observer_(this),
       dealer_handle_(dealer_handle),
       client_(client),
@@ -170,7 +171,7 @@ void ContentSubresourceFilterThrottleManager::DidFinishNavigation(
   // this navigation's activation state.
   if (filter) {
     base::OnceClosure disallowed_callback(base::BindOnce(
-        &ContentSubresourceFilterThrottleManager::MaybeCallFirstDisallowedLoad,
+        &ContentSubresourceFilterThrottleManager::MaybeShowNotification,
         weak_ptr_factory_.GetWeakPtr()));
     filter->set_first_disallowed_load_callback(std::move(disallowed_callback));
     activated_frame_hosts_[frame_host] = std::move(filter);
@@ -198,19 +199,7 @@ bool ContentSubresourceFilterThrottleManager::OnMessageReceived(
     const IPC::Message& message,
     content::RenderFrameHost* render_frame_host) {
   bool handled = true;
-
-  IPC_BEGIN_MESSAGE_MAP_WITH_PARAM(ContentSubresourceFilterThrottleManager,
-                                   message, render_frame_host)
-    IPC_MESSAGE_HANDLER(SubresourceFilterHostMsg_FrameIsAdSubframe,
-                        OnFrameIsAdSubframe)
-    IPC_MESSAGE_UNHANDLED(handled = false)
-  IPC_END_MESSAGE_MAP()
-  if (handled)
-    return true;
-
   IPC_BEGIN_MESSAGE_MAP(ContentSubresourceFilterThrottleManager, message)
-    IPC_MESSAGE_HANDLER(SubresourceFilterHostMsg_DidDisallowFirstSubresource,
-                        MaybeCallFirstDisallowedLoad)
     IPC_MESSAGE_HANDLER(SubresourceFilterHostMsg_DocumentLoadStatistics,
                         OnDocumentLoadStatistics)
     IPC_MESSAGE_UNHANDLED(handled = false)
@@ -360,7 +349,7 @@ ContentSubresourceFilterThrottleManager::GetParentFrameFilter(
   return nullptr;
 }
 
-void ContentSubresourceFilterThrottleManager::MaybeCallFirstDisallowedLoad() {
+void ContentSubresourceFilterThrottleManager::MaybeShowNotification() {
   if (current_committed_load_has_notified_disallowed_load_)
     return;
 
@@ -406,6 +395,14 @@ void ContentSubresourceFilterThrottleManager::OnFrameIsAdSubframe(
       ->NotifyAdSubframeDetected(render_frame_host);
 }
 
+void ContentSubresourceFilterThrottleManager::DidDisallowFirstSubresource() {
+  MaybeShowNotification();
+}
+
+void ContentSubresourceFilterThrottleManager::FrameIsAdSubframe() {
+  OnFrameIsAdSubframe(binding_.GetCurrentTargetFrame());
+}
+
 void ContentSubresourceFilterThrottleManager::MaybeActivateSubframeSpecialUrls(
     content::NavigationHandle* navigation_handle) {
   if (navigation_handle->IsInMainFrame())

components/subresource_filter/content/browser/content_subresource_filter_throttle_manager.h

@@ -19,6 +19,8 @@
 #include "components/subresource_filter/content/browser/verified_ruleset_dealer.h"
 #include "components/subresource_filter/core/common/activation_decision.h"
 #include "components/subresource_filter/core/common/activation_state.h"
+#include "components/subresource_filter/mojom/subresource_filter.mojom.h"
+#include "content/public/browser/web_contents_binding_set.h"
 #include "content/public/browser/web_contents_observer.h"
 
 namespace content {
@@ -51,6 +53,7 @@ struct DocumentLoadStatistics;
 // navgation, and has veto power for frame activation.
 class ContentSubresourceFilterThrottleManager
     : public content::WebContentsObserver,
+      public mojom::SubresourceFilterHost,
       public SubresourceFilterObserver,
       public SubframeNavigationFilteringThrottle::Delegate {
  public:
@@ -128,9 +131,9 @@ class ContentSubresourceFilterThrottleManager
   AsyncDocumentSubresourceFilter* GetParentFrameFilter(
       content::NavigationHandle* child_frame_navigation);
 
-  // Calls OnFirstSubresourceLoadDisallowed on the Delegate at most once per
-  // committed, non-same-page navigation in the main frame.
-  void MaybeCallFirstDisallowedLoad();
+  // Calls ShowNotification on |client_| at most once per committed,
+  // non-same-page navigation in the main frame.
+  void MaybeShowNotification();
 
   VerifiedRuleset::Handle* EnsureRulesetHandle();
   void DestroyRulesetHandleIfNoLongerUsed();
@@ -141,6 +144,10 @@ class ContentSubresourceFilterThrottleManager
   // a new process its RenderHost will be told that it's an ad.
   void OnFrameIsAdSubframe(content::RenderFrameHost* render_frame_host);
 
+  // mojom::SubresourceFilterHost:
+  void DidDisallowFirstSubresource() override;
+  void FrameIsAdSubframe() 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
   // parent frame is activated as well. The filter for these frames is set
@@ -171,6 +178,8 @@ class ContentSubresourceFilterThrottleManager
   //    OOPIF)
   std::set<content::RenderFrameHost*> ad_frames_;
 
+  content::WebContentsFrameBindingSet<mojom::SubresourceFilterHost> binding_;
+
   ScopedObserver<SubresourceFilterObserverManager, SubresourceFilterObserver>
       scoped_observer_;
 

components/subresource_filter/content/browser/content_subresource_filter_throttle_manager_unittest.cc

@@ -941,10 +941,7 @@ TEST_P(ContentSubresourceFilterThrottleManagerTest,
   // Simulate the previous navigation sending an IPC that a load was disallowed.
   // This could happen e.g. for cross-process navigations, which have no
   // ordering guarantees.
-  throttle_manager()->OnMessageReceived(
-      SubresourceFilterHostMsg_DidDisallowFirstSubresource(
-          main_rfh()->GetRoutingID()),
-      main_rfh());
+  throttle_manager()->DidDisallowFirstSubresource();
   EXPECT_EQ(0, disallowed_notification_count());
 }
 

components/subresource_filter/content/common/subresource_filter_messages.h

@@ -68,11 +68,6 @@ IPC_MESSAGE_ROUTED2(SubresourceFilterMsg_ActivateForNextCommittedLoad,
 // Messages sent from the renderer to the browser.
 // ----------------------------------------------------------------------------
 
-// Sent to the browser the first time a subresource load is disallowed for the
-// most recently commited document load in a frame. It is used to trigger a
-// UI prompt to inform the user and allow them to turn off filtering.
-IPC_MESSAGE_ROUTED0(SubresourceFilterHostMsg_DidDisallowFirstSubresource)
-
 // 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
@@ -81,9 +76,3 @@ IPC_MESSAGE_ROUTED0(SubresourceFilterHostMsg_DidDisallowFirstSubresource)
 // if performance measurements were disabled for the load.
 IPC_MESSAGE_ROUTED1(SubresourceFilterHostMsg_DocumentLoadStatistics,
                     subresource_filter::DocumentLoadStatistics /* statistics */)
-
-// Sent to the browser when a RenderFrame is created and is tagged as an ad
-// subframe. The browser keeps track of this in case the frame later changes
-// processes, at which point it will inform the new RenderFrame that it has been
-// tagged as an ad via SubresourceFilterMsg_ActivateForNextCommittedLoad.
-IPC_MESSAGE_ROUTED0(SubresourceFilterHostMsg_FrameIsAdSubframe)

components/subresource_filter/content/renderer/subresource_filter_agent.cc

@@ -25,6 +25,7 @@
 #include "content/public/common/url_constants.h"
 #include "content/public/renderer/render_frame.h"
 #include "ipc/ipc_message.h"
+#include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"
 #include "third_party/blink/public/platform/web_worker_fetch_context.h"
 #include "third_party/blink/public/web/web_document.h"
 #include "third_party/blink/public/web/web_document_loader.h"
@@ -66,8 +67,7 @@ void SubresourceFilterAgent::SetSubresourceFilterForCommittedLoad(
 
 void SubresourceFilterAgent::
     SignalFirstSubresourceDisallowedForCommittedLoad() {
-  render_frame()->Send(new SubresourceFilterHostMsg_DidDisallowFirstSubresource(
-      render_frame()->GetRoutingID()));
+  GetSubresourceFilterHost()->DidDisallowFirstSubresource();
 }
 
 void SubresourceFilterAgent::SendDocumentLoadStatistics(
@@ -77,8 +77,7 @@ void SubresourceFilterAgent::SendDocumentLoadStatistics(
 }
 
 void SubresourceFilterAgent::SendFrameIsAdSubframe() {
-  render_frame()->Send(new SubresourceFilterHostMsg_FrameIsAdSubframe(
-      render_frame()->GetRoutingID()));
+  GetSubresourceFilterHost()->FrameIsAdSubframe();
 }
 
 bool SubresourceFilterAgent::IsAdSubframe() {
@@ -173,6 +172,15 @@ void SubresourceFilterAgent::ResetInfoForNextCommit() {
       ActivationState(mojom::ActivationLevel::kDisabled);
 }
 
+const mojom::SubresourceFilterHostAssociatedPtr&
+SubresourceFilterAgent::GetSubresourceFilterHost() {
+  if (!subresource_filter_host_) {
+    render_frame()->GetRemoteAssociatedInterfaces()->GetInterface(
+        &subresource_filter_host_);
+  }
+  return subresource_filter_host_;
+}
+
 void SubresourceFilterAgent::OnDestruct() {
   delete this;
 }

components/subresource_filter/content/renderer/subresource_filter_agent.h

@@ -11,6 +11,7 @@
 #include "base/memory/weak_ptr.h"
 #include "components/subresource_filter/content/renderer/ad_resource_tracker.h"
 #include "components/subresource_filter/core/common/activation_state.h"
+#include "components/subresource_filter/mojom/subresource_filter.mojom.h"
 #include "content/public/renderer/render_frame_observer.h"
 #include "content/public/renderer/render_frame_observer_tracker.h"
 #include "url/gurl.h"
@@ -83,6 +84,8 @@ class SubresourceFilterAgent
   void RecordHistogramsOnLoadFinished();
   void ResetInfoForNextCommit();
 
+  const mojom::SubresourceFilterHostAssociatedPtr& GetSubresourceFilterHost();
+
   // content::RenderFrameObserver:
   void OnDestruct() override;
   void DidCreateNewDocument() override;
@@ -101,6 +104,10 @@ class SubresourceFilterAgent
   // Tracks all ad resource observers.
   std::unique_ptr<AdResourceTracker> ad_resource_tracker_;
 
+  // Use associated interface to make sure mojo messages are ordered with regard
+  // to legacy IPC messages.
+  mojom::SubresourceFilterHostAssociatedPtr subresource_filter_host_;
+
   // If a document has been created for this frame before. The first document
   // for a new local subframe should be about:blank.
   bool first_document_ = true;

components/subresource_filter/mojom/subresource_filter.mojom

@@ -9,3 +9,17 @@ enum ActivationLevel {
   kDryRun,
   kEnabled
 };
+
+interface SubresourceFilterHost {
+  // Sent to the browser 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 prompt to inform the user and allow them to turn off filtering.
+  DidDisallowFirstSubresource();
+
+  // Sent to the browser when a RenderFrame is created and is tagged as an ad
+  // subframe. The browser keeps track of this in case the frame later changes
+  // processes, at which point it will inform the new RenderFrame that it has
+  // been tagged as an ad via SubresourceFilterMsg_ActivateForNextCommittedLoad,
+  // at ReadyToCommitNavigation time.
+  FrameIsAdSubframe();
+};