Convert TestFrameNavigationObserver to use the new navigation callbacks.

content/public/test/test_frame_navigation_observer.cc

Index: content/public/test/test_frame_navigation_observer.cc
diff --git a/content/public/test/test_frame_navigation_observer.cc b/content/public/test/test_frame_navigation_observer.cc
index 46f1906221118a97101fc6c4823083e604151c43..933ae611ecaaadab38b3448f615b7e9846a3f43f 100644
--- a/content/public/test/test_frame_navigation_observer.cc
+++ b/content/public/test/test_frame_navigation_observer.cc
@@ -4,16 +4,11 @@
 
 #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 {
 
@@ -54,29 +49,25 @@ void TestFrameNavigationObserver::WaitForCommit() {
   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_)