Move GeolocationDispatcher into blink.

third_party/WebKit/Source/modules/geolocation/Geolocation.h

Index: third_party/WebKit/Source/modules/geolocation/Geolocation.h
diff --git a/third_party/WebKit/Source/modules/geolocation/Geolocation.h b/third_party/WebKit/Source/modules/geolocation/Geolocation.h
index 6518b959b7977789051bfb3c7acc4774cf8be38b..778d137787ff013d3b93843b02a9dd1840c71024 100644
--- a/third_party/WebKit/Source/modules/geolocation/Geolocation.h
+++ b/third_party/WebKit/Source/modules/geolocation/Geolocation.h
@@ -29,6 +29,7 @@
 
 #include "bindings/core/v8/ScriptWrappable.h"
 #include "core/dom/ActiveDOMObject.h"
+#include "core/page/PageLifecycleObserver.h"
 #include "modules/ModulesExport.h"
 #include "modules/geolocation/GeoNotifier.h"
 #include "modules/geolocation/GeolocationWatchers.h"
@@ -39,6 +40,9 @@
 #include "modules/geolocation/PositionOptions.h"
 #include "platform/Timer.h"
 #include "platform/heap/Handle.h"
+#include "third_party/WebKit/public/platform/modules/geolocation/geolocation.mojom-blink.h"

[Michael van Ouwerkerk, 2016/05/04 13:52:17]

Can we omit "third_party/WebKit" here and below?

[Sam McNally, 2016/05/05 11:50:24]

No, only the root of the generated files tree is added to the include path.

+#include "third_party/WebKit/public/platform/modules/permissions/permission.mojom-blink.h"
+#include "third_party/WebKit/public/platform/modules/permissions/permission_status.mojom-blink.h"
 
 namespace blink {
 
@@ -50,7 +54,8 @@ class ExecutionContext;
 class MODULES_EXPORT Geolocation final
     : public GarbageCollectedFinalized<Geolocation>

[Michael van Ouwerkerk, 2016/05/04 13:52:17]

This class should probably use "USING_PRE_FINALIZER(Geolocation, stop);"

[Sam McNally, 2016/05/05 11:50:24]

Done.

     , public ScriptWrappable
-    , public ActiveDOMObject {
+    , public ActiveDOMObject
+    , public PageLifecycleObserver {
     DEFINE_WRAPPERTYPEINFO();
     USING_GARBAGE_COLLECTED_MIXIN(Geolocation);
 public:
@@ -73,17 +78,12 @@ public:
     // Removes all references to the watcher, it will not be updated again.
     void clearWatch(int watchID);
 
-    void setIsAllowed(bool);
-
     bool isAllowed() const { return m_geolocationPermission == PermissionAllowed; }
 
     // Notifies this that a new position is available. Must never be called
     // before permission is granted by the user.
     void positionChanged();
 
-    // Notifies this that an error has occurred, it must be handled immediately.
-    void setError(GeolocationError*);
-
     // Discards the notifier because a fatal error occurred for it.
     void fatalErrorOccurred(GeoNotifier*);
 
@@ -95,6 +95,9 @@ public:
     // Discards the notifier if it is a oneshot because it timed it.
     void requestTimedOut(GeoNotifier*);
 
+    // Inherited from PageLifecycleObserver.
+    void pageVisibilityChanged() override;
+
 private:
     // Returns the last known position, if any. May return null.
     Geoposition* lastPosition();
@@ -141,33 +144,37 @@ private:
     // Requests permission to share positions with the page.
     void requestPermission();
 
-    // Attempts to register this with the controller for receiving updates.
-    // Returns false if there is no controller to register with.
-    bool startUpdating(GeoNotifier*);
+    // Connects to the Geolocation mojo service and starts polling for updates.
+    void startUpdating(GeoNotifier*);
 
     void stopUpdating();
 
-    // Processes the notifiers that were waiting for a permission decision. If
-    // granted and this can be registered with the controller then the
-    // notifier's timers are started. Otherwise, a fatal error is set on them.
-    void handlePendingPermissionNotifiers();
+    void updateGeolocationServiceConnection();
+    void queryNextPosition();
 
     // Attempts to obtain a position for the given notifier, either by using
-    // the cached position or by requesting one from the controller. Sets a
-    // fatal error if permission is denied or no position can be obtained.
+    // the cached position or by requesting one from the GeolocationService.
+    // Sets a fatal error if permission is denied or no position can be
+    // obtained.
     void startRequest(GeoNotifier*);
 
     bool haveSuitableCachedPosition(const PositionOptions&);
 
-    // Runs the success callbacks for the set of notifiers awaiting a cached
-    // position, the set is then cleared. The oneshots are removed everywhere.
-    void makeCachedPositionCallbacks();
-
     // Record whether the origin trying to access Geolocation would be allowed
     // to access a feature that can only be accessed by secure origins.
     // See https://goo.gl/Y0ZkNV
     void recordOriginTypeAccess() const;
 
+    void onPositionUpdated(mojom::blink::GeopositionPtr);
+
+    // Processes the notifiers that were waiting for a permission decision. If
+    // granted then the notifier's timers are started. Otherwise, a fatal error
+    // is set on them.
+    void onGeolocationPermissionUpdated(mojom::blink::PermissionStatus);
+
+    void onGeolocationConnectionError();
+    void onPermissionConnectionError();
+
     GeoNotifierSet m_oneShots;
     GeolocationWatchers m_watchers;
     GeoNotifierSet m_pendingForPermissionNotifiers;
@@ -184,8 +191,12 @@ private:
     };
 
     Permission m_geolocationPermission;
+    mojom::blink::GeolocationServicePtr m_geolocationService;

[Michael van Ouwerkerk, 2016/05/04 13:52:17]

When is this destroyed? The owner is GC'd at some point, but I think we need
some hooks to shut down the mojo service in a timely manner. Perhaps in
willDetachFrameHost() from LocalFrameLifecycleObserver and didCommitLoad from
PageLifeCycleObserver?

Same question for the permission service, but that does not keep an expensive
subsystem alive in the browser process, scanning wifi access points, doing
network requests, and using GPS.

[Sam McNally, 2016/05/05 11:50:24]

A mojo InterfacePtr isn't always connected to something else. In this case,
m_geolocationService is only connected while a geolocation request is active and
the page is visible. stop() cancels any active requests too so this would be
disconnected when DOM objects are stopped. Is this sufficient?

m_permissionService is connected when a permission request is needed and
disconnected once it's completed.

+    bool m_enableHighAccuracy = false;
+    mojom::blink::PermissionServicePtr m_permissionService;
 
-    GeoNotifierSet m_requestsAwaitingCachedPosition;
+    bool m_updating = false;

[Michael van Ouwerkerk, 2016/05/04 13:52:17]

Please document the distinction between m_updating and m_queryInProgress.

[Sam McNally, 2016/05/05 11:50:24]

Done.

+    bool m_queryInProgress = false;
 };
 
 } // namespace blink