Skip to content

Commit

Permalink
Move InputMsg_SetFocus to mojo
Browse files Browse the repository at this point in the history
This CL changes InputMsg_SetFocus so that it is implemented in the
RemoteFrame interface since it is only got through RenderFrameProxy.

It also introduces FakeRemoteFrame to use RemoteFrame on
render_frame_host_manager_unittest.cc.

Bug: 1039256
Change-Id: If7d1d2971fabf3c64777281b1bf5eedfa9fcf5c7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1989714
Commit-Queue: Julie Kim <jkim@igalia.com>
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#732808}
  • Loading branch information
jkim-julie authored and Commit Bot committed Jan 17, 2020
1 parent c68a814 commit 2002cb3
Show file tree
Hide file tree
Showing 19 changed files with 202 additions and 64 deletions.
2 changes: 1 addition & 1 deletion content/browser/frame_host/frame_tree.cc
Original file line number Diff line number Diff line change
Expand Up @@ -489,7 +489,7 @@ void FrameTree::SetPageFocus(SiteInstance* instance, bool is_focused) {
if (instance != root_manager->current_frame_host()->GetSiteInstance()) {
RenderFrameProxyHost* proxy =
root_manager->GetRenderFrameProxyHost(instance);
proxy->Send(new InputMsg_SetFocus(proxy->GetRoutingID(), is_focused));
proxy->GetAssociatedRemoteFrame()->SetPageFocus(is_focused);
}
}

Expand Down
1 change: 0 additions & 1 deletion content/browser/frame_host/render_frame_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1641,7 +1641,6 @@ PageVisibilityState RenderFrameHostImpl::GetVisibilityState() {
}

bool RenderFrameHostImpl::Send(IPC::Message* message) {
DCHECK(IPC_MESSAGE_ID_CLASS(message->type()) != InputMsgStart);
return GetProcess()->Send(message);
}

Expand Down
80 changes: 50 additions & 30 deletions content/browser/frame_host/render_frame_host_manager_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
#include "content/public/common/url_constants.h"
#include "content/public/common/url_utils.h"
#include "content/public/test/fake_local_frame.h"
#include "content/public/test/fake_remote_frame.h"
#include "content/public/test/mock_render_process_host.h"
#include "content/public/test/test_utils.h"
#include "content/test/mock_widget_input_handler.h"
Expand All @@ -73,20 +74,6 @@
namespace content {
namespace {

// Helper to check that the provided RenderProcessHost received exactly one
// page focus message with the provided focus and routing ID values.
void VerifyPageFocusMessage(MockRenderProcessHost* rph,
bool expected_focus,
int expected_routing_id) {
const IPC::Message* message =
rph->sink().GetUniqueMessageMatching(InputMsg_SetFocus::ID);
EXPECT_TRUE(message);
EXPECT_EQ(expected_routing_id, message->routing_id());
InputMsg_SetFocus::Param params;
EXPECT_TRUE(InputMsg_SetFocus::Read(message, &params));
EXPECT_EQ(expected_focus, std::get<0>(params));
}

// VerifyPageFocusMessage from the mojo input handler.
void VerifyPageFocusMessage(TestRenderWidgetHost* twh, bool expected_focus) {
MockWidgetInputHandler::MessageVector events =
Expand Down Expand Up @@ -2288,6 +2275,21 @@ TEST_F(RenderFrameHostManagerTest, TraverseComplexOpenerChain) {
nodes_with_back_links.end());
}

// Stub out remote frame mojo binding. Intercepts calls to SetPageFocus
// and marks the message as received.
class PageFocusRemoteFrame : public content::FakeRemoteFrame {
public:
explicit PageFocusRemoteFrame(RenderFrameProxyHost* render_frame_proxy_host) {
Init(render_frame_proxy_host->GetRemoteAssociatedInterfacesTesting());
}

void SetPageFocus(bool is_focused) override { set_page_focus_ = is_focused; }
bool IsPageFocused() { return set_page_focus_; }

private:
bool set_page_focus_ = false;
};

// Check that when a window is focused/blurred, the message that sets
// page-level focus updates is sent to each process involved in rendering the
// current page.
Expand Down Expand Up @@ -2343,8 +2345,18 @@ TEST_F(RenderFrameHostManagerTest, PageFocusPropagatesToSubframeProcesses) {
false /* is_renderer_init */, nullptr /* blob_url_loader_factory */);
TestRenderFrameHost* host1 =
static_cast<TestRenderFrameHost*>(NavigateToEntry(child1, &entryB));

// The main frame should have proxies for B.
RenderFrameProxyHost* proxyB =
root->render_manager()->GetRenderFrameProxyHost(host1->GetSiteInstance());
EXPECT_TRUE(proxyB);

// Create PageFocusRemoteFrame to intercept SetPageFocus to RemoteFrame.
PageFocusRemoteFrame remote_frame1(proxyB);

TestRenderFrameHost* host2 =
static_cast<TestRenderFrameHost*>(NavigateToEntry(child2, &entryB));

child1->DidNavigateFrame(host1, true /* was_caused_by_user_gesture */,
false /* is_same_document_navigation */,
blink::FramePolicy());
Expand All @@ -2360,6 +2372,15 @@ TEST_F(RenderFrameHostManagerTest, PageFocusPropagatesToSubframeProcesses) {
false /* is_renderer_init */, nullptr /* blob_url_loader_factory */);
TestRenderFrameHost* host3 =
static_cast<TestRenderFrameHost*>(NavigateToEntry(child3, &entryC));

// The main frame should have proxies for C.
RenderFrameProxyHost* proxyC =
root->render_manager()->GetRenderFrameProxyHost(host3->GetSiteInstance());
EXPECT_TRUE(proxyC);

// Create PageFocusRemoteFrame to intercept SetPageFocus to RemoteFrame.
PageFocusRemoteFrame remote_frame3(proxyC);

child3->DidNavigateFrame(host3, true /* was_caused_by_user_gesture */,
false /* is_same_document_navigation */,
blink::FramePolicy());
Expand All @@ -2371,13 +2392,6 @@ TEST_F(RenderFrameHostManagerTest, PageFocusPropagatesToSubframeProcesses) {
EXPECT_NE(host3->GetProcess(), main_test_rfh()->GetProcess());
EXPECT_NE(host3->GetProcess(), host1->GetProcess());

// The main frame should have proxies for B and C.
RenderFrameProxyHost* proxyB =
root->render_manager()->GetRenderFrameProxyHost(host1->GetSiteInstance());
EXPECT_TRUE(proxyB);
RenderFrameProxyHost* proxyC =
root->render_manager()->GetRenderFrameProxyHost(host3->GetSiteInstance());
EXPECT_TRUE(proxyC);
base::RunLoop().RunUntilIdle();

// Focus the main page, and verify that the focus message was sent to all
Expand All @@ -2390,8 +2404,8 @@ TEST_F(RenderFrameHostManagerTest, PageFocusPropagatesToSubframeProcesses) {
main_test_rfh()->GetRenderWidgetHost()->Focus();
base::RunLoop().RunUntilIdle();
VerifyPageFocusMessage(main_test_rfh()->GetRenderWidgetHost(), true);
VerifyPageFocusMessage(host1->GetProcess(), true, proxyB->GetRoutingID());
VerifyPageFocusMessage(host3->GetProcess(), true, proxyC->GetRoutingID());
EXPECT_TRUE(remote_frame1.IsPageFocused());
EXPECT_TRUE(remote_frame3.IsPageFocused());

// Similarly, simulate focus loss on main page, and verify that the focus
// message was sent to all processes.
Expand All @@ -2401,8 +2415,8 @@ TEST_F(RenderFrameHostManagerTest, PageFocusPropagatesToSubframeProcesses) {
main_test_rfh()->GetRenderWidgetHost()->Blur();
base::RunLoop().RunUntilIdle();
VerifyPageFocusMessage(main_test_rfh()->GetRenderWidgetHost(), false);
VerifyPageFocusMessage(host1->GetProcess(), false, proxyB->GetRoutingID());
VerifyPageFocusMessage(host3->GetProcess(), false, proxyC->GetRoutingID());
EXPECT_FALSE(remote_frame1.IsPageFocused());
EXPECT_FALSE(remote_frame3.IsPageFocused());
}

// Check that page-level focus state is preserved across subframe navigations.
Expand Down Expand Up @@ -2458,19 +2472,25 @@ TEST_F(RenderFrameHostManagerTest,
false /* is_renderer_init */, nullptr /* blob_url_loader_factory */);
TestRenderFrameHost* hostC =
static_cast<TestRenderFrameHost*>(NavigateToEntry(child, &entryC));

// The main frame should now have a proxy for C.
RenderFrameProxyHost* proxyC =
root->render_manager()->GetRenderFrameProxyHost(hostC->GetSiteInstance());
EXPECT_TRUE(proxyC);

// Create PageFocusRemoteFrame to intercept SetPageFocus to RemoteFrame.
PageFocusRemoteFrame remote_frame(proxyC);

child->DidNavigateFrame(hostC, true /* was_caused_by_user_gesture */,
false /* is_same_document_navigation */,
blink::FramePolicy());

// The main frame should now have a proxy for C.
RenderFrameProxyHost* proxy =
root->render_manager()->GetRenderFrameProxyHost(hostC->GetSiteInstance());
EXPECT_TRUE(proxy);
base::RunLoop().RunUntilIdle();

// Since the B->C navigation happened while the current page was focused,
// page focus should propagate to the new subframe process. Check that
// process C received the proper focus message.
VerifyPageFocusMessage(hostC->GetProcess(), true, proxy->GetRoutingID());
EXPECT_TRUE(remote_frame.IsPageFocused());
}

// Checks that a restore navigation to a WebUI works.
Expand Down
5 changes: 5 additions & 0 deletions content/browser/frame_host/render_frame_proxy_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -593,4 +593,9 @@ void RenderFrameProxyHost::OnPrintCrossProcessSubframe(const gfx::Rect& rect,
rfh->delegate()->PrintCrossProcessSubframe(rect, document_cookie, rfh);
}

blink::AssociatedInterfaceProvider*
RenderFrameProxyHost::GetRemoteAssociatedInterfacesTesting() {
return GetRemoteAssociatedInterfaces();
}

} // namespace content
3 changes: 3 additions & 0 deletions content/browser/frame_host/render_frame_proxy_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,9 @@ class RenderFrameProxyHost : public IPC::Listener,
void DidFocusFrame() override;
void CheckCompleted() override;

CONTENT_EXPORT blink::AssociatedInterfaceProvider*
GetRemoteAssociatedInterfacesTesting();

private:
// IPC Message handlers.
void OnDetach();
Expand Down
1 change: 0 additions & 1 deletion content/browser/renderer_host/render_widget_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,6 @@ bool RenderWidgetHostImpl::OnMessageReceived(const IPC::Message &msg) {
}

bool RenderWidgetHostImpl::Send(IPC::Message* msg) {
DCHECK(IPC_MESSAGE_ID_CLASS(msg->type()) != InputMsgStart);
return process_->Send(msg);
}

Expand Down
10 changes: 0 additions & 10 deletions content/common/input_messages.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,6 @@
#undef IPC_MESSAGE_EXPORT
#define IPC_MESSAGE_EXPORT CONTENT_EXPORT

#ifdef IPC_MESSAGE_START
#error IPC_MESSAGE_START
#endif

#define IPC_MESSAGE_START InputMsgStart

IPC_ENUM_TRAITS_MAX_VALUE(content::InputEventAckSource,
content::InputEventAckSource::MAX_FROM_RENDERER)
IPC_ENUM_TRAITS_MAX_VALUE(
Expand Down Expand Up @@ -154,8 +148,4 @@ IPC_STRUCT_TRAITS_BEGIN(content::SyntheticPointerActionListParams)
IPC_STRUCT_TRAITS_MEMBER(params)
IPC_STRUCT_TRAITS_END()

// TODO(dtapuska): Remove this as only OOPIF uses this
IPC_MESSAGE_ROUTED1(InputMsg_SetFocus,
bool /* enable */)

#endif // CONTENT_COMMON_INPUT_MESSAGES_H_
62 changes: 62 additions & 0 deletions content/public/test/fake_remote_frame.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// Copyright 2020 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.

#include "content/public/test/fake_remote_frame.h"

namespace content {

FakeRemoteFrame::FakeRemoteFrame() = default;

FakeRemoteFrame::~FakeRemoteFrame() = default;

void FakeRemoteFrame::Init(blink::AssociatedInterfaceProvider* provider) {
provider->OverrideBinderForTesting(
blink::mojom::RemoteFrame::Name_,
base::BindRepeating(&FakeRemoteFrame::BindFrameHostReceiver,
base::Unretained(this)));
}

void FakeRemoteFrame::WillEnterFullscreen() {}

void FakeRemoteFrame::AddReplicatedContentSecurityPolicies(
std::vector<network::mojom::ContentSecurityPolicyHeaderPtr> headers) {}

void FakeRemoteFrame::ResetReplicatedContentSecurityPolicy() {}

void FakeRemoteFrame::EnforceInsecureNavigationsSet(
const std::vector<uint32_t>& set) {}

void FakeRemoteFrame::SetReplicatedOrigin(
const url::Origin& origin,
bool is_potentially_trustworthy_unique_origin) {}

void FakeRemoteFrame::DispatchLoadEventForFrameOwner() {}

void FakeRemoteFrame::Collapse(bool collapsed) {}

void FakeRemoteFrame::Focus() {}

void FakeRemoteFrame::SetHadStickyUserActivationBeforeNavigation(bool value) {}

void FakeRemoteFrame::SetNeedsOcclusionTracking(bool needs_tracking) {}

void FakeRemoteFrame::BubbleLogicalScroll(
blink::mojom::ScrollDirection direction,
ui::input_types::ScrollGranularity granularity) {}

void FakeRemoteFrame::UpdateUserActivationState(
blink::mojom::UserActivationUpdateType) {}

void FakeRemoteFrame::SetEmbeddingToken(
const base::UnguessableToken& embedding_token) {}

void FakeRemoteFrame::SetPageFocus(bool is_focused) {}

void FakeRemoteFrame::FakeRemoteFrame::BindFrameHostReceiver(
mojo::ScopedInterfaceEndpointHandle handle) {
receiver_.Bind(mojo::PendingAssociatedReceiver<blink::mojom::RemoteFrame>(
std::move(handle)));
}

} // namespace content
68 changes: 68 additions & 0 deletions content/public/test/fake_remote_frame.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// Copyright 2020 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 CONTENT_PUBLIC_TEST_FAKE_REMOTE_FRAME_H_
#define CONTENT_PUBLIC_TEST_FAKE_REMOTE_FRAME_H_

#include "mojo/public/cpp/bindings/associated_receiver.h"
#include "services/network/public/mojom/content_security_policy.mojom.h"
#include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"
#include "third_party/blink/public/mojom/frame/frame.mojom.h"
#include "third_party/blink/public/mojom/frame/user_activation_update_types.mojom.h"
#include "ui/events/types/scroll_types.h"

namespace base {
class UnguessableToken;
}

namespace url {
class Origin;
} // namespace url

namespace content {

// This class implements a RemoteFrame that can be attached to the
// AssociatedInterfaceProvider so that it will be called when the browser
// normally sends a request to the renderer process. But for a unittest
// setup it can be intercepted by this class.
class FakeRemoteFrame : public blink::mojom::RemoteFrame {
public:
FakeRemoteFrame();
~FakeRemoteFrame() override;

void Init(blink::AssociatedInterfaceProvider* provider);

// blink::mojom::RemoteFrame overrides:
void WillEnterFullscreen() override;
void AddReplicatedContentSecurityPolicies(
std::vector<network::mojom::ContentSecurityPolicyHeaderPtr> headers)
override;
void ResetReplicatedContentSecurityPolicy() override;
void EnforceInsecureNavigationsSet(const std::vector<uint32_t>& set) override;
void SetReplicatedOrigin(
const url::Origin& origin,
bool is_potentially_trustworthy_unique_origin) override;
void DispatchLoadEventForFrameOwner() override;
void Collapse(bool collapsed) final;
void Focus() override;
void SetHadStickyUserActivationBeforeNavigation(bool value) override;
void SetNeedsOcclusionTracking(bool needs_tracking) override;
void BubbleLogicalScroll(
blink::mojom::ScrollDirection direction,
ui::input_types::ScrollGranularity granularity) override;
void UpdateUserActivationState(
blink::mojom::UserActivationUpdateType) override;
void SetEmbeddingToken(
const base::UnguessableToken& embedding_token) override;
void SetPageFocus(bool is_focused) override;

private:
void BindFrameHostReceiver(mojo::ScopedInterfaceEndpointHandle handle);

mojo::AssociatedReceiver<blink::mojom::RemoteFrame> receiver_{this};
};

} // namespace content

#endif // CONTENT_PUBLIC_TEST_FAKE_REMOTE_FRAME_H_
5 changes: 0 additions & 5 deletions content/renderer/render_frame_proxy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,6 @@ bool RenderFrameProxy::OnMessageReceived(const IPC::Message& msg) {
OnEnforceInsecureRequestPolicy)
IPC_MESSAGE_HANDLER(FrameMsg_SetFrameOwnerProperties,
OnSetFrameOwnerProperties)
IPC_MESSAGE_HANDLER(InputMsg_SetFocus, OnSetPageFocus)
IPC_MESSAGE_HANDLER(FrameMsg_DidUpdateVisualProperties,
OnDidUpdateVisualProperties)
IPC_MESSAGE_HANDLER(FrameMsg_EnableAutoResize, OnEnableAutoResize)
Expand Down Expand Up @@ -512,10 +511,6 @@ void RenderFrameProxy::OnSetFrameOwnerProperties(
ConvertFrameOwnerPropertiesToWebFrameOwnerProperties(properties));
}

void RenderFrameProxy::OnSetPageFocus(bool is_focused) {
render_view_->SetFocus(is_focused);
}

void RenderFrameProxy::OnTransferUserActivationFrom(int32_t source_routing_id) {
RenderFrameProxy* source_proxy =
RenderFrameProxy::FromRoutingID(source_routing_id);
Expand Down
1 change: 0 additions & 1 deletion content/renderer/render_frame_proxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,6 @@ class CONTENT_EXPORT RenderFrameProxy : public IPC::Listener,
void OnDidUpdateName(const std::string& name, const std::string& unique_name);
void OnEnforceInsecureRequestPolicy(blink::WebInsecureRequestPolicy policy);
void OnSetFrameOwnerProperties(const FrameOwnerProperties& properties);
void OnSetPageFocus(bool is_focused);
void OnTransferUserActivationFrom(int32_t source_routing_id);
void OnScrollRectToVisible(const gfx::Rect& rect_to_scroll,
const blink::WebScrollIntoViewParams& params);
Expand Down
6 changes: 0 additions & 6 deletions content/renderer/render_view_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1956,12 +1956,6 @@ void RenderViewImpl::OnSetInsidePortal(bool inside_portal) {
webview()->SetInsidePortal(inside_portal);
}

void RenderViewImpl::SetFocus(bool enable) {
// This is only called from RenderFrameProxy.
CHECK(!webview()->MainFrame()->IsWebLocalFrame());
webview()->SetFocus(enable);
}

void RenderViewImpl::PageScaleFactorChanged(float page_scale_factor) {
if (!webview())
return;
Expand Down
Loading

0 comments on commit 2002cb3

Please sign in to comment.