Remove WebContentsObserver::DidCommitNavigation

content/test/web_contents_observer_sanity_checker.cc

Index: content/test/web_contents_observer_sanity_checker.cc
diff --git a/content/test/web_contents_observer_sanity_checker.cc b/content/test/web_contents_observer_sanity_checker.cc
index 82bcb60461840a005453f0d271ce50cd3de1e7bd..f34a193c04f22ff7eb0bb6c9642b047fa578a3b4 100644
--- a/content/test/web_contents_observer_sanity_checker.cc
+++ b/content/test/web_contents_observer_sanity_checker.cc
@@ -124,11 +124,9 @@ void WebContentsObserverSanityChecker::FrameDeleted(
 void WebContentsObserverSanityChecker::DidStartNavigation(
     NavigationHandle* navigation_handle) {
   CHECK(!NavigationIsOngoing(navigation_handle));
-  CHECK(!NavigationIsOngoingAndCommitted(navigation_handle));
 
   CHECK(navigation_handle->GetNetErrorCode() == net::OK);
-  CHECK(!navigation_handle->HasCommittedDocument());
-  CHECK(!navigation_handle->HasCommittedErrorPage());
+  CHECK(!navigation_handle->HasCommitted());

[Charlie Reis, 2015/09/21 16:40:42]

We can check !IsErrorPage() as well, right?  (Same below?)

[clamy, 2015/09/22 00:38:05]

Done.

 
   ongoing_navigations_.insert(navigation_handle);
 }
@@ -136,51 +134,32 @@ void WebContentsObserverSanityChecker::DidStartNavigation(
 void WebContentsObserverSanityChecker::DidRedirectNavigation(
     NavigationHandle* navigation_handle) {
   CHECK(NavigationIsOngoing(navigation_handle));
-  CHECK(!NavigationIsOngoingAndCommitted(navigation_handle));
 
   CHECK(navigation_handle->GetNetErrorCode() == net::OK);
-  CHECK(!navigation_handle->HasCommittedDocument());
-  CHECK(!navigation_handle->HasCommittedErrorPage());
+  CHECK(!navigation_handle->HasCommitted());
 }
 
 void WebContentsObserverSanityChecker::ReadyToCommitNavigation(
     NavigationHandle* navigation_handle) {
   CHECK(NavigationIsOngoing(navigation_handle));
-  CHECK(!NavigationIsOngoingAndCommitted(navigation_handle));
 
-  CHECK(!navigation_handle->HasCommittedDocument());
-  CHECK(!navigation_handle->HasCommittedErrorPage());
-}
-
-void WebContentsObserverSanityChecker::DidCommitNavigation(
-    NavigationHandle* navigation_handle) {
-  CHECK(NavigationIsOngoing(navigation_handle));
-  CHECK(!NavigationIsOngoingAndCommitted(navigation_handle));
-
-  CHECK_NE(navigation_handle->HasCommittedDocument(),
-           navigation_handle->HasCommittedErrorPage());
-  CHECK_IMPLIES(navigation_handle->HasCommittedDocument(),
-                navigation_handle->GetNetErrorCode() == net::OK);
-  CHECK_IMPLIES(navigation_handle->HasCommittedErrorPage(),
-                navigation_handle->GetNetErrorCode() != net::OK);
-
-  ongoing_committed_navigations_.insert(navigation_handle);
+  CHECK(!navigation_handle->HasCommitted());
+  CHECK(navigation_handle->GetRenderFrameHost());
 }
 
 void WebContentsObserverSanityChecker::DidFinishNavigation(
     NavigationHandle* navigation_handle) {
   CHECK(NavigationIsOngoing(navigation_handle));
 
-  CHECK_IMPLIES(NavigationIsOngoingAndCommitted(navigation_handle),
-                navigation_handle->HasCommittedDocument() !=
-                    navigation_handle->HasCommittedErrorPage());
-  CHECK_IMPLIES(navigation_handle->HasCommittedDocument(),
-                navigation_handle->GetNetErrorCode() == net::OK);
-  CHECK_IMPLIES(navigation_handle->HasCommittedErrorPage(),
-                navigation_handle->GetNetErrorCode() != net::OK);
+  CHECK_IMPLIES(
+      navigation_handle->HasCommitted() && !navigation_handle->IsErrorPage(),
+      navigation_handle->GetNetErrorCode() == net::OK);
+  CHECK_IMPLIES(
+      navigation_handle->HasCommitted() && navigation_handle->IsErrorPage(),
+      navigation_handle->GetNetErrorCode() != net::OK);
 
-  if (NavigationIsOngoingAndCommitted(navigation_handle))
-    ongoing_committed_navigations_.erase(navigation_handle);
+  CHECK_IMPLIES(navigation_handle->HasCommitted(),

[Charlie Reis, 2015/09/21 16:40:42]

Should this be CHECK_EQ?  (In other words, does the implication go both ways,
where a null RFH implies the navigation hasn't committed?)

[clamy, 2015/09/22 00:38:05]

No this is an implication because I've put a CHECK in the accessor that will
break if called before the navigation is ready to commit.

+                navigation_handle->GetRenderFrameHost() != nullptr);
 
   ongoing_navigations_.erase(navigation_handle);
 }
@@ -287,7 +266,6 @@ void WebContentsObserverSanityChecker::WebContentsDestroyed() {
   CHECK(!web_contents_destroyed_);
   web_contents_destroyed_ = true;
   CHECK(ongoing_navigations_.empty());
-  CHECK(ongoing_committed_navigations_.empty());
 }
 
 WebContentsObserverSanityChecker::WebContentsObserverSanityChecker(
@@ -340,10 +318,4 @@ bool WebContentsObserverSanityChecker::NavigationIsOngoing(
   return it != ongoing_navigations_.end();
 }
 
-bool WebContentsObserverSanityChecker::NavigationIsOngoingAndCommitted(
-    NavigationHandle* navigation_handle) {
-  auto it = ongoing_committed_navigations_.find(navigation_handle);
-  return it != ongoing_committed_navigations_.end();
-}
-
 }  // namespace content