Renderer/browser frame state can get out of sync |
||
Issue descriptionI wrote a test, but the basic idea is: 1. Load an A(B) frame tree. 2. Start a navigation in B's iframe to A (remote-to-local swap in A). 3. After the browser has started sending data to the renderer, but before the renderer has sent the DidCommitProvisionalLoad, start a browser-initiated navigation in B to C. 4. The speculative RFH is discarded in the browser (https://chromium.googlesource.com/chromium/src/+/363e1ec60c11f8037fbeca0addeb6eb68d3503fa/content/browser/frame_host/render_frame_host_manager.cc#654), but the renderer commits synchronously (https://chromium.googlesource.com/chromium/src/+/363e1ec60c11f8037fbeca0addeb6eb68d3503fa/content/renderer/render_frame_impl.cc#4211). The timing in 3 is really hard to hit, the test I wrote isn't ideal, because it's hooking into DidCommitProvisionalLoad. I couldn't find the right place to make sure that the data has been sent via mojo to the renderer. This can also be reproduced by having a RunLoop on the browser, attaching a debugger to the A's renderer and quitting the RunLoop in the browser after the renderer has seen DocumentLoader::DataReceived(). class DidCommitProvisionalLoadObserver : public mojom::FrameHostInterceptorForTesting { public: DidCommitProvisionalLoadObserver(RenderFrameHostImpl* rfhi, base::OnceCallback<void()> callback) : forwarding_interface_(rfhi), callback_(std::move(callback)) { rfhi->frame_host_binding_for_testing().SwapImplForTesting(this); } void DidCommitProvisionalLoad( std::unique_ptr<::FrameHostMsg_DidCommitProvisionalLoad_Params> params, ::service_manager::mojom::InterfaceProviderRequest interface_provider_request) override { std::move(callback_).Run(); // RFHI will be destroyed in the callback, so DidCommit is dropped. run_loop_.Quit(); } FrameHost* GetForwardingInterface() override { return forwarding_interface_; } void Wait() { run_loop_.Run(); } private: FrameHost* forwarding_interface_; base::OnceCallback<void()> callback_; base::RunLoop run_loop_; }; IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, SwappingWrongFrame) { GURL main_url(embedded_test_server()->GetURL( "a.com", "/cross_site_iframe_factory.html?a(b)")); GURL a_url(embedded_test_server()->GetURL("a.com", "/title1.html")); GURL c_url(embedded_test_server()->GetURL("c.com", "/title1.html")); EXPECT_TRUE(NavigateToURL(shell(), main_url)); FrameTreeNode* root = web_contents()->GetFrameTree()->root(); FrameTreeNode* child = root->child_at(0); { NavigationController::LoadURLParams params(a_url); params.transition_type = ui::PAGE_TRANSITION_LINK; params.frame_tree_node_id = child->frame_tree_node_id(); child->navigator()->GetController()->LoadURLWithParams(params); } DidCommitProvisionalLoadObserver observer( child->render_manager()->speculative_frame_host(), base::Bind( [](FrameTreeNode* child, const GURL& url) { NavigationController::LoadURLParams params(url); params.transition_type = ui::PAGE_TRANSITION_LINK; params.frame_tree_node_id = child->frame_tree_node_id(); child->navigator()->GetController()->LoadURLWithParams(params); }, child, c_url)); observer.Wait(); base::RunLoop().Run(); }
,
Mar 8 2018
Thanks for catching this! clamy@, can you help triage it? I'm curious if we should avoid discarding the speculative RFH after we get past the stage where we tell the renderer to use it (i.e., CommitNavigation). Maybe that doesn't work, or isn't sufficient? I'm curious what happens after we get into this state-- does it crash, or risk memory bugs?
,
Mar 8 2018
I'm not sure if I understand the issue correctly. We're trying to commit a navigation in a speculative RFH, it commits in the render process but the process itself gets deleted before the DidCommitProvisionalLoad IPC is processed on the browser side. This is something that could have happened with the pending RFH in the past (with the exact same test) and didn't seem to be an issue. I don't see why it would be now?
,
Mar 8 2018
Re#2: There are no crashes, and while there is an extra RFH/RPH that isn't being used, they are still owned by the FrameTree and will be cleaned up with by the WebContents. Since the RFP in the parent's renderer is deleted, I don't think they can communicate with each other, so I couldn't think of any possible exploits either. Re#3: The process doesn't get deleted because the parent is still there. The problem is that the parent committed locally, while the browser committed the navigation in a different RFH. |
||
►
Sign in to add a comment |
||
Comment 1 by lfg@chromium.org
, Feb 5 2018