Convert TestFrameNavigationObserver to use the new navigation callbacks.

content/public/test/test_frame_navigation_observer.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/public/test/test_frame_navigation_observer.h"
 
-#include "base/bind.h"
-#include "base/message_loop/message_loop.h"
-#include "base/run_loop.h"
-#include "base/stl_util.h"
 #include "content/browser/frame_host/navigation_entry_impl.h"
 #include "content/browser/frame_host/render_frame_host_impl.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
 #include "content/browser/web_contents/web_contents_impl.h"
-#include "content/public/browser/web_contents_observer.h"
-#include "testing/gtest/include/gtest/gtest.h"
+#include "content/public/browser/navigation_handle.h"
 
 namespace content {
 
 namespace {
 
 RenderFrameHostImpl* ToRenderFrameHostImpl(const ToRenderFrameHost& frame) {
   return static_cast<RenderFrameHostImpl*>(frame.render_frame_host());
 }
 
 }  // namespace
@@ skipping 20 matching lines @@
 void TestFrameNavigationObserver::WaitForCommit() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   if (has_committed_)
     return;
 
   wait_for_commit_ = true;
   run_loop_.Run();
 }
 
-void TestFrameNavigationObserver::DidStartProvisionalLoadForFrame(
-    RenderFrameHost* render_frame_host,
-    const GURL& validated_url,
-    bool is_error_page) {
-  RenderFrameHostImpl* rfh =
-      static_cast<RenderFrameHostImpl*>(render_frame_host);
-  if (rfh->frame_tree_node()->frame_tree_node_id() == frame_tree_node_id_) {
+void TestFrameNavigationObserver::DidStartNavigation(
+    NavigationHandle* navigation_handle) {
+  if (!navigation_handle->IsSamePage() &&
+      navigation_handle->GetFrameTreeNodeId() == frame_tree_node_id_) {
     navigation_started_ = true;
     has_committed_ = false;
   }
 }
 
-void TestFrameNavigationObserver::DidCommitProvisionalLoadForFrame(
-    RenderFrameHost* render_frame_host,
-    const GURL& url,
-    ui::PageTransition transition_type) {
+void TestFrameNavigationObserver::DidFinishNavigation(
+    NavigationHandle* navigation_handle) {
   if (!navigation_started_)
     return;
 
-  RenderFrameHostImpl* rfh =
-      static_cast<RenderFrameHostImpl*>(render_frame_host);
-  if (rfh->frame_tree_node()->frame_tree_node_id() != frame_tree_node_id_)
+  if (!navigation_handle->HasCommitted() ||
+      navigation_handle->IsErrorPage() ||

[nasko, 2017/01/26 14:51:20]

I think DidCommitProvisionalLoadForFrame is getting called when we commit the
error page, so that condition might not be necessary. I have to double check to
be sure though.

[jam, 2017/01/26 16:04:12]

Yeah we get a didfailprovisionalload for the URL and then
didcommitprovisionalload for the error page. I figured all the users of this
class are waiting for successful commits, which we can see from the try run
results. So it's a tradeoff between maintaining the exact behavior as before,
which may just be a side effect of the previous navigation callbacks, vs waiting
only for successful commits (which the comments in the header seem to imply).

i don't feel strongly one way or another, but lean slightly towards the first
patchset. I uploaded the one that waits for success or error pages in patchset
2. do you have a preference?

+      navigation_handle->GetFrameTreeNodeId() != frame_tree_node_id_) {
     return;
+  }
 
   has_committed_ = true;
   if (wait_for_commit_)
     run_loop_.Quit();
 }
 
 void TestFrameNavigationObserver::DidStopLoading() {
   if (!navigation_started_)
     return;
 
   navigation_started_ = false;
   run_loop_.Quit();
 }
 
 }  // namespace content