diff options
Diffstat (limited to 'chromium/content/renderer/render_frame_impl.cc')
| -rw-r--r-- | chromium/content/renderer/render_frame_impl.cc | 65 |
1 files changed, 58 insertions, 7 deletions
diff --git a/chromium/content/renderer/render_frame_impl.cc b/chromium/content/renderer/render_frame_impl.cc index c75b6177f06..dc3c7420d89 100644 --- a/chromium/content/renderer/render_frame_impl.cc +++ b/chromium/content/renderer/render_frame_impl.cc @@ -2427,7 +2427,46 @@ void RenderFrameImpl::OnSwapIn() { SwapIn(); } -void RenderFrameImpl::OnDeleteFrame() { +void RenderFrameImpl::OnDeleteFrame(FrameDeleteIntention intent) { + // The main frame (when not provisional) is owned by the renderer's frame tree + // via WebViewImpl. When a provisional main frame is swapped in, the ownership + // moves from the browser to the renderer, but this happens in the renderer + // process and is then the browser is informed. + // If the provisional main frame is swapped in while the browser is destroying + // it, the browser may request to delete |this|, thinking it has ownership + // of it, but the renderer has already taken ownership via SwapIn(). + switch (intent) { + case FrameDeleteIntention::kNotMainFrame: + // The frame was not a main frame, so the browser should always have + // ownership of it and we can just proceed with deleting it on + // request. + DCHECK(!is_main_frame_); + break; + case FrameDeleteIntention::kSpeculativeMainFrameForShutdown: + // In this case the renderer has taken ownership of the provisional main + // frame but the browser did not know yet and is shutting down. We can + // ignore this request as the frame will be destroyed when the RenderView + // is. This handles the shutdown case of https://crbug.com/957858. + DCHECK(is_main_frame_); + if (in_frame_tree_) + return; + break; + case FrameDeleteIntention::kSpeculativeMainFrameForNavigationCancelled: + // In this case the browser was navigating and cancelled the speculative + // navigation. The renderer *should* undo the SwapIn() but the old state + // has already been destroyed. Both ignoring the message or handling it + // would leave the renderer in an inconsistent state now. If we ignore it + // then the browser thinks the RenderView has a remote main frame, but it + // is incorrect. If we handle it, then we are deleting a local main frame + // out from under the RenderView and we will have bad pointers in the + // renderer. So all we can do is crash. We should instead prevent this + // scenario by blocking the browser from dropping the speculative main + // frame when a commit (and ownership transfer) is imminent. + // TODO(dcheng): This is the case of https://crbug.com/838348. + DCHECK(is_main_frame_); + break; + } + // This will result in a call to RenderFrameImpl::FrameDetached, which // deletes the object. Do not access |this| after detach. frame_->Detach(); @@ -2508,6 +2547,7 @@ void RenderFrameImpl::OnAddMessageToConsole( void RenderFrameImpl::JavaScriptExecuteRequest( const base::string16& javascript, + bool wants_result, JavaScriptExecuteRequestCallback callback) { TRACE_EVENT_INSTANT0("test_tracing", "JavaScriptExecuteRequest", TRACE_EVENT_SCOPE_THREAD); @@ -2522,11 +2562,15 @@ void RenderFrameImpl::JavaScriptExecuteRequest( if (!weak_this) return; - std::move(callback).Run(GetJavaScriptExecutionResult(result)); + if (wants_result) + std::move(callback).Run(GetJavaScriptExecutionResult(result)); + else + std::move(callback).Run({}); } void RenderFrameImpl::JavaScriptExecuteRequestForTests( const base::string16& javascript, + bool wants_result, bool has_user_gesture, JavaScriptExecuteRequestForTestsCallback callback) { TRACE_EVENT_INSTANT0("test_tracing", "JavaScriptExecuteRequestForTests", @@ -2548,11 +2592,15 @@ void RenderFrameImpl::JavaScriptExecuteRequestForTests( if (!weak_this) return; - std::move(callback).Run(GetJavaScriptExecutionResult(result)); + if (wants_result) + std::move(callback).Run(GetJavaScriptExecutionResult(result)); + else + std::move(callback).Run({}); } void RenderFrameImpl::JavaScriptExecuteRequestInIsolatedWorld( const base::string16& javascript, + bool wants_result, int32_t world_id, JavaScriptExecuteRequestInIsolatedWorldCallback callback) { TRACE_EVENT_INSTANT0("test_tracing", @@ -2571,15 +2619,18 @@ void RenderFrameImpl::JavaScriptExecuteRequestInIsolatedWorld( v8::HandleScope handle_scope(v8::Isolate::GetCurrent()); WebScriptSource script = WebScriptSource(WebString::FromUTF16(javascript)); JavaScriptIsolatedWorldRequest* request = new JavaScriptIsolatedWorldRequest( - weak_factory_.GetWeakPtr(), std::move(callback)); + weak_factory_.GetWeakPtr(), wants_result, std::move(callback)); frame_->RequestExecuteScriptInIsolatedWorld( world_id, &script, 1, false, WebLocalFrame::kSynchronous, request); } RenderFrameImpl::JavaScriptIsolatedWorldRequest::JavaScriptIsolatedWorldRequest( base::WeakPtr<RenderFrameImpl> render_frame_impl, + bool wants_result, JavaScriptExecuteRequestInIsolatedWorldCallback callback) - : render_frame_impl_(render_frame_impl), callback_(std::move(callback)) {} + : render_frame_impl_(render_frame_impl), + wants_result_(wants_result), + callback_(std::move(callback)) {} RenderFrameImpl::JavaScriptIsolatedWorldRequest:: ~JavaScriptIsolatedWorldRequest() { @@ -2594,7 +2645,7 @@ void RenderFrameImpl::JavaScriptIsolatedWorldRequest::Completed( } base::Value value; - if (!result.empty()) { + if (!result.empty() && wants_result_) { // It's safe to always use the main world context when converting // here. V8ValueConverterImpl shouldn't actually care about the // context scope, and it switches to v8::Object's creation context @@ -3101,7 +3152,7 @@ void RenderFrameImpl::LoadErrorPage(int reason) { } void RenderFrameImpl::ExecuteJavaScript(const base::string16& javascript) { - JavaScriptExecuteRequest(javascript, base::DoNothing()); + JavaScriptExecuteRequest(javascript, false, base::DoNothing()); } void RenderFrameImpl::BindLocalInterface( |