Remove IsSynchronous API from NavigationHandle.

content/browser/frame_host/render_frame_host_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/browser/frame_host/render_frame_host_impl.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/command_line.h"
@@ skipping 1200 matching lines @@
   // Synchronous renderer-initiated navigations will send a
   // DidCommitProvisionalLoad IPC without a prior DidStartProvisionalLoad
   // message.
   if (!navigation_handle_) {
     // There is no pending NavigationEntry in these cases, so pass 0 as the
     // nav_id. If the previous handle was a prematurely aborted navigation
     // loaded via LoadDataWithBaseURL, propogate the entry id.
     navigation_handle_ = NavigationHandleImpl::Create(
         validated_params.url, frame_tree_node_,
         is_renderer_initiated,
-        true,  // is_synchronous
+        validated_params.was_within_same_page,

[Charlie Reis, 2016/09/16 22:23:53]

I don't understand this change.  When is this false?

[nasko, 2016/09/16 23:38:11]

Unfortunately, there are some cases where this is false:
* loading of data URL with base URL - the URL in the handle is the base URL and
the committed URL is the real data URL.
* blocking a document load due to CSP - the URL in the handle is the real URL we
started loading, but when blocked, we load "data:," into the frame and that
comes as the committed URL.

These cases cause the code above, on line 1208, to reset the existing
navigation_handle_. So going into this branch, we assume it is a synchronous
load, even though it isn't. Using validated_params.was_within_same_page accounts
for those cases as it has the correct value passed by the renderer.

[Charlie Reis, 2016/09/16 23:59:16]

Thanks!  That clarifies it.  Can you update the comment on line 1211, since this
block is not just about synchronous navigations?

[nasko, 2016/09/20 23:11:20]

Tried to clarify both this comment and the one above.

         validated_params.is_srcdoc, base::TimeTicks::Now(),
         entry_id_for_data_nav);
     // PlzNavigate
     if (IsBrowserSideNavigationEnabled()) {
       // PlzNavigate: synchronous loads happen in the renderer, and the browser
       // has not been notified about the start of the load yet. Do it now.
       if (!is_loading()) {
         bool was_loading = frame_tree_node()->frame_tree()->IsLoading();
         is_loading_ = true;
         frame_tree_node()->DidStartLoading(true, was_loading);
@@ skipping 1860 matching lines @@
   web_bluetooth_service_->SetClientConnectionErrorHandler(base::Bind(
       &RenderFrameHostImpl::DeleteWebBluetoothService, base::Unretained(this)));
   return web_bluetooth_service_.get();
 }
 
 void RenderFrameHostImpl::DeleteWebBluetoothService() {
   web_bluetooth_service_.reset();
 }
 
 }  // namespace content