Commit 5c21f3ef authored by Alex Moshchuk's avatar Alex Moshchuk Committed by Commit Bot

Delay SwapOut ACK to fix postMessages sent from subframe unload handlers.

After r550769, cross-process postMessages are scheduled with PostTask,
so that the actual postMessage IPC goes out on next iteration of the
event loop.  This is problematic when postMessage is executed from an
unload handler that is triggered on a local-to-remote frame swap in
RenderFrameImpl::OnSwapOut: the SwapOut ACK ends up being sent before
any postMessage IPCs, and if this was the last active frame in the
process, the browser process, upon receiving the SwapOut ACK, might
destroy proxies that a postMessage was targeting.  To prevent this,
schedule the SwapOut ACK to ensure that it gets sent after any
postMessage IPCs.

Bug: 857274
Change-Id: I9e7339c0abc409fd201e7f73927e871c0f0d3b95
Reviewed-on: https://chromium-review.googlesource.com/1119391Reviewed-by: default avatarCharlie Reis <creis@chromium.org>
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571625}
parent dbef4748
...@@ -4767,4 +4767,51 @@ IN_PROC_BROWSER_TEST_F(RenderFrameHostManagerUnloadBrowserTest, ...@@ -4767,4 +4767,51 @@ IN_PROC_BROWSER_TEST_F(RenderFrameHostManagerUnloadBrowserTest,
process_exit_observer.Wait(); process_exit_observer.Wait();
} }
// Verify that when an OOPIF with an unload handler navigates cross-process,
// its unload handler is able to send a postMessage to the parent frame.
// See https://crbug.com/857274.
IN_PROC_BROWSER_TEST_F(RenderFrameHostManagerUnloadBrowserTest,
PostMessageToParentWhenSubframeNavigates) {
GURL main_url(embedded_test_server()->GetURL(
"a.com", "/cross_site_iframe_factory.html?a(b)"));
EXPECT_TRUE(NavigateToURL(shell(), main_url));
FrameTreeNode* root = static_cast<WebContentsImpl*>(shell()->web_contents())
->GetFrameTree()
->root();
FrameTreeNode* child = root->child_at(0);
// Add an onmessage listener in the main frame.
EXPECT_TRUE(ExecuteScript(root, R"(
window.addEventListener('message', function(e) {
domAutomationController.send(e.data);
});)"));
// Add an unload handler in the child frame to send a postMessage to the
// parent frame.
AddUnloadHandler(child->current_frame_host(),
"parent.postMessage('foo', '*')");
// Navigate the subframe cross-site to c.com and wait for the message.
GURL c_url(embedded_test_server()->GetURL("c.com", "/title1.html"));
std::string message;
EXPECT_TRUE(ExecuteScriptAndExtractString(
root,
base::StringPrintf("document.querySelector('iframe').src = '%s';",
c_url.spec().c_str()),
&message));
EXPECT_EQ("foo", message);
// Now repeat the test with a remote-to-local navigation that brings the
// subframe back to a.com.
AddUnloadHandler(child->current_frame_host(),
"parent.postMessage('bar', '*')");
GURL a_url(embedded_test_server()->GetURL("a.com", "/title2.html"));
EXPECT_TRUE(ExecuteScriptAndExtractString(
root,
base::StringPrintf("document.querySelector('iframe').src = '%s';",
a_url.spec().c_str()),
&message));
EXPECT_EQ("bar", message);
}
} // namespace content } // namespace content
...@@ -1976,6 +1976,12 @@ void RenderFrameImpl::OnSwapOut( ...@@ -1976,6 +1976,12 @@ void RenderFrameImpl::OnSwapOut(
bool is_main_frame = is_main_frame_; bool is_main_frame = is_main_frame_;
int routing_id = GetRoutingID(); int routing_id = GetRoutingID();
// Before |this| is destroyed, grab the TaskRunner to be used for sending the
// SwapOut ACK. This will be used to schedule SwapOut ACK to be sent after
// any postMessage IPCs scheduled from the unload event above.
scoped_refptr<base::SingleThreadTaskRunner> task_runner =
GetTaskRunner(blink::TaskType::kPostedMessage);
// Now that all of the cleanup is complete and the browser side is notified, // Now that all of the cleanup is complete and the browser side is notified,
// start using the RenderFrameProxy. // start using the RenderFrameProxy.
// //
...@@ -2017,8 +2023,15 @@ void RenderFrameImpl::OnSwapOut( ...@@ -2017,8 +2023,15 @@ void RenderFrameImpl::OnSwapOut(
render_view->WasSwappedOut(); render_view->WasSwappedOut();
// Notify the browser that this frame was swapped. Use the RenderThread // Notify the browser that this frame was swapped. Use the RenderThread
// directly because |this| is deleted. // directly because |this| is deleted. Post a task to send the ACK, so that
RenderThread::Get()->Send(new FrameHostMsg_SwapOut_ACK(routing_id)); // any postMessage IPCs scheduled from the unload handler are sent before
// the ACK (see https://crbug.com/857274).
auto send_swapout_ack = base::BindOnce(
[](int routing_id) {
RenderThread::Get()->Send(new FrameHostMsg_SwapOut_ACK(routing_id));
},
routing_id);
task_runner->PostTask(FROM_HERE, std::move(send_swapout_ack));
} }
void RenderFrameImpl::OnSwapIn() { void RenderFrameImpl::OnSwapIn() {
......
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