Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(940)

Unified Diff: content/renderer/render_frame_impl.cc

Issue 2974553002: Fix NavigationHandle being thrown out and recreated (without all the state) after rapid navigations. (Closed)
Patch Set: fix Created 3 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « content/renderer/render_frame_impl.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: content/renderer/render_frame_impl.cc
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc
index 4ae46edcbcfd761358664b569604c6c7a59dc2b6..eadb6fcefd26c357799a907f71ed60080dd42a11 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -1753,6 +1753,7 @@ void RenderFrameImpl::OnBeforeUnload(bool is_reload) {
// Save the routing_id, as the RenderFrameImpl can be deleted in
// dispatchBeforeUnloadEvent. See https://crbug.com/666714 for details.
int routing_id = routing_id_;
+ base::WeakPtr<RenderFrameImpl> weak_this = weak_factory_.GetWeakPtr();
base::TimeTicks before_unload_start_time = base::TimeTicks::Now();
@@ -1761,6 +1762,15 @@ void RenderFrameImpl::OnBeforeUnload(bool is_reload) {
// execute the BeforeUnload event in this frame and local child frames. It
// should also be dispatched to out-of-process child frames.
bool proceed = frame_->DispatchBeforeUnloadEvent(is_reload);
+ if (IsBrowserSideNavigationEnabled() && proceed && weak_this) {
+ ran_before_unload_ = true;
+ // If there's already a pending navigation its didcommitprovisionalload or
+ // didstoploading IPCs (exact one which fires is racy) would interfere with
+ // the new navigation. By forcing the old navigation to be cancelled first,
+ // then we are assured that the IPC pipe is flushed of IPCs related to a
+ // previous navigation.
+ frame_->StopLoading();
jam 2017/07/08 01:32:49 this prevents stray FrameHostMsg_DidCommitProvisio
clamy 2017/07/10 13:02:05 That seems wrong to me. If we're trying to load a
+ }
base::TimeTicks before_unload_end_time = base::TimeTicks::Now();
RenderThread::Get()->Send(new FrameHostMsg_BeforeUnload_ACK(
@@ -5073,6 +5083,13 @@ void RenderFrameImpl::DidStopLoading() {
SendUpdateFaviconURL(icon_types_mask);
render_view_->FrameDidStopLoading(frame_);
+
+ if (IsBrowserSideNavigationEnabled() && ran_before_unload_) {
+ // Avoid a previous navigation's didstop IPC from resetting the
+ // NavigationHandle of the next navigation.
+ return;
jam 2017/07/08 01:32:49 this prevents the other case of stray FrameHostMsg
+ }
+
Send(new FrameHostMsg_DidStopLoading(routing_id_));
}
@@ -5163,6 +5180,7 @@ void RenderFrameImpl::OnCommitNavigation(
: nullptr);
browser_side_navigation_pending_ = false;
+ ran_before_unload_ = false;
NavigateInternal(common_params, StartNavigationParams(), request_params,
std::move(stream_override));
@@ -5194,6 +5212,7 @@ void RenderFrameImpl::OnFailedNavigation(
pending_navigation_params_.reset(new NavigationParams(
common_params, StartNavigationParams(), request_params));
+ ran_before_unload_ = false;
// Send the provisional load failure.
blink::WebURLError error =
« no previous file with comments | « content/renderer/render_frame_impl.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698