Commit df4a1a70 authored by Mustaq Ahmed's avatar Mustaq Ahmed Committed by Commit Bot

Propagate user activation notification from inner to outer WebContents.

User activation state sync across frames didn't previously consider
outer-vs-inner WebContents boundary.  As a result, a user interaction
in an inner WebContents didn't activate any frames in the outer
WebContents.  This CL minimally completes the notification state
propagation from inner to outer WebContents to fix the bug.

Bug: 1013447
Change-Id: If06fa2c1c5c431b45219eeb4b75ec8cf93164042
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1907209Reviewed-by: default avatarAlex Moshchuk <alexmos@chromium.org>
Reviewed-by: default avatarJames MacLean <wjmaclean@chromium.org>
Commit-Queue: Mustaq Ahmed <mustaq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#719142}
parent 9b4a9378
......@@ -296,6 +296,14 @@ var tests = [
chrome.mimeHandlerPrivate.setShowBeforeUnloadDialog(
true, chrome.test.succeed);
},
// TODO(mustaq): Every test above have a unique csv, which seems redundant.
// This particular one is used in two browser tests.
function testBeforeUnloadWithUserActivation() {
checkStreamDetails('testBeforeUnloadWithUserActivation.csv', false);
chrome.mimeHandlerPrivate.setShowBeforeUnloadDialog(
true, chrome.test.succeed);
},
];
var testsByName = {};
......
......@@ -1041,6 +1041,24 @@ void RenderFrameHostManager::UpdateUserActivationState(
pair.second->Send(new FrameMsg_UpdateUserActivationState(
pair.second->GetRoutingID(), update_type));
}
// If any frame in an inner delegate is activated, then the FrameTreeNode that
// embeds the inner delegate in the outer delegate should be activated as well
// (crbug.com/1013447).
//
// TODO(mustaq): We should add activation consumption propagation from inner
// to outer delegates, and also all state propagation from outer to inner
// delegates. crbug.com/1026617.
RenderFrameProxyHost* outer_delegate_proxy = frame_tree_node_->frame_tree()
->root()
->render_manager()
->GetProxyToOuterDelegate();
if (outer_delegate_proxy &&
update_type == blink::UserActivationUpdateType::kNotifyActivation) {
outer_delegate_proxy->Send(new FrameMsg_UpdateUserActivationState(
outer_delegate_proxy->GetRoutingID(), update_type));
GetOuterDelegateNode()->UpdateUserActivationState(update_type);
}
}
void RenderFrameHostManager::TransferUserActivationFrom(
......
......@@ -755,11 +755,11 @@ bool WaitForLoadStop(WebContents* web_contents) {
return IsLastCommittedEntryOfPageType(web_contents, PAGE_TYPE_NORMAL);
}
void PrepContentsForBeforeUnloadTest(WebContents* web_contents) {
void PrepContentsForBeforeUnloadTest(WebContents* web_contents,
bool trigger_user_activation) {
for (auto* frame : web_contents->GetAllFrames()) {
// JavaScript onbeforeunload dialogs are ignored unless the frame received a
// user gesture. Make sure the frames have user gestures.
frame->ExecuteJavaScriptWithUserGestureForTests(base::string16());
if (trigger_user_activation)
frame->ExecuteJavaScriptWithUserGestureForTests(base::string16());
// Disable the hang monitor, otherwise there will be a race between the
// beforeunload dialog and the beforeunload hang timer.
......@@ -3266,6 +3266,26 @@ void ContextMenuFilter::OnContextMenu(
std::move(quit_closure_).Run();
}
bool UpdateUserActivationStateMsgWaiter::OnMessageReceived(
const IPC::Message& message) {
IPC_BEGIN_MESSAGE_MAP(UpdateUserActivationStateMsgWaiter, message)
IPC_MESSAGE_HANDLER(FrameHostMsg_UpdateUserActivationState,
OnUpdateUserActivationState)
IPC_END_MESSAGE_MAP()
return false;
}
void UpdateUserActivationStateMsgWaiter::Wait() {
if (!received_)
run_loop_.Run();
}
void UpdateUserActivationStateMsgWaiter::OnUpdateUserActivationState(
blink::UserActivationUpdateType) {
received_ = true;
run_loop_.Quit();
}
WebContents* GetEmbedderForGuest(content::WebContents* guest) {
CHECK(guest);
return static_cast<content::WebContentsImpl*>(guest)->GetOuterWebContents();
......
......@@ -41,6 +41,7 @@
#include "services/network/public/mojom/network_service.mojom.h"
#include "storage/common/file_system/file_system_types.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/public/common/frame/user_activation_update_type.h"
#include "third_party/blink/public/platform/web_input_event.h"
#include "third_party/blink/public/platform/web_mouse_event.h"
#include "third_party/blink/public/platform/web_mouse_wheel_event.h"
......@@ -157,8 +158,12 @@ void WaitForLoadStopWithoutSuccessCheck(WebContents* web_contents);
bool WaitForLoadStop(WebContents* web_contents);
// If a test uses a beforeunload dialog, it must be prepared to avoid flakes.
// This function collects everything that needs to be done.
void PrepContentsForBeforeUnloadTest(WebContents* web_contents);
// This function collects everything that needs to be done, except for user
// activation which is triggered only when |trigger_user_activation| is true.
// Note that beforeunload dialog attempts are ignored unless the frame has
// received a user activation.
void PrepContentsForBeforeUnloadTest(WebContents* web_contents,
bool trigger_user_activation = true);
#if defined(USE_AURA) || defined(OS_ANDROID)
// If WebContent's view is currently being resized, this will wait for the ack
......@@ -1651,6 +1656,24 @@ class ContextMenuFilter : public content::BrowserMessageFilter {
DISALLOW_COPY_AND_ASSIGN(ContextMenuFilter);
};
// This class allows tests to wait until FrameHostMsg_UpdateUserActivationState
// IPC reaches the browser process from a renderer.
class UpdateUserActivationStateMsgWaiter : public BrowserMessageFilter {
public:
UpdateUserActivationStateMsgWaiter() : BrowserMessageFilter(FrameMsgStart) {}
bool OnMessageReceived(const IPC::Message& message) override;
void Wait();
private:
~UpdateUserActivationStateMsgWaiter() override = default;
void OnUpdateUserActivationState(blink::UserActivationUpdateType);
bool received_ = false;
base::RunLoop run_loop_;
};
WebContents* GetEmbedderForGuest(content::WebContents* guest);
// Load the given |url| with |network_context| and return the |net::Error| code.
......
......@@ -539,7 +539,7 @@ IN_PROC_BROWSER_TEST_P(MimeHandlerViewCrossProcessTest, TargetBlankAnchor) {
ASSERT_EQ(2, browser()->tab_strip_model()->count());
content::WaitForLoadStop(browser()->tab_strip_model()->GetWebContentsAt(1));
EXPECT_EQ(
GURL("about:blank"),
GURL(url::kAboutBlankURL),
browser()->tab_strip_model()->GetWebContentsAt(1)->GetLastCommittedURL());
}
......@@ -555,7 +555,7 @@ IN_PROC_BROWSER_TEST_P(MimeHandlerViewCrossProcessTest, BeforeUnload_NoDialog) {
// Try to navigate away from the page. If the beforeunload listener is
// triggered and a dialog is shown, this navigation will never complete,
// causing the test to timeout and fail.
ui_test_utils::NavigateToURL(browser(), GURL("about:blank"));
ui_test_utils::NavigateToURL(browser(), GURL(url::kAboutBlankURL));
}
IN_PROC_BROWSER_TEST_P(MimeHandlerViewCrossProcessTest,
......@@ -568,7 +568,60 @@ IN_PROC_BROWSER_TEST_P(MimeHandlerViewCrossProcessTest,
// toggle IPC has had time to reach the browser.
ExecuteScriptAndGetValue(web_contents->GetMainFrame(), "");
web_contents->GetController().LoadURL(GURL("about:blank"), {},
web_contents->GetController().LoadURL(GURL(url::kAboutBlankURL), {},
ui::PAGE_TRANSITION_TYPED, "");
app_modal::JavaScriptAppModalDialog* before_unload_dialog =
ui_test_utils::WaitForAppModalDialog();
EXPECT_TRUE(before_unload_dialog->is_before_unload_dialog());
EXPECT_FALSE(before_unload_dialog->is_reload());
before_unload_dialog->OnAccept(base::string16(), false);
}
IN_PROC_BROWSER_TEST_P(MimeHandlerViewCrossProcessTest,
BeforeUnloadEnabled_WithoutUserActivation) {
ASSERT_NO_FATAL_FAILURE(RunTest("testBeforeUnloadWithUserActivation.csv"));
auto* web_contents = GetEmbedderWebContents();
// Prepare frames but don't trigger user activation.
content::PrepContentsForBeforeUnloadTest(web_contents, false);
// Even though this test's JS setup enables BeforeUnload dialogs, the dialog
// is still suppressed here because of lack of user activation. As a result,
// the following navigation away from the page works fine. If a beforeunload
// dialog were shown, this navigation would fail, causing the test to timeout.
ui_test_utils::NavigateToURL(browser(), GURL(url::kAboutBlankURL));
}
IN_PROC_BROWSER_TEST_P(MimeHandlerViewCrossProcessTest,
BeforeUnloadEnabled_WithUserActivation) {
// Only run this test in cross-process mode.
if (!GetParam())
return;
ASSERT_NO_FATAL_FAILURE(RunTest("testBeforeUnloadWithUserActivation.csv"));
auto* web_contents = GetEmbedderWebContents();
// Prepare frames but don't trigger user activation across all frames.
content::PrepContentsForBeforeUnloadTest(web_contents, false);
// Make sure we have a guestviewmanager.
auto* guest_contents = GetGuestViewManager()->WaitForSingleGuestCreated();
// Add a filter for FrameHostMsg_UpdateUserActivationState IPC.
auto filter =
base::MakeRefCounted<content::UpdateUserActivationStateMsgWaiter>();
guest_contents->GetMainFrame()->GetProcess()->AddFilter(filter.get());
// Activate |guest_contents| through a click, then wait until the activation
// IPC reaches the browser process.
SimulateMouseClick(guest_contents, 0, blink::WebMouseEvent::Button::kLeft);
filter->Wait();
// Wait for a round trip to the outer renderer to ensure any beforeunload
// toggle IPC has had time to reach the browser.
ExecuteScriptAndGetValue(web_contents->GetMainFrame(), "");
// Try to navigate away, this should invoke a beforeunload dialog.
web_contents->GetController().LoadURL(GURL(url::kAboutBlankURL), {},
ui::PAGE_TRANSITION_TYPED, "");
app_modal::JavaScriptAppModalDialog* before_unload_dialog =
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment