ServiceWorker: Fix the bug in navigate() if windowclient is nested frame.

content/browser/service_worker/service_worker_version.cc

Index: content/browser/service_worker/service_worker_version.cc
diff --git a/content/browser/service_worker/service_worker_version.cc b/content/browser/service_worker/service_worker_version.cc
index cf70eab83d49c1cf7d4d029491414160ee7d08f7..3183d8778ea61b320c7d781f36380d62972d7032 100644
--- a/content/browser/service_worker/service_worker_version.cc
+++ b/content/browser/service_worker/service_worker_version.cc
@@ -21,6 +21,8 @@
 #include "base/time/time.h"
 #include "content/browser/bad_message.h"
 #include "content/browser/child_process_security_policy_impl.h"
+#include "content/browser/frame_host/frame_tree_node.h"
+#include "content/browser/frame_host/render_frame_host_impl.h"
 #include "content/browser/message_port_message_filter.h"
 #include "content/browser/message_port_service.h"
 #include "content/browser/service_worker/embedded_worker_instance.h"
@@ -36,7 +38,6 @@
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/page_navigator.h"
-#include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_observer.h"
@@ -187,8 +188,12 @@ using OpenURLCallback = base::Callback<void(int, int)>;
 // The callback will be called in the IO thread.
 class OpenURLObserver : public WebContentsObserver {
  public:
-  OpenURLObserver(WebContents* web_contents, const OpenURLCallback& callback)
-      : WebContentsObserver(web_contents), callback_(callback) {}
+  OpenURLObserver(WebContents* web_contents,
+                  RenderFrameHost* render_frame_host,
+                  const OpenURLCallback& callback)
+      : WebContentsObserver(web_contents),
+        render_frame_host_(render_frame_host),
+        callback_(callback) {}
 
   void DidCommitProvisionalLoadForFrame(
       RenderFrameHost* render_frame_host,
@@ -196,7 +201,7 @@ class OpenURLObserver : public WebContentsObserver {
       ui::PageTransition transition_type) override {
     DCHECK(web_contents());
 
-    if (render_frame_host != web_contents()->GetMainFrame())
+    if (render_frame_host != render_frame_host_)

[Charlie Reis, 2015/09/17 17:14:34]

What happens if this was a cross-process navigation?  The destination
render_frame_host won't match the render_frame_host_ from the start of the
navigation.  (The same bug appears to exist in the old code.)

Seems like you should compare the FrameTreeNodes (or FrameTreeNode IDs) of the
two RenderFrameHosts instead.  You can cast them to RenderFrameHostImpls to get
that here in content, though it gets back to Avi's question about exposing a
RenderFrameHost::GetFrameID() method in the public API in general.

[zino, 2015/09/22 08:01:14]

Done.

       return;
 
     RunCallback(render_frame_host->GetProcess()->GetID(),
@@ -226,6 +231,7 @@ class OpenURLObserver : public WebContentsObserver {
     base::MessageLoop::current()->DeleteSoon(FROM_HERE, this);
   }
 
+  RenderFrameHost* render_frame_host_;
   const OpenURLCallback callback_;
 
   DISALLOW_COPY_AND_ASSIGN(OpenURLObserver);
@@ -234,7 +240,7 @@ class OpenURLObserver : public WebContentsObserver {
 void DidOpenURL(const OpenURLCallback& callback, WebContents* web_contents) {
   DCHECK(web_contents);
 
-  new OpenURLObserver(web_contents, callback);
+  new OpenURLObserver(web_contents, web_contents->GetMainFrame(), callback);
 }
 
 void NavigateClientOnUI(const GURL& url,
@@ -244,8 +250,8 @@ void NavigateClientOnUI(const GURL& url,
                         const OpenURLCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  RenderFrameHost* render_frame_host =
-      RenderFrameHost::FromID(process_id, frame_id);
+  RenderFrameHostImpl* render_frame_host =
+      RenderFrameHostImpl::FromID(process_id, frame_id);
   WebContents* web_contents =
       WebContents::FromRenderFrameHost(render_frame_host);
 
@@ -257,13 +263,17 @@ void NavigateClientOnUI(const GURL& url,
     return;
   }
 
+  ui::PageTransition transition = render_frame_host->GetParent()
+                                      ? ui::PAGE_TRANSITION_AUTO_SUBFRAME
+                                      : ui::PAGE_TRANSITION_AUTO_TOPLEVEL;
+
   OpenURLParams params(
       url, Referrer::SanitizeForRequest(
                url, Referrer(script_url, blink::WebReferrerPolicyDefault)),
-      CURRENT_TAB, ui::PAGE_TRANSITION_AUTO_TOPLEVEL,
-      true /* is_renderer_initiated */);
+      render_frame_host->frame_tree_node()->frame_tree_node_id(), CURRENT_TAB,
+      transition, true /* is_renderer_initiated */);
   web_contents->OpenURL(params);
-  DidOpenURL(callback, web_contents);
+  new OpenURLObserver(web_contents, render_frame_host, callback);
 }
 
 void OpenWindowOnUI(