Remove WebContentsObserver::DidCommitNavigation

content/test/web_contents_observer_sanity_checker.cc

 // Copyright 2014 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/test/web_contents_observer_sanity_checker.h"
 
 #include "base/strings/stringprintf.h"
 #include "content/browser/frame_host/render_frame_host_impl.h"
 #include "content/common/frame_messages.h"
 #include "content/public/browser/navigation_handle.h"
@@ skipping 106 matching lines @@
 void WebContentsObserverSanityChecker::FrameDeleted(
     RenderFrameHost* render_frame_host) {
   // A frame can be deleted before RenderFrame in the renderer process is
   // created, so there is not much that can be enforced here.
   CHECK(!web_contents_destroyed_);
 }
 
 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);
 }
 
 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);
 }
 
 void WebContentsObserverSanityChecker::DidStartProvisionalLoadForFrame(
     RenderFrameHost* render_frame_host,
     const GURL& validated_url,
     bool is_error_page,
     bool is_iframe_srcdoc) {
   AssertRenderFrameExists(render_frame_host);
@@ skipping 86 matching lines @@
 // TODO(avi): Disabled because of http://crbug.com/445054
   AssertRenderFrameExists(render_frame_host);
 #endif
   return false;
 }
 
 void WebContentsObserverSanityChecker::WebContentsDestroyed() {
   CHECK(!web_contents_destroyed_);
   web_contents_destroyed_ = true;
   CHECK(ongoing_navigations_.empty());
-  CHECK(ongoing_committed_navigations_.empty());
 }
 
 WebContentsObserverSanityChecker::WebContentsObserverSanityChecker(
     WebContents* web_contents)
     : WebContentsObserver(web_contents), web_contents_destroyed_(false) {
   // Prime the pump with the initial objects.
   // TODO(nasko): Investigate why this is needed.
   RenderViewCreated(web_contents->GetRenderViewHost());
 }
 
@@ skipping 32 matching lines @@
       render_frame_host->GetRoutingID(),
       render_frame_host->GetSiteInstance()->GetSiteURL().spec().c_str());
 }
 
 bool WebContentsObserverSanityChecker::NavigationIsOngoing(
     NavigationHandle* navigation_handle) {
   auto it = ongoing_navigations_.find(navigation_handle);
   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