diff options
| -rw-r--r-- | chromium/content/renderer/render_frame_impl.cc | 10 | ||||
| -rw-r--r-- | chromium/content/renderer/render_frame_impl.h | 5 | ||||
| -rw-r--r-- | chromium/content/renderer/render_view_browsertest.cc | 25 | ||||
| -rw-r--r-- | chromium/content/renderer/render_view_impl.h | 1 |
4 files changed, 39 insertions, 2 deletions
diff --git a/chromium/content/renderer/render_frame_impl.cc b/chromium/content/renderer/render_frame_impl.cc index b715060a9c7..861eda159b4 100644 --- a/chromium/content/renderer/render_frame_impl.cc +++ b/chromium/content/renderer/render_frame_impl.cc @@ -887,6 +887,11 @@ void RenderFrameImpl::DidHideExternalPopupMenu() { #endif bool RenderFrameImpl::OnMessageReceived(const IPC::Message& msg) { + // We may get here while detaching, when the WebFrame has been deleted. Do + // not process any messages in this state. + if (!frame_) + return false; + // TODO(kenrb): document() should not be null, but as a transitional step // we have RenderFrameProxy 'wrapping' a RenderFrameImpl, passing messages // to this method. This happens for a top-level remote frame, where a @@ -1932,8 +1937,11 @@ void RenderFrameImpl::frameDetached(blink::WebFrame* frame) { if (is_subframe) frame->parent()->removeChild(frame); - // |frame| is invalid after here. + // |frame| is invalid after here. Be sure to clear frame_ as well, since this + // object may not be deleted immediately and other methods may try to access + // it. frame->close(); + frame_ = nullptr; if (is_subframe) { delete this; diff --git a/chromium/content/renderer/render_frame_impl.h b/chromium/content/renderer/render_frame_impl.h index 77d49414589..706c29dd036 100644 --- a/chromium/content/renderer/render_frame_impl.h +++ b/chromium/content/renderer/render_frame_impl.h @@ -678,7 +678,10 @@ class CONTENT_EXPORT RenderFrameImpl RendererCdmManager* GetCdmManager(); #endif - // Stores the WebLocalFrame we are associated with. + // Stores the WebLocalFrame we are associated with. This is null from the + // constructor until SetWebFrame is called, and it is null after + // frameDetached is called until destruction (which is asynchronous in the + // case of the main frame, but not subframes). blink::WebLocalFrame* frame_; base::WeakPtr<RenderViewImpl> render_view_; diff --git a/chromium/content/renderer/render_view_browsertest.cc b/chromium/content/renderer/render_view_browsertest.cc index bc75a42a891..eb90dd17aee 100644 --- a/chromium/content/renderer/render_view_browsertest.cc +++ b/chromium/content/renderer/render_view_browsertest.cc @@ -287,6 +287,31 @@ class RenderViewImplTest : public RenderViewTest { scoped_ptr<MockKeyboard> mock_keyboard_; }; +// Test for https://crbug.com/461191. +TEST_F(RenderViewImplTest, RenderFrameMessageAfterDetach) { + // Create a new main frame RenderFrame so that we don't interfere with the + // shutdown of frame() in RenderViewTest.TearDown. + blink::WebURLRequest popup_request(GURL("http://foo.com")); + blink::WebView* new_web_view = view()->createView( + GetMainFrame(), popup_request, blink::WebWindowFeatures(), "foo", + blink::WebNavigationPolicyNewForegroundTab, false); + RenderViewImpl* new_view = RenderViewImpl::FromWebView(new_web_view); + RenderFrameImpl* new_frame = + static_cast<RenderFrameImpl*>(new_view->GetMainRenderFrame()); + + // Detach the main frame. + new_view->Close(); + + // Before the frame is asynchronously deleted, it may receive a message. + // We should not crash here, and the message should not be processed. + scoped_ptr<const IPC::Message> msg( + new FrameMsg_Stop(frame()->GetRoutingID())); + EXPECT_FALSE(new_frame->OnMessageReceived(*msg)); + + // Clean up after the new view so we don't leak it. + new_view->Release(); +} + TEST_F(RenderViewImplTest, SaveImageFromDataURL) { const IPC::Message* msg1 = render_thread_->sink().GetFirstMessageMatching( ViewHostMsg_SaveImageFromDataURL::ID); diff --git a/chromium/content/renderer/render_view_impl.h b/chromium/content/renderer/render_view_impl.h index be982b09b50..86e6d19b542 100644 --- a/chromium/content/renderer/render_view_impl.h +++ b/chromium/content/renderer/render_view_impl.h @@ -562,6 +562,7 @@ class CONTENT_EXPORT RenderViewImpl // code away from this class. friend class RenderFrameImpl; + FRIEND_TEST_ALL_PREFIXES(RenderViewImplTest, RenderFrameMessageAfterDetach); FRIEND_TEST_ALL_PREFIXES(RenderViewImplTest, DecideNavigationPolicyForWebUI); FRIEND_TEST_ALL_PREFIXES(RenderViewImplTest, DidFailProvisionalLoadWithErrorForError); |