Convert ExtensionWebContentsObserver to use the new navigation callbacks.

extensions/browser/extension_web_contents_observer.cc

Index: extensions/browser/extension_web_contents_observer.cc
diff --git a/extensions/browser/extension_web_contents_observer.cc b/extensions/browser/extension_web_contents_observer.cc
index a4309b0a7a4941f8e0f144335fdbef3b0d2ffc89..73527607b429720e31488915bfdea54ea5400bfe 100644
--- a/extensions/browser/extension_web_contents_observer.cc
+++ b/extensions/browser/extension_web_contents_observer.cc
@@ -5,7 +5,6 @@
 #include "extensions/browser/extension_web_contents_observer.h"
 
 #include "content/public/browser/child_process_security_policy.h"
-#include "content/public/browser/navigation_details.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
@@ -145,38 +144,19 @@ void ExtensionWebContentsObserver::DidFinishNavigation(
 
   content::RenderFrameHost* render_frame_host =
       navigation_handle->GetRenderFrameHost();
+  const Extension* frame_extension =
+      GetExtensionFromFrame(render_frame_host, true);
   if (pm->IsRenderFrameHostRegistered(render_frame_host)) {
-    const Extension* frame_extension =
-        GetExtensionFromFrame(render_frame_host, true);
+    if (!navigation_handle->IsSamePage()) {
+      // Notify ProcessManager, because some clients do not just want to know
+      // whether the frame is in an extension process, but also whether the
+      // frame was navigated.
+      pm->DidNavigateRenderFrameHost(render_frame_host);

[Devlin, 2017/02/06 17:15:27]

This should only be called if |frame_extension| is valid, right?  (The original
code would early-exit on line 171)

[jam, 2017/02/06 18:03:38]

Sure. I didn't know if it was possible to reach here with a null frame_extension
in practice, so I figured I'd send a cl to see (as opposed to the section below,
where we do need it). Added a check.

+    }
 
     if (!frame_extension)
       pm->UnregisterRenderFrameHost(render_frame_host);
-  }
-}
-
-void ExtensionWebContentsObserver::DidNavigateAnyFrame(
-    content::RenderFrameHost* render_frame_host,
-    const content::LoadCommittedDetails& details,
-    const content::FrameNavigateParams& params) {
-  if (details.is_in_page)
-    return;
-
-  const Extension* frame_extension =
-      GetExtensionFromFrame(render_frame_host, true);
-  ProcessManager* pm = ProcessManager::Get(browser_context_);
-
-  if (!frame_extension) {
-    // Should have been unregistered by DidFinishNavigation.
-    DCHECK(!pm->IsRenderFrameHostRegistered(render_frame_host));
-    return;
-  }
-
-  if (pm->IsRenderFrameHostRegistered(render_frame_host)) {
-    // Notify ProcessManager, because some clients do not just want to know
-    // whether the frame is in an extension process, but also whether the frame
-    // was navigated.
-    pm->DidNavigateRenderFrameHost(render_frame_host);
-  } else {
+  } else if (!navigation_handle->IsSamePage() && frame_extension) {

[Devlin, 2017/02/06 17:15:27]

Hmm... this is a match with previous behavior, but the fact that we
unconditionally unregister the frame but conditionally register it is a little
weird.  My guess is that the details.is_in_page check before (in
DidNavigateAnyFrame) was because that's cheaper than GetExtensionFromFrame() and
IsRenderFrameHostRegistered().  If the frame isn't registered at this point, it
probably should be.

I'd suggest we take out the IsSamePage() check here, but if you wanted to
summarize it in a TODO(devlin), we can address it in a followup.

[jam, 2017/02/06 18:03:38]

Done.

     pm->RegisterRenderFrameHost(web_contents(), render_frame_host,
                                 frame_extension);
   }