Revert "Convert FrameMsg_EnforceInsecureRequestPolicy to blink RemoteFrame mojom"

This reverts commit 99fe56da.

Reason for revert: depends on https://chromium-review.googlesource.com/c/chromium/src/+/2045293 that broke navigation to facebook.com, see bug for details.

BUG=1055450

Original change's description:
> Convert FrameMsg_EnforceInsecureRequestPolicy to blink RemoteFrame mojom
> 
> This CL converts FrameMsg_EnforceInsecureRequestPolicy to method on
> blink RemoteFrame mojom. It also updates the unit test with
> intercepting RemoteFrame and removes checking the IPC message.
> 
> BUG=1047284
> 
> Change-Id: I9578c49cfcc08b3a682ec2899627dc6503965654
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2038513
> Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Commit-Queue: Julie Kim <jkim@igalia.com>
> Cr-Commit-Position: refs/heads/master@{#743924}

TBR=kinuko@chromium.org,haraken@chromium.org,dtapuska@chromium.org,jkim@igalia.com

Change-Id: I39e9cdaa9e63df07cfdca5105a1933ef3c6a860d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1047284
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2068683Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Commit-Queue: Andrey Kosyakov <caseq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#743991}

content/browser/frame_host/render_frame_host_manager.cc

@@ -1027,8 +1027,8 @@ void RenderFrameHostManager::OnDidResetContentSecurityPolicy() {
 void RenderFrameHostManager::OnEnforceInsecureRequestPolicy(
     blink::mojom::InsecureRequestPolicy policy) {
   for (const auto& pair : proxy_hosts_) {
-    pair.second->GetAssociatedRemoteFrame()->EnforceInsecureRequestPolicy(
-        policy);
+    pair.second->Send(new FrameMsg_EnforceInsecureRequestPolicy(
+        pair.second->GetRoutingID(), policy));
   }
 }
 

content/browser/frame_host/render_frame_host_manager_unittest.cc

@@ -87,6 +87,21 @@ void VerifyPageFocusMessage(TestRenderWidgetHost* twh, bool expected_focus) {
   EXPECT_EQ(expected_focus, focus_message->focused());
 }
 
+// Helper function for strict mixed content checking tests.
+void CheckInsecureRequestPolicyIPC(
+    TestRenderFrameHost* rfh,
+    blink::mojom::InsecureRequestPolicy expected_param,
+    int expected_routing_id) {
+  const IPC::Message* message =
+      rfh->GetProcess()->sink().GetUniqueMessageMatching(
+          FrameMsg_EnforceInsecureRequestPolicy::ID);
+  ASSERT_TRUE(message);
+  EXPECT_EQ(expected_routing_id, message->routing_id());
+  FrameMsg_EnforceInsecureRequestPolicy::Param params;
+  EXPECT_TRUE(FrameMsg_EnforceInsecureRequestPolicy::Read(message, &params));
+  EXPECT_EQ(expected_param, std::get<0>(params));
+}
+
 class RenderFrameHostManagerTestWebUIControllerFactory
     : public WebUIControllerFactory {
  public:
@@ -2902,53 +2917,6 @@ TEST_P(RenderFrameHostManagerTest, NavigateCrossSiteBetweenWebUIs) {
   EXPECT_FALSE(GetPendingFrameHost(manager));
 }
 
-// This class intercepts RenderFrameProxyHost creations, and overrides their
-// respective blink::mojom::RemoteFrame instances.
-class InsecureRequestPolicyProxyObserver {
- public:
-  InsecureRequestPolicyProxyObserver() {
-    RenderFrameProxyHost::SetCreatedCallbackForTesting(
-        base::BindRepeating(&InsecureRequestPolicyProxyObserver::
-                                RenderFrameProxyHostCreatedCallback,
-                            base::Unretained(this)));
-  }
-  ~InsecureRequestPolicyProxyObserver() {
-    RenderFrameProxyHost::SetCreatedCallbackForTesting(
-        RenderFrameProxyHost::CreatedCallback());
-  }
-  blink::mojom::InsecureRequestPolicy GetRequestPolicy(
-      RenderFrameProxyHost* proxy_host) {
-    return remote_frames_[proxy_host]->enforce_insecure_request_policy();
-  }
-
- private:
-  // Stub out remote frame mojo binding. Intercepts calls to
-  // EnforceInsecureRequestPolicy and marks the message as received.
-  class RemoteFrame : public content::FakeRemoteFrame {
-   public:
-    explicit RemoteFrame(RenderFrameProxyHost* render_frame_proxy_host) {
-      Init(render_frame_proxy_host->GetRemoteAssociatedInterfacesTesting());
-    }
-
-    void EnforceInsecureRequestPolicy(
-        blink::mojom::InsecureRequestPolicy policy) override {
-      enforce_insecure_request_policy_ = policy;
-    }
-    blink::mojom::InsecureRequestPolicy enforce_insecure_request_policy() {
-      return enforce_insecure_request_policy_;
-    }
-
-   private:
-    blink::mojom::InsecureRequestPolicy enforce_insecure_request_policy_;
-  };
-
-  void RenderFrameProxyHostCreatedCallback(RenderFrameProxyHost* proxy_host) {
-    remote_frames_[proxy_host] = std::make_unique<RemoteFrame>(proxy_host);
-  }
-
-  std::map<RenderFrameProxyHost*, std::unique_ptr<RemoteFrame>> remote_frames_;
-};
-
 // Tests that frame proxies receive updates when a frame's enforcement
 // of insecure request policy changes.
 TEST_P(RenderFrameHostManagerTestWithSiteIsolation,
@@ -2956,7 +2924,6 @@ TEST_P(RenderFrameHostManagerTestWithSiteIsolation,
   const GURL kUrl1("http://www.google.test");
   const GURL kUrl2("http://www.google2.test");
   const GURL kUrl3("http://www.google2.test/foo");
-  InsecureRequestPolicyProxyObserver observer;
 
   contents()->NavigateAndCommit(kUrl1);
 
@@ -2992,9 +2959,9 @@ TEST_P(RenderFrameHostManagerTestWithSiteIsolation,
   RenderFrameProxyHost* proxy_to_child =
       root->render_manager()->GetRenderFrameProxyHost(
           child_host->GetSiteInstance());
-  base::RunLoop().RunUntilIdle();
-  EXPECT_EQ(blink::mojom::InsecureRequestPolicy::kBlockAllMixedContent,
-            observer.GetRequestPolicy(proxy_to_child));
+  EXPECT_NO_FATAL_FAILURE(CheckInsecureRequestPolicyIPC(
+      child_host, blink::mojom::InsecureRequestPolicy::kBlockAllMixedContent,
+      proxy_to_child->GetRoutingID()));
   EXPECT_EQ(blink::mojom::InsecureRequestPolicy::kBlockAllMixedContent,
             root->current_replication_state().insecure_request_policy);
 
@@ -3011,9 +2978,10 @@ TEST_P(RenderFrameHostManagerTestWithSiteIsolation,
       blink::mojom::InsecureRequestPolicy::kBlockAllMixedContent);
   RenderFrameProxyHost* proxy_to_parent =
       child->GetRenderFrameProxyHost(main_test_rfh()->GetSiteInstance());
-  base::RunLoop().RunUntilIdle();
-  EXPECT_EQ(blink::mojom::InsecureRequestPolicy::kBlockAllMixedContent,
-            observer.GetRequestPolicy(proxy_to_parent));
+  EXPECT_NO_FATAL_FAILURE(CheckInsecureRequestPolicyIPC(
+      main_test_rfh(),
+      blink::mojom::InsecureRequestPolicy::kBlockAllMixedContent,
+      proxy_to_parent->GetRoutingID()));
   EXPECT_EQ(
       blink::mojom::InsecureRequestPolicy::kBlockAllMixedContent,
       root->child_at(0)->current_replication_state().insecure_request_policy);
@@ -3022,9 +2990,10 @@ TEST_P(RenderFrameHostManagerTestWithSiteIsolation,
   // when the child navigates.
   main_test_rfh()->GetProcess()->sink().ClearMessages();
   NavigationSimulator::NavigateAndCommitFromDocument(kUrl3, child_host);
-  base::RunLoop().RunUntilIdle();
-  EXPECT_EQ(blink::mojom::InsecureRequestPolicy::kLeaveInsecureRequestsAlone,
-            observer.GetRequestPolicy(proxy_to_parent));
+  EXPECT_NO_FATAL_FAILURE(CheckInsecureRequestPolicyIPC(
+      main_test_rfh(),
+      blink::mojom::InsecureRequestPolicy::kLeaveInsecureRequestsAlone,
+      proxy_to_parent->GetRoutingID()));
   EXPECT_EQ(
       blink::mojom::InsecureRequestPolicy::kLeaveInsecureRequestsAlone,
       root->child_at(0)->current_replication_state().insecure_request_policy);

content/common/frame_messages.h

@@ -558,6 +558,11 @@ IPC_MESSAGE_ROUTED2(FrameMsg_DidUpdateName,
                     std::string /* name */,
                     std::string /* unique_name */)
 
+// Update a proxy's replicated enforcement of insecure request policy.
+// Used when the frame's policy is changed in another process.
+IPC_MESSAGE_ROUTED1(FrameMsg_EnforceInsecureRequestPolicy,
+                    blink::mojom::InsecureRequestPolicy)
+
 // Send to the RenderFrame to set text tracks state and style settings.
 // Sent for top-level frames.
 IPC_MESSAGE_ROUTED1(FrameMsg_SetTextTrackSettings,

content/public/test/fake_remote_frame.cc

@@ -32,9 +32,6 @@ void FakeRemoteFrame::EnforceInsecureNavigationsSet(
 void FakeRemoteFrame::SetFrameOwnerProperties(
     blink::mojom::FrameOwnerPropertiesPtr properties) {}
 
-void FakeRemoteFrame::EnforceInsecureRequestPolicy(
-    blink::mojom::InsecureRequestPolicy policy) {}
-
 void FakeRemoteFrame::SetReplicatedOrigin(
     const url::Origin& origin,
     bool is_potentially_trustworthy_unique_origin) {}

content/public/test/fake_remote_frame.h

@@ -13,7 +13,6 @@
 #include "third_party/blink/public/mojom/frame/intrinsic_sizing_info.mojom.h"
 #include "third_party/blink/public/mojom/frame/user_activation_update_types.mojom.h"
 #include "third_party/blink/public/mojom/scroll/scroll_into_view_params.mojom.h"
-#include "third_party/blink/public/mojom/security_context/insecure_request_policy.mojom.h"
 #include "ui/events/types/scroll_types.h"
 
 namespace base {
@@ -46,8 +45,6 @@ class FakeRemoteFrame : public blink::mojom::RemoteFrame {
   void EnforceInsecureNavigationsSet(const std::vector<uint32_t>& set) override;
   void SetFrameOwnerProperties(
       blink::mojom::FrameOwnerPropertiesPtr properties) override;
-  void EnforceInsecureRequestPolicy(
-      blink::mojom::InsecureRequestPolicy policy) override;
   void SetReplicatedOrigin(
       const url::Origin& origin,
       bool is_potentially_trustworthy_unique_origin) override;

content/renderer/render_frame_proxy.cc

@@ -374,6 +374,8 @@ bool RenderFrameProxy::OnMessageReceived(const IPC::Message& msg) {
   IPC_BEGIN_MESSAGE_MAP(RenderFrameProxy, msg)
     IPC_MESSAGE_HANDLER(FrameMsg_UpdateOpener, OnUpdateOpener)
     IPC_MESSAGE_HANDLER(FrameMsg_DidUpdateName, OnDidUpdateName)
+    IPC_MESSAGE_HANDLER(FrameMsg_EnforceInsecureRequestPolicy,
+                        OnEnforceInsecureRequestPolicy)
     IPC_MESSAGE_HANDLER(FrameMsg_TransferUserActivationFrom,
                         OnTransferUserActivationFrom)
     IPC_MESSAGE_HANDLER(UnfreezableFrameMsg_DeleteProxy, OnDeleteProxy)
@@ -426,6 +428,11 @@ void RenderFrameProxy::OnDidUpdateName(const std::string& name,
   unique_name_ = unique_name;
 }
 
+void RenderFrameProxy::OnEnforceInsecureRequestPolicy(
+    blink::mojom::InsecureRequestPolicy policy) {
+  web_frame_->SetReplicatedInsecureRequestPolicy(policy);
+}
+
 void RenderFrameProxy::OnTransferUserActivationFrom(int32_t source_routing_id) {
   RenderFrameProxy* source_proxy =
       RenderFrameProxy::FromRoutingID(source_routing_id);

third_party/blink/public/mojom/frame/frame.mojom

@@ -500,11 +500,6 @@ interface RemoteFrame {
   // navigation.  This matches the in-process frame behavior.
   SetFrameOwnerProperties(FrameOwnerProperties properties);
 
-  // Updates the remote frame's replicated enforcement of insecure request
-  // policy. Used when the frame's policy is changed in another renderer
-  // process. Argument |policy| is a bitfield for InsecureRequestPolicy.
-  EnforceInsecureRequestPolicy(blink.mojom.InsecureRequestPolicy policy);
-
   // Update the replicated origin. Used when the frame is navigated to a
   // new origin.
   SetReplicatedOrigin(url.mojom.Origin origin,

third_party/blink/renderer/core/frame/remote_frame.cc

@@ -381,11 +381,6 @@ void RemoteFrame::SetFrameOwnerProperties(
   Frame::ApplyFrameOwnerProperties(std::move(properties));
 }
 
-void RemoteFrame::EnforceInsecureRequestPolicy(
-    mojom::blink::InsecureRequestPolicy policy) {
-  SetInsecureRequestPolicy(policy);
-}
-
 void RemoteFrame::SetReplicatedOrigin(
     const scoped_refptr<const SecurityOrigin>& origin,
     bool is_potentially_trustworthy_unique_origin) {

third_party/blink/renderer/core/frame/remote_frame.h

@@ -103,8 +103,6 @@ class CORE_EXPORT RemoteFrame final : public Frame,
   void EnforceInsecureNavigationsSet(const WTF::Vector<uint32_t>& set) override;
   void SetFrameOwnerProperties(
       mojom::blink::FrameOwnerPropertiesPtr properties) override;
-  void EnforceInsecureRequestPolicy(
-      mojom::blink::InsecureRequestPolicy policy) override;
   void SetReplicatedOrigin(
       const scoped_refptr<const SecurityOrigin>& origin,
       bool is_potentially_trustworthy_unique_origin) override;