Chrome side of passing on the service worker registration with geofencing API calls.

content/browser/geofencing/geofencing_dispatcher_host.cc

Index: content/browser/geofencing/geofencing_dispatcher_host.cc
diff --git a/content/browser/geofencing/geofencing_dispatcher_host.cc b/content/browser/geofencing/geofencing_dispatcher_host.cc
index 077485b38f6a31407c7dc2400408c09bce9bcd31..49836e0bd8758521388d27ba5b74b6f5eb3fe417 100644
--- a/content/browser/geofencing/geofencing_dispatcher_host.cc
+++ b/content/browser/geofencing/geofencing_dispatcher_host.cc
@@ -5,6 +5,9 @@
 #include "content/browser/geofencing/geofencing_dispatcher_host.h"
 
 #include "content/browser/geofencing/geofencing_manager.h"
+#include "content/browser/service_worker/service_worker_dispatcher_host.h"
+#include "content/browser/service_worker/service_worker_registration.h"
+#include "content/browser/service_worker/service_worker_registration_handle.h"
 #include "content/common/geofencing_messages.h"
 #include "third_party/WebKit/public/platform/WebCircularGeofencingRegion.h"
 #include "url/gurl.h"
@@ -14,9 +17,11 @@ namespace content {
 static const int kMaxRegionIdLength = 200;
 
 GeofencingDispatcherHost::GeofencingDispatcherHost(
-    BrowserContext* browser_context)
+    BrowserContext* browser_context,
+    scoped_refptr<ServiceWorkerDispatcherHost> service_worker_filter)
     : BrowserMessageFilter(GeofencingMsgStart),
       browser_context_(browser_context),
+      service_worker_filter_(service_worker_filter),
       weak_factory_(this) {
 }
 
@@ -39,18 +44,29 @@ void GeofencingDispatcherHost::OnRegisterRegion(
     int thread_id,
     int request_id,
     const std::string& region_id,
-    const blink::WebCircularGeofencingRegion& region) {
+    const blink::WebCircularGeofencingRegion& region,
+    int service_worker_registration_handle_id) {
   // Sanity check on region_id
   if (region_id.length() > kMaxRegionIdLength) {
     Send(new GeofencingMsg_RegisterRegionComplete(
         thread_id, request_id, GeofencingStatus::GEOFENCING_STATUS_ERROR));
     return;
   }
-  // TODO(mek): Actually pass service worker information to manager.
+  // Look up service worker.
+  ServiceWorkerRegistrationHandle* service_worker_handle =
+      service_worker_filter_->GetRegistrationHandle(

[Michael van Ouwerkerk, 2014/10/09 12:28:09]

I think GetRegistration(Handle) should be exposed on ServiceWorkerContextCore or
we should go directly to ServiceWorkerStorage.

[Marijn Kruisselbrink, 2014/10/09 22:28:58]

The problem is that handle IDs currently only exists in the
ServiceWorkerDispatcherHost. These IDs are per-renderer process, and
ServiceWorkerRegistrationHandle instances are tied to a specific renderer
process. But yes, I agree that it would be great to have some API that doesn't
require one message filter directly accessing another filter. One way to maybe
make this less icky would be for ServiceWorkerDispatcherHost to implement a new
"ServiceWorkerRenderHostContext" interface (or something with a better name),
which would expose this GetRegistrationHandle method to get to the handle from
the handle id for a specific renderer.

+          service_worker_registration_handle_id);
+  ServiceWorkerRegistration* service_worker =

[Michael van Ouwerkerk, 2014/10/09 12:28:09]

Let's be clear and call the variable service_worker_registration.

[Marijn Kruisselbrink, 2014/10/13 19:29:48]

Done.

+      service_worker_handle ? service_worker_handle->registration() : nullptr;
+  int64 service_worker_registration_id =
+      service_worker ? service_worker->id() : -1;
+  GURL service_worker_origin =
+      service_worker ? service_worker->pattern().GetOrigin() : GURL();
+
   GeofencingManager::GetInstance()->RegisterRegion(
       browser_context_,
-      0,      /* service_worker_registration_id */
-      GURL(), /* service_worker_origin */
+      service_worker_registration_id,
+      service_worker_origin,
       region_id,
       region,
       base::Bind(&GeofencingDispatcherHost::RegisterRegionCompleted,
@@ -62,18 +78,29 @@ void GeofencingDispatcherHost::OnRegisterRegion(
 void GeofencingDispatcherHost::OnUnregisterRegion(
     int thread_id,
     int request_id,
-    const std::string& region_id) {
+    const std::string& region_id,
+    int service_worker_registration_handle_id) {
   // Sanity check on region_id
   if (region_id.length() > kMaxRegionIdLength) {
     Send(new GeofencingMsg_UnregisterRegionComplete(
         thread_id, request_id, GeofencingStatus::GEOFENCING_STATUS_ERROR));
     return;
   }
-  // TODO(mek): Actually pass service worker information to manager.
+  // Look up service worker.
+  ServiceWorkerRegistrationHandle* service_worker_handle =
+      service_worker_filter_->GetRegistrationHandle(
+          service_worker_registration_handle_id);
+  ServiceWorkerRegistration* service_worker =
+      service_worker_handle ? service_worker_handle->registration() : nullptr;
+  int64 service_worker_registration_id =
+      service_worker ? service_worker->id() : -1;
+  GURL service_worker_origin =
+      service_worker ? service_worker->pattern().GetOrigin() : GURL();
+
   GeofencingManager::GetInstance()->UnregisterRegion(
       browser_context_,
-      0,      /* service_worker_registration_id */
-      GURL(), /* service_worker_origin */
+      service_worker_registration_id,
+      service_worker_origin,
       region_id,
       base::Bind(&GeofencingDispatcherHost::UnregisterRegionCompleted,
                  weak_factory_.GetWeakPtr(),
@@ -81,15 +108,27 @@ void GeofencingDispatcherHost::OnUnregisterRegion(
                  request_id));
 }
 
-void GeofencingDispatcherHost::OnGetRegisteredRegions(int thread_id,
-                                                      int request_id) {
+void GeofencingDispatcherHost::OnGetRegisteredRegions(
+    int thread_id,
+    int request_id,
+    int service_worker_registration_handle_id) {
+  // Look up service worker.
+  ServiceWorkerRegistrationHandle* service_worker_handle =
+      service_worker_filter_->GetRegistrationHandle(
+          service_worker_registration_handle_id);
+  ServiceWorkerRegistration* service_worker =
+      service_worker_handle ? service_worker_handle->registration() : nullptr;
+  int64 service_worker_registration_id =
+      service_worker ? service_worker->id() : -1;

[jsbell, 2014/10/08 18:41:29]

Can you make this an enum value in the GeofencingManager rather than a magic
number?

(I see it's documented in a comment in
https://codereview.chromium.org/586163003/)

[Marijn Kruisselbrink, 2014/10/13 19:29:48]

I changed the code to just never call GeofencingManager if we don't have a valid
service worker registration.

+  GURL service_worker_origin =
+      service_worker ? service_worker->pattern().GetOrigin() : GURL();
+
   GeofencingRegistrations result;
-  // TODO(mek): Actually pass service worker information to manager.
   GeofencingStatus status =
       GeofencingManager::GetInstance()->GetRegisteredRegions(
           browser_context_,
-          0,      /* service_worker_registration_id */
-          GURL(), /* service_worker_origin */
+          service_worker_registration_id,
+          service_worker_origin,
           &result);
   Send(new GeofencingMsg_GetRegisteredRegionsComplete(
       thread_id, request_id, status, result));