Skip to content

Commit

Permalink
Move OpenURL from content.mojom.RenderFrameProxyHost to blink
Browse files Browse the repository at this point in the history 
As one of Onionsoup'ing, this CL moves OpenURL method from
content/mojom.RenderFrameProxyHost to blink.mojom.RemoteFrameHost.

MaybeSetDownloadFramePolicy is also moved to
blink::NavigationDownloadPolicy, then the existing callers
begin to call the API.

Lastly, this CL removes //content/common/frame_proxy.mojom
file because the interfaces don't have any method.

Bug: 1189209
Change-Id: I8ad01cbc7a29ea4b03c88284a6edbfa568c2263d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2780248
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Gyuyoung Kim <gyuyoung@igalia.com>
Cr-Commit-Position: refs/heads/master@{#866831}
  • Loading branch information
@Gyuyoung
Gyuyoung authored and Chromium LUCI CQ committed Mar 25, 2021
1 parent eea415a commit eff73ad
Show file tree
Hide file tree
Showing 22 changed files with 217 additions and 370 deletions.
19 changes: 1 addition & 18 deletions content/browser/renderer_host/render_frame_proxy_host.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -337,12 +337,7 @@ void RenderFrameProxyHost::OnAssociatedInterfaceRequest(
// receivers by resetting them before binding.
// TODO(dcheng): Maybe there should be an equivalent to RenderFrameHostImpl's
// InvalidateMojoConnection()?
if (interface_name == mojom::RenderFrameProxyHost::Name_) {
frame_proxy_host_associated_receiver_.reset();
frame_proxy_host_associated_receiver_.Bind(
mojo::PendingAssociatedReceiver<mojom::RenderFrameProxyHost>(
std::move(handle)));
} else if (interface_name == blink::mojom::RemoteFrameHost::Name_) {
if (interface_name == blink::mojom::RemoteFrameHost::Name_) {
remote_frame_host_receiver_.reset();
remote_frame_host_receiver_.Bind(
mojo::PendingAssociatedReceiver<blink::mojom::RemoteFrameHost>(
Expand Down Expand Up @@ -378,11 +373,6 @@ RenderFrameProxyHost::GetRemoteAssociatedInterfaces() {
}

void RenderFrameProxyHost::SetRenderFrameProxyCreated(bool created) {
if (!created) {
// If the renderer process has gone away, created can be false. In that
// case, invalidate the mojo connection.
frame_proxy_host_associated_receiver_.reset();
}
render_frame_proxy_created_ = created;
}

Expand All @@ -400,13 +390,6 @@ RenderFrameProxyHost::GetAssociatedRemoteMainFrame() {
return remote_main_frame_;
}

const mojo::AssociatedRemote<content::mojom::RenderFrameProxy>&
RenderFrameProxyHost::GetAssociatedRenderFrameProxy() {
if (!render_frame_proxy_)
GetRemoteAssociatedInterfaces()->GetInterface(&render_frame_proxy_);
return render_frame_proxy_;
}

void RenderFrameProxyHost::SetInheritedEffectiveTouchAction(
cc::TouchAction touch_action) {
cross_process_frame_connector_->OnSetInheritedEffectiveTouchAction(
Expand Down
26 changes: 2 additions & 24 deletions content/browser/renderer_host/render_frame_proxy_host.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
#include "base/macros.h"
#include "content/browser/site_instance_impl.h"
#include "content/common/frame.mojom.h"
#include "content/common/frame_proxy.mojom.h"
#include "ipc/ipc_listener.h"
#include "ipc/ipc_sender.h"
#include "mojo/public/cpp/bindings/associated_receiver.h"
Expand Down Expand Up @@ -69,7 +68,6 @@ class RenderWidgetHostView;
class CONTENT_EXPORT RenderFrameProxyHost
: public IPC::Listener,
public IPC::Sender,
public mojom::RenderFrameProxyHost,
public blink::mojom::RemoteFrameHost,
public blink::mojom::RemoteMainFrameHost {
public:
Expand Down Expand Up @@ -202,6 +200,7 @@ class CONTENT_EXPORT RenderFrameProxyHost
override;
void SynchronizeVisualProperties(
const blink::FrameVisualProperties& frame_visual_properties) override;
void OpenURL(blink::mojom::OpenURLParamsPtr params) override;

// blink::mojom::RemoteMainFrameHost overrides:
void FocusPage() override;
Expand All @@ -212,13 +211,6 @@ class CONTENT_EXPORT RenderFrameProxyHost
override;
void RouteCloseEvent() override;

// mojom::RenderFrameProxyHost:
void OpenURL(blink::mojom::OpenURLParamsPtr params) override;

// Returns associated remote for the content::mojom::RenderFrameProxy Mojo
// interface.
const mojo::AssociatedRemote<mojom::RenderFrameProxy>&
GetAssociatedRenderFrameProxy();
// Requests a viz::LocalSurfaceId to enable auto-resize mode from the parent
// renderer.
void EnableAutoResize(const gfx::Size& min_size, const gfx::Size& max_size);
Expand All @@ -235,8 +227,7 @@ class CONTENT_EXPORT RenderFrameProxyHost

private:
// These interceptor need access to frame_host_receiver_for_testing().
friend class RouteMessageEventInterceptor;
friend class OpenURLInterceptor;
friend class RemoteFrameHostInterceptor;
friend class UpdateViewportIntersectionMessageFilter;
friend class SynchronizeVisualPropertiesInterceptor;

Expand All @@ -257,12 +248,6 @@ class CONTENT_EXPORT RenderFrameProxyHost
return remote_frame_host_receiver_;
}

// Needed for tests to be able to swap the implementation and intercept calls.
mojo::AssociatedReceiver<mojom::RenderFrameProxyHost>&
frame_proxy_host_receiver_for_testing() {
return frame_proxy_host_associated_receiver_;
}

// This RenderFrameProxyHost's routing id.
int routing_id_;

Expand Down Expand Up @@ -299,20 +284,13 @@ class CONTENT_EXPORT RenderFrameProxyHost
std::unique_ptr<blink::AssociatedInterfaceProvider>
remote_associated_interfaces_;

// Mojo receiver to this RenderFrameProxyHost.
mojo::AssociatedReceiver<mojom::RenderFrameProxyHost>
frame_proxy_host_associated_receiver_{this};

// Holder of Mojo connection with the Frame service in Blink.
mojo::AssociatedRemote<blink::mojom::RemoteFrame> remote_frame_;

// Holder of Mojo connection with the RemoteMainFrame in Blink. This remote
// will be valid when the frame is the active main frame.
mojo::AssociatedRemote<blink::mojom::RemoteMainFrame> remote_main_frame_;

// Holder of Mojo connection with the content::mojom::RenderFrameProxy.
mojo::AssociatedRemote<mojom::RenderFrameProxy> render_frame_proxy_;

mojo::AssociatedReceiver<blink::mojom::RemoteFrameHost>
remote_frame_host_receiver_{this};

Expand Down
55 changes: 15 additions & 40 deletions content/browser/security_exploit_browsertest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
#include "content/browser/web_contents/web_contents_impl.h"
#include "content/common/frame.mojom.h"
#include "content/common/frame_messages.h"
#include "content/common/frame_proxy.mojom-test-utils.h"
#include "content/common/render_message_filter.mojom.h"
#include "content/public/browser/blob_handle.h"
#include "content/public/browser/browser_context.h"
Expand Down Expand Up @@ -801,10 +800,10 @@ IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest,
child->current_frame_host()->GetLastCommittedURL());
}

class RouteMessageEventInterceptor
class RemoteFrameHostInterceptor
: public blink::mojom::RemoteFrameHostInterceptorForTesting {
public:
explicit RouteMessageEventInterceptor(
explicit RemoteFrameHostInterceptor(
RenderFrameProxyHost* render_frame_proxy_host,
const std::u16string& evil_origin)
: render_frame_proxy_host_(render_frame_proxy_host),
Expand All @@ -829,9 +828,19 @@ class RouteMessageEventInterceptor
std::move(target_origin), std::move(message));
}

void OpenURL(blink::mojom::OpenURLParamsPtr params) override {
intercepted_params_ = std::move(params);
}

blink::mojom::OpenURLParamsPtr GetInterceptedParams() {
return std::move(intercepted_params_);
}

private:
RenderFrameProxyHost* render_frame_proxy_host_;

std::u16string evil_origin_;
blink::mojom::OpenURLParamsPtr intercepted_params_;
};

// Test verifying that a compromised renderer can't lie about the source_origin
Expand Down Expand Up @@ -872,8 +881,8 @@ IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest, PostMessageSourceOrigin) {
web_contents->GetMainFrame()->GetLastCommittedOrigin();
std::u16string evil_source_origin =
base::UTF8ToUTF16(invalid_origin.Serialize());
RouteMessageEventInterceptor mojo_interceptor(main_frame_proxy_host,
evil_source_origin);
RemoteFrameHostInterceptor mojo_interceptor(main_frame_proxy_host,
evil_source_origin);

// Post a message from the subframe to the cross-site parent and intercept the
// associated IPC message, changing it to simulate a compromised subframe
Expand All @@ -885,40 +894,6 @@ IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest, PostMessageSourceOrigin) {
kill_waiter.Wait());
}

// Intercepts calls to RenderFramHost's OpenURL mojo method, and
// store the passed parameter.
class OpenURLInterceptor
: public mojom::RenderFrameProxyHostInterceptorForTesting {
public:
explicit OpenURLInterceptor(
content::RenderFrameProxyHost* render_frame_proxy_host)
: render_frame_proxy_host_(render_frame_proxy_host),
intercepted_params_(blink::mojom::OpenURLParams::New()) {
render_frame_proxy_host_->frame_proxy_host_receiver_for_testing()
.SwapImplForTesting(this);
}

~OpenURLInterceptor() override = default;

mojom::RenderFrameProxyHost* GetForwardingInterface() override {
return render_frame_proxy_host_;
}

void OpenURL(blink::mojom::OpenURLParamsPtr params) override {
intercepted_params_ = std::move(params);
}

blink::mojom::OpenURLParamsPtr GetInterceptedParams() {
return std::move(intercepted_params_);
}

private:
content::RenderFrameProxyHost* render_frame_proxy_host_;
blink::mojom::OpenURLParamsPtr intercepted_params_;

DISALLOW_COPY_AND_ASSIGN(OpenURLInterceptor);
};

IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest,
InvalidRemoteNavigationInitiator) {
// Explicitly isolating a.com helps ensure that this test is applicable on
Expand Down Expand Up @@ -950,7 +925,7 @@ IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest,
RenderFrameProxyHost* proxy =
child_node->render_manager()->GetRenderFrameProxyHost(a_com_instance);

OpenURLInterceptor interceptor(proxy);
RemoteFrameHostInterceptor interceptor(proxy, std::u16string());

// Have the main frame request navigation in the "remote" subframe. This will
// result in OpenURL Mojo message being sent to the RenderFrameProxyHost.
Expand Down
1 change: 0 additions & 1 deletion content/browser/site_per_process_browsertest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@
#include "content/common/content_navigation_policy.h"
#include "content/common/frame.mojom-test-utils.h"
#include "content/common/frame_messages.h"
#include "content/common/frame_proxy.mojom-test-utils.h"
#include "content/common/input/actions_parser.h"
#include "content/common/input/synthetic_pinch_gesture_params.h"
#include "content/common/renderer.mojom.h"
Expand Down
1 change: 0 additions & 1 deletion content/common/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,6 @@ mojom("mojo_bindings") {
"field_trial_recorder.mojom",
"frame.mojom",
"frame_messages.mojom",
"frame_proxy.mojom",
"histogram_fetcher.mojom",
"input/input_injector.mojom",
"media/media_log_records.mojom",
Expand Down
26 changes: 0 additions & 26 deletions content/common/frame_proxy.mojom
View file

This file was deleted.

67 changes: 10 additions & 57 deletions content/renderer/render_frame_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -539,10 +539,11 @@ mojom::CommonNavigationParamsPtr MakeCommonNavigationParams(
info->initiator_origin_trial_features.end());

blink::NavigationDownloadPolicy download_policy;
RenderFrameImpl::MaybeSetDownloadFramePolicy(
info->is_opener_navigation, info->url_request, current_origin,
download_policy.ApplyDownloadFramePolicy(
info->is_opener_navigation, info->url_request.HasUserGesture(),
info->url_request.RequestorOrigin().CanAccess(current_origin),
has_download_sandbox_flag, info->blocking_downloads_in_sandbox_enabled,
from_ad, &download_policy);
from_ad);

return mojom::CommonNavigationParams::New(
info->url_request.Url(), info->url_request.RequestorOrigin(),
Expand Down Expand Up @@ -1922,55 +1923,6 @@ blink::WebFrame* RenderFrameImpl::ResolveWebFrame(int frame_routing_id) {
return nullptr;
}

// static
void RenderFrameImpl::MaybeSetDownloadFramePolicy(
bool is_opener_navigation,
const blink::WebURLRequest& request,
const blink::WebSecurityOrigin& current_origin,
bool has_download_sandbox_flag,
bool blocking_downloads_in_sandbox_enabled,
bool from_ad,
blink::NavigationDownloadPolicy* download_policy) {
bool has_gesture = request.HasUserGesture();
if (!has_gesture) {
download_policy->SetAllowed(blink::NavigationDownloadType::kNoGesture);
}

// Disallow downloads on an opener if the requestor is cross origin.
// See crbug.com/632514.
if (is_opener_navigation &&
!request.RequestorOrigin().CanAccess(current_origin)) {
download_policy->SetDisallowed(
blink::NavigationDownloadType::kOpenerCrossOrigin);
}

if (has_download_sandbox_flag) {
if (blocking_downloads_in_sandbox_enabled) {
download_policy->SetDisallowed(blink::NavigationDownloadType::kSandbox);
} else {
download_policy->SetAllowed(blink::NavigationDownloadType::kSandbox);
}
}

if (from_ad) {
download_policy->SetAllowed(blink::NavigationDownloadType::kAdFrame);
if (!has_gesture) {
if (base::FeatureList::IsEnabled(
blink::features::
kBlockingDownloadsInAdFrameWithoutUserActivation)) {
download_policy->SetDisallowed(
blink::NavigationDownloadType::kAdFrameNoGesture);
} else {
download_policy->SetAllowed(
blink::NavigationDownloadType::kAdFrameNoGesture);
}
}
}

download_policy->blocking_downloads_in_sandbox_enabled =
blocking_downloads_in_sandbox_enabled;
}

blink::WebURL RenderFrameImpl::OverrideFlashEmbedWithHTML(
const blink::WebURL& url) {
return GetContentClient()->renderer()->OverrideFlashEmbedWithHTML(url);
Expand Down Expand Up @@ -5604,11 +5556,12 @@ void RenderFrameImpl::OpenURL(std::unique_ptr<blink::WebNavigationInfo> info) {
current_frame_has_download_sandbox_flag;
bool from_ad = info->initiator_frame_is_ad || frame_->IsAdSubframe();

MaybeSetDownloadFramePolicy(info->is_opener_navigation, info->url_request,
frame_->GetSecurityOrigin(),
has_download_sandbox_flag,
info->blocking_downloads_in_sandbox_enabled,
from_ad, &params->download_policy);
params->download_policy.ApplyDownloadFramePolicy(
info->is_opener_navigation, info->url_request.HasUserGesture(),
info->url_request.RequestorOrigin().CanAccess(
frame_->GetSecurityOrigin()),
has_download_sandbox_flag, info->blocking_downloads_in_sandbox_enabled,
from_ad);
GetFrameHost()->OpenURL(std::move(params));
}

Expand Down
12 changes: 0 additions & 12 deletions content/renderer/render_frame_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,6 @@ class WebElement;
class WebFrameRequestBlocker;
class WebLocalFrame;
class WebMediaStreamDeviceObserver;
class WebSecurityOrigin;
class WebString;
class WebURL;
struct FramePolicy;
Expand Down Expand Up @@ -260,17 +259,6 @@ class CONTENT_EXPORT RenderFrameImpl
// ID.
static blink::WebFrame* ResolveWebFrame(int opener_frame_routing_id);

// Possibly set the kOpenerCrossOrigin and kSandboxNoGesture policy in
// |download_policy|.
static void MaybeSetDownloadFramePolicy(
bool is_opener_navigation,
const blink::WebURLRequest& request,
const blink::WebSecurityOrigin& current_origin,
bool has_download_sandbox_flag,
bool blocking_downloads_in_sandbox_enabled,
bool from_ad,
blink::NavigationDownloadPolicy* download_policy);

// Overwrites the given URL to use an HTML5 embed if possible.
blink::WebURL OverrideFlashEmbedWithHTML(const blink::WebURL& url) override;

Expand Down
Loading

0 comments on commit eff73ad

Please sign in to comment.