Remove IsSynchronous API from NavigationHandle.

content/test/test_render_frame_host.cc

Index: content/test/test_render_frame_host.cc
diff --git a/content/test/test_render_frame_host.cc b/content/test/test_render_frame_host.cc
index 2aced691760742d41c2c17acd630916e5af61826..cf0c5be0317683648490cc650f0bdff46921eee5 100644
--- a/content/test/test_render_frame_host.cc
+++ b/content/test/test_render_frame_host.cc
@@ -64,7 +64,8 @@ TestRenderFrameHost::TestRenderFrameHost(SiteInstance* site_instance,
                           flags),
       child_creation_observer_(delegate ? delegate->GetAsWebContents() : NULL),
       contents_mime_type_("text/html"),
-      simulate_history_list_was_cleared_(false) {}
+      simulate_history_list_was_cleared_(false),
+      last_commit_was_error_page_(false) {}
 
 TestRenderFrameHost::~TestRenderFrameHost() {
 }
@@ -157,8 +158,10 @@ void TestRenderFrameHost::SimulateNavigationCommit(const GURL& url) {
   url::Replacements<char> replacements;
   replacements.ClearRef();
   params.was_within_same_page =
-      url.ReplaceComponents(replacements) ==
-      GetLastCommittedURL().ReplaceComponents(replacements);
+      (GetLastCommittedURL().is_valid() &&
+       !last_commit_was_error_page_ &&
+       url.ReplaceComponents(replacements) ==
+           GetLastCommittedURL().ReplaceComponents(replacements));

[Charlie Reis, 2016/10/27 20:51:24]

This replacements check seems wrong to me, but maybe it's sufficient for unit
tests.  (For example, it can't handle pushState, which is allowed to change the
URL to be anything else within the same origin.)

Maybe it's worth adding a comment as a warning to others?  (I certainly don't
want this pattern to be copy/pasted elsewhere.)  :)

[nasko, 2016/10/27 21:22:34]

Unit tests can definitely use some more love : ). This is also why the change of
start URL was needed in the captive portal unit tests.
My goal here was to make it as correct as I can in the context of the changes
I'm making. I don't think we can handle pushState with this method and will
likely require a separate one.
Took a stab at adding a scary comment :).

 
   params.page_state = PageState::CreateForTesting(url, false, nullptr, nullptr);
 
@@ -353,8 +356,10 @@ void TestRenderFrameHost::SendNavigateWithParameters(
   params.was_within_same_page =
       !ui::PageTransitionCoreTypeIs(transition, ui::PAGE_TRANSITION_RELOAD) &&
       !ui::PageTransitionCoreTypeIs(transition, ui::PAGE_TRANSITION_TYPED) &&
-      url_copy.ReplaceComponents(replacements) ==
-          GetLastCommittedURL().ReplaceComponents(replacements);
+      (GetLastCommittedURL().is_valid() &&
+       !last_commit_was_error_page_ &&
+       url_copy.ReplaceComponents(replacements) ==
+           GetLastCommittedURL().ReplaceComponents(replacements));
 
   params.page_state =
       PageState::CreateForTesting(url_copy, false, nullptr, nullptr);
@@ -369,6 +374,7 @@ void TestRenderFrameHost::SendNavigateWithParams(
     FrameHostMsg_DidCommitProvisionalLoad_Params* params) {
   FrameHostMsg_DidCommitProvisionalLoad msg(GetRoutingID(), *params);
   OnDidCommitProvisionalLoad(msg);
+  last_commit_was_error_page_ = params->url_is_unreachable;
 }
 
 void TestRenderFrameHost::SendRendererInitiatedNavigationRequest(