Remove IsSynchronous API from NavigationHandle.

content/browser/frame_host/navigation_handle_impl.cc

 // Copyright 2015 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/navigation_handle_impl.h"
 
 #include <utility>
 
 #include "base/logging.h"
 #include "content/browser/browsing_data/clear_site_data_throttle.h"
@@ skipping 21 matching lines @@
   *to_update = result;
 }
 
 }  // namespace
 
 // static
 std::unique_ptr<NavigationHandleImpl> NavigationHandleImpl::Create(
     const GURL& url,
     FrameTreeNode* frame_tree_node,
     bool is_renderer_initiated,
-    bool is_synchronous,
+    bool is_same_page,
     bool is_srcdoc,
     const base::TimeTicks& navigation_start,
     int pending_nav_entry_id) {
-  return std::unique_ptr<NavigationHandleImpl>(
-      new NavigationHandleImpl(url, frame_tree_node, is_renderer_initiated,
-                               is_synchronous, is_srcdoc, navigation_start,
-                               pending_nav_entry_id));
+  return std::unique_ptr<NavigationHandleImpl>(new NavigationHandleImpl(
+      url, frame_tree_node, is_renderer_initiated, is_same_page, is_srcdoc,
+      navigation_start, pending_nav_entry_id));
 }
 
 NavigationHandleImpl::NavigationHandleImpl(
     const GURL& url,
     FrameTreeNode* frame_tree_node,
     bool is_renderer_initiated,
-    bool is_synchronous,
+    bool is_same_page,
     bool is_srcdoc,
     const base::TimeTicks& navigation_start,
     int pending_nav_entry_id)
     : url_(url),
       has_user_gesture_(false),
       transition_(ui::PAGE_TRANSITION_LINK),
       is_external_protocol_(false),
       net_error_code_(net::OK),
       render_frame_host_(nullptr),
       is_renderer_initiated_(is_renderer_initiated),
-      is_same_page_(false),
-      is_synchronous_(is_synchronous),
+      is_same_page_(is_same_page),
       is_srcdoc_(is_srcdoc),
       was_redirected_(false),
       state_(INITIAL),
       is_transferring_(false),
       frame_tree_node_(frame_tree_node),
       next_index_(0),
       navigation_start_(navigation_start),
       pending_nav_entry_id_(pending_nav_entry_id),
       request_context_type_(REQUEST_CONTEXT_TYPE_UNSPECIFIED) {
   DCHECK(!navigation_start.is_null());
@@ skipping 42 matching lines @@
   if (frame_tree_node_->parent())
     return frame_tree_node_->parent()->IsMainFrame();
 
   return false;
 }
 
 bool NavigationHandleImpl::IsRendererInitiated() {
   return is_renderer_initiated_;
 }
 
-bool NavigationHandleImpl::IsSynchronousNavigation() {
-  return is_synchronous_;
-}
-
 bool NavigationHandleImpl::IsSrcdoc() {
   return is_srcdoc_;
 }
 
 bool NavigationHandleImpl::WasServerRedirect() {
   return was_redirected_;
 }
 
 int NavigationHandleImpl::GetFrameTreeNodeId() {
   return frame_tree_node_->frame_tree_node_id();
@@ skipping 48 matching lines @@
   // TODO(mkwst): Change this to check against 'READY_TO_COMMIT' once
   // ReadyToCommitNavigation is available whether or not PlzNavigate is
   // enabled. https://crbug.com/621856
   CHECK_GE(state_, WILL_PROCESS_RESPONSE)
       << "This accessor should only be called after a response has been "
          "delivered for processing.";
   return render_frame_host_;
 }
 
 bool NavigationHandleImpl::IsSamePage() {
-  DCHECK(state_ == DID_COMMIT || state_ == DID_COMMIT_ERROR_PAGE)

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

Yes, this is exactly what I hoped for.  It should be possible to call IsSamePage
at DidStartnavigation so that callers can understand if they will be getting a
different document or not.

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

Acknowledged.

-      << "This accessor should not be called before the navigation has "
-         "committed.";
-  return is_same_page_;
+  return is_same_page_ && url_ != GURL(url::kAboutBlankURL);

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

Hmm, I don't think it makes sense for is_same_page_ to be true for about:blank,
even if IsSamePage() returns the right value.  It seems too risky for other code
in NavigationHandleImpl to do the wrong thing by checking is_same_page_
directly.

Is there another way to handle this?

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

After poking at this a bit more, I think I don't need the about:blank
restriction, so I'm very happy to remove it.

 }
 
 const net::HttpResponseHeaders* NavigationHandleImpl::GetResponseHeaders() {
   DCHECK_GE(state_, WILL_REDIRECT_REQUEST);
   return response_headers_.get();
 }
 
 bool NavigationHandleImpl::HasCommitted() {
   return state_ == DID_COMMIT || state_ == DID_COMMIT_ERROR_PAGE;
 }
@@ skipping 229 matching lines @@
   if (IsBrowserSideNavigationEnabled())
     GetDelegate()->ReadyToCommitNavigation(this);
 }
 
 void NavigationHandleImpl::DidCommitNavigation(
     const FrameHostMsg_DidCommitProvisionalLoad_Params& params,
     bool same_page,
     RenderFrameHostImpl* render_frame_host) {
   DCHECK(!render_frame_host_ || render_frame_host_ == render_frame_host);
   DCHECK_EQ(frame_tree_node_, render_frame_host->frame_tree_node());
+  DCHECK_EQ(same_page, IsSamePage());
   CHECK_EQ(url_, params.url);
 
   method_ = params.method;
   has_user_gesture_ = (params.gesture == NavigationGestureUser);
   transition_ = params.transition;
   render_frame_host_ = render_frame_host;
-  is_same_page_ = same_page;
 
   state_ = net_error_code_ == net::OK ? DID_COMMIT : DID_COMMIT_ERROR_PAGE;
 }
 
 NavigationThrottle::ThrottleCheckResult
 NavigationHandleImpl::CheckWillStartRequest() {
   DCHECK(state_ == WILL_SEND_REQUEST || state_ == DEFERRING_START);
   DCHECK(state_ != WILL_SEND_REQUEST || next_index_ == 0);
   DCHECK(state_ != DEFERRING_START || next_index_ != 0);
   for (size_t i = next_index_; i < throttles_.size(); ++i) {
@@ skipping 119 matching lines @@
     throttles_to_register.push_back(std::move(clear_site_data_throttle));
 
   if (throttles_to_register.size() > 0) {
     throttles_.insert(throttles_.begin(), throttles_to_register.begin(),
                       throttles_to_register.end());
     throttles_to_register.weak_clear();
   }
 }
 
 }  // namespace content