Fix NavigationHandle being thrown out and recreated (without all the state) after rapid navigations.

content/renderer/render_frame_impl.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/renderer/render_frame_impl.h"
 
 #include <algorithm>
 #include <map>
 #include <string>
 #include <utility>
@@ skipping 1735 matching lines @@
     std::unique_ptr<NavigationParams> navigation_params) {
   pending_navigation_params_ = std::move(navigation_params);
 }
 
 void RenderFrameImpl::OnBeforeUnload(bool is_reload) {
   TRACE_EVENT1("navigation,rail", "RenderFrameImpl::OnBeforeUnload",
                "id", routing_id_);
   // 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();
 
   // TODO(clamy): Ensure BeforeUnload is dispatched to all subframes, even when
   // --site-per-process is enabled. |dispatchBeforeUnloadEvent| will only
   // 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_DidCommitProvisionalLoad and sometimes
FrameHostMsg_DidStopLoading

[clamy, 2017/07/10 13:02:05]

That seems wrong to me. If we're trying to load a page and the navigation
doesn't go through (eg if it's cancelled), the user will be stuck with a
half-loaded page. I don't think this is what we want.

+  }
 
   base::TimeTicks before_unload_end_time = base::TimeTicks::Now();
   RenderThread::Get()->Send(new FrameHostMsg_BeforeUnload_ACK(
       routing_id, proceed, before_unload_start_time, before_unload_end_time));
 }
 
 void RenderFrameImpl::OnSwapOut(
     int proxy_routing_id,
     bool is_loading,
     const FrameReplicationState& replicated_frame_state) {
@@ skipping 3292 matching lines @@
   history_subframe_unique_names_.clear();
 
   blink::WebIconURL::Type icon_types_mask =
       static_cast<blink::WebIconURL::Type>(
           blink::WebIconURL::kTypeFavicon |
           blink::WebIconURL::kTypeTouchPrecomposed |
           blink::WebIconURL::kTypeTouch);
   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_DidStopLoading being sent
(when the loading was scheduled later)

+  }
+
   Send(new FrameHostMsg_DidStopLoading(routing_id_));
 }
 
 void RenderFrameImpl::DidChangeLoadProgress(double load_progress) {
   Send(new FrameHostMsg_DidChangeLoadProgress(routing_id_, load_progress));
 }
 
 void RenderFrameImpl::HandleWebAccessibilityEvent(
     const blink::WebAXObject& obj, blink::WebAXEvent event) {
   if (render_accessibility_)
@@ skipping 70 matching lines @@
         mojo::ScopedMessagePipeHandle(commit_data.url_loader_factory), 0u));
   }
 
   // If the request was initiated in the context of a user gesture then make
   // sure that the navigation also executes in the context of a user gesture.
   std::unique_ptr<blink::WebScopedUserGesture> gesture(
       request_params.has_user_gesture ? new blink::WebScopedUserGesture(frame_)
                                       : nullptr);
 
   browser_side_navigation_pending_ = false;
+  ran_before_unload_ = false;
 
   NavigateInternal(common_params, StartNavigationParams(), request_params,
                    std::move(stream_override));
 
   // Don't add code after this since NavigateInternal may have destroyed this
   // RenderFrameImpl.
 }
 
 // PlzNavigate
 void RenderFrameImpl::OnFailedNavigation(
@@ skipping 11 matching lines @@
 
   // If this frame is navigating cross-process, it may naively assume that this
   // is the first navigation in the frame, but this may not actually be the
   // case. Inform the frame's state machine if this frame has already committed
   // other loads.
   if (request_params.has_committed_real_load)
     frame_->SetCommittedFirstRealLoad();
 
   pending_navigation_params_.reset(new NavigationParams(
       common_params, StartNavigationParams(), request_params));
+  ran_before_unload_ = false;
 
   // Send the provisional load failure.
   blink::WebURLError error =
       CreateWebURLError(common_params.url, has_stale_copy_in_cache, error_code);
   WebURLRequest failed_request =
       CreateURLRequestForNavigation(common_params, request_params,
                                     std::unique_ptr<StreamOverrideParameters>(),
                                     frame_->IsViewSourceModeEnabled(),
                                     false);  // is_same_document_navigation
 
@@ skipping 1696 matching lines @@
       replaces_current_history_item(info.replaces_current_history_item),
       history_navigation_in_new_child_frame(
           info.is_history_navigation_in_new_child_frame),
       client_redirect(info.is_client_redirect),
       triggering_event_info(info.triggering_event_info),
       cache_disabled(info.is_cache_disabled),
       form(info.form),
       source_location(info.source_location) {}
 
 }  // namespace content