[ServiceWorker] Implement WebServiceWorkerContextClient::openWindow().

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 fd2138850b6531f9350cece3dd1f83d84278caa5..2da764a5160028a8f75d4fab3538cf86421d1bc1 100644
--- a/content/browser/service_worker/service_worker_version.cc
+++ b/content/browser/service_worker/service_worker_version.cc
@@ -13,10 +13,20 @@
 #include "content/browser/service_worker/embedded_worker_instance.h"
 #include "content/browser/service_worker/embedded_worker_registry.h"
 #include "content/browser/service_worker/service_worker_context_core.h"
+#include "content/browser/service_worker/service_worker_context_wrapper.h"
 #include "content/browser/service_worker/service_worker_registration.h"
 #include "content/browser/service_worker/service_worker_utils.h"
+#include "content/browser/storage_partition_impl.h"
 #include "content/common/service_worker/service_worker_messages.h"
 #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"
+#include "content/public/common/child_process_host.h"
+#include "content/public/common/content_client.h"
 #include "content/public/common/content_switches.h"
 
 namespace content {
@@ -143,6 +153,82 @@ void RunErrorCrossOriginConnectCallback(
   callback.Run(status, false);
 }
 
+using WindowOpenedCallback = base::Callback<void(int,int)>;
+
+// The WindowOpenedObserver class is a WebContentsObserver that will wait for a
+// new Window's WebContents to be initialized, run the |callback| passed to its
+// constructor then self destroy.
+// The callback will receive the process and frame ids. If something went wrong
+// those will be (kInvalidUniqueID,MSG_ROUTING_NONE).
+class WindowOpenedObserver : public WebContentsObserver {
+ public:
+  WindowOpenedObserver(WebContents* web_contents,
+                       const WindowOpenedCallback& callback)
+    : WebContentsObserver(web_contents)
+    , callback_(callback)
+  {}
+
+  void DocumentAvailableInMainFrame() override {
+    DCHECK(web_contents());
+
+    RenderFrameHost* render_frame_host = web_contents()->GetMainFrame();
+    DCHECK(render_frame_host);
+
+    RunCallback(render_frame_host->GetProcess()->GetID(),
+                render_frame_host->GetRoutingID());
+  }
+
+  void RenderProcessGone(base::TerminationStatus status) override {
+    RunCallback(ChildProcessHost::kInvalidUniqueID, MSG_ROUTING_NONE);
+  }
+
+  void WebContentsDestroyed() override {
+    RunCallback(ChildProcessHost::kInvalidUniqueID, MSG_ROUTING_NONE);
+  }
+
+ private:
+  void RunCallback(int render_process_id, int render_frame_id) {
+    // After running the callback, |this| will stop observing, thus
+    // web_contents() should return nullptr and |RunCallback| should no longer
+    // be called. Then, |this| will self destroy.
+    DCHECK(web_contents());
+
+    BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                            base::Bind(callback_,
+                                       render_process_id,
+                                       render_frame_id));
+    Observe(nullptr);
+    base::MessageLoop::current()->DeleteSoon(FROM_HERE, this);
+  }
+
+  const WindowOpenedCallback callback_;
+};
+
+void OpenWindowOnUI(
+    const GURL& url,
+    const GURL& referrer,
+    const scoped_refptr<ServiceWorkerContextWrapper>& context_wrapper,
+    const WindowOpenedCallback& callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+  BrowserContext* browser_context = context_wrapper->storage_partition()
+      ? context_wrapper->storage_partition()->browser_context()
+      : nullptr;
+  // We are shutting down.
+  if (!browser_context)
+    return;
+
+  OpenURLParams params(url,
+                       Referrer(referrer, blink::WebReferrerPolicyDefault),
+                       NEW_FOREGROUND_TAB,
+                       ui::PAGE_TRANSITION_AUTO_TOPLEVEL,
+                       false);
+  WebContents* web_contents =
+      GetContentClient()->browser()->OpenURL(browser_context, params);

[michaeln, 2015/01/17 00:25:32]

imo, storage_partition ptr would make sense as an optional input to this method

[michaeln, 2015/01/17 00:25:32]

can this method return nullptr for a web_contents, just checking?

[mlamouri (slow - plz ping), 2015/01/26 13:19:08]

Let's discuss about that in the CL related to ContentBrowserClient::OpenURL.
No, unless it's not supported (Android/iOS). I've added a DCHECK() to make that
more clear.

+
+  new WindowOpenedObserver(web_contents, callback);
+}
+
 }  // namespace
 
 ServiceWorkerVersion::ServiceWorkerVersion(
@@ -788,6 +874,8 @@ bool ServiceWorkerVersion::OnMessageReceived(const IPC::Message& message) {
                         OnGeofencingEventFinished)
     IPC_MESSAGE_HANDLER(ServiceWorkerHostMsg_CrossOriginConnectEventFinished,
                         OnCrossOriginConnectEventFinished)
+    IPC_MESSAGE_HANDLER(ServiceWorkerHostMsg_OpenWindow,
+                        OnOpenWindow)
     IPC_MESSAGE_HANDLER(ServiceWorkerHostMsg_PostMessageToDocument,
                         OnPostMessageToDocument)
     IPC_MESSAGE_HANDLER(ServiceWorkerHostMsg_FocusClient,
@@ -1038,6 +1126,85 @@ void ServiceWorkerVersion::OnCrossOriginConnectEventFinished(
   cross_origin_connect_callbacks_.Remove(request_id);
 }
 
+void ServiceWorkerVersion::OnOpenWindow(
+    int request_id, const GURL& url, const GURL& referrer) {
+  // Just abort if we are shutting down.
+  if (!context_)
+    return;
+  scoped_refptr<ServiceWorkerContextWrapper> context_wrapper(
+      context_->wrapper());
+
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&OpenWindowOnUI,
+                 url,
+                 referrer,
+                 context_wrapper,

[michaeln, 2015/01/17 00:25:32]

nit: make_scoped_refptr(context_->wrapper()) might be more readable

[mlamouri (slow - plz ping), 2015/01/26 13:19:08]

Done.

+                 base::Bind(&ServiceWorkerVersion::DidOpenWindow,
+                            weak_factory_.GetWeakPtr(),
+                            request_id)));
+}
+
+void ServiceWorkerVersion::DidOpenWindow(int request_id,
+                                         int render_process_id,
+                                         int render_frame_id) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  if (running_status() != RUNNING)
+    return;
+
+  if (render_process_id == ChildProcessHost::kInvalidUniqueID &&

[nasko, 2015/01/17 00:05:28]

RenderProcessHost is what hosts Blink renderers. ChildProcessHost is for all
other types of processes. While this statement might be ok in practice, it feels
strange to mix up the two types.

[mlamouri (slow - plz ping), 2015/01/26 13:19:08]

My understanding was that a RenderProcess was a type of ChildProcess. Anyhow,
RPH::GetID() says:
"// This will never return ChildProcessHost::kInvalidUniqueID."
so I guess checking against that is the right move, isn't?

+      render_frame_id == MSG_ROUTING_NONE) {
+    embedded_worker_->SendMessage(ServiceWorkerMsg_OpenWindowError(request_id));
+    return;
+  }
+
+  for (const auto& it : controllee_map_) {
+    const ServiceWorkerProviderHost* provider_host = it.first;
+    if (provider_host->process_id() != render_process_id ||
+        provider_host->frame_id() != render_frame_id) {
+      continue;
+    }
+
+    int client_request_id = get_client_info_callbacks_.Add(
+        new GetClientInfoCallback(base::Bind(
+            &ServiceWorkerVersion::OnOpenWindowFinished,
+            weak_factory_.GetWeakPtr(), request_id)));
+    provider_host->GetClientInfo(embedded_worker_->embedded_worker_id(),
+                                 client_request_id);
+    return;
+  }
+
+  // If here, it means that no provider_host was found, in which case, the
+  // renderer should still be informed that the window was opened.
+  OnOpenWindowFinished(request_id,
+                       SERVICE_WORKER_ERROR_FAILED,
+                       ServiceWorkerClientInfo());
+}
+
+void ServiceWorkerVersion::OnOpenWindowFinished(
+    int request_id,
+    ServiceWorkerStatusCode status,
+    const ServiceWorkerClientInfo& client_info) {

[michaeln, 2015/01/17 00:25:32]

how is the client_info.client_id field filled in, looks like the renderer always
returns 0 for that value?

also can you please add a ctor to the ServiceWorkerClientInfo struct?

[mlamouri (slow - plz ping), 2015/01/26 13:19:08]

Fixed.
Done as part of https://codereview.chromium.org/871013003

+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  if (running_status() != RUNNING)
+    return;
+
+  // If the status is an error, it means that it wasn't possible to find the
+  // client but the window was correctly opened. In that case, the renderer is
+  // expecting to receive the |client_info| with the boolean (|dummy_client|)
+  // set to true.
+  if (status != SERVICE_WORKER_OK) {
+    embedded_worker_->SendMessage(ServiceWorkerMsg_OpenWindowResponse(
+      request_id, client_info, true));
+    return;
+  }
+
+  embedded_worker_->SendMessage(ServiceWorkerMsg_OpenWindowResponse(
+      request_id, client_info, false));
+}
+
 void ServiceWorkerVersion::OnPostMessageToDocument(
     int client_id,
     const base::string16& message,