Move GeolocationDispatcher into blink.

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

Index: third_party/WebKit/Source/modules/geolocation/Geolocation.cpp
diff --git a/third_party/WebKit/Source/modules/geolocation/Geolocation.cpp b/third_party/WebKit/Source/modules/geolocation/Geolocation.cpp
index 036b29e820c6a6426612f9dea719ddb00a1122c2..b669b724ea8e8076875f3a9dc7b4c748709662f9 100644
--- a/third_party/WebKit/Source/modules/geolocation/Geolocation.cpp
+++ b/third_party/WebKit/Source/modules/geolocation/Geolocation.cpp
@@ -32,53 +32,53 @@
 #include "core/frame/OriginsUsingFeatures.h"
 #include "core/frame/Settings.h"
 #include "modules/geolocation/Coordinates.h"
-#include "modules/geolocation/GeolocationController.h"
 #include "modules/geolocation/GeolocationError.h"
-#include "modules/geolocation/GeolocationPosition.h"
-#include "platform/weborigin/SecurityOrigin.h"
+#include "platform/mojo/MojoHelper.h"
+#include "public/platform/ServiceRegistry.h"
 #include "wtf/CurrentTime.h"
 
 namespace blink {
+namespace {
 
 static const char permissionDeniedErrorMessage[] = "User denied Geolocation";
 static const char failedToStartServiceErrorMessage[] = "Failed to start Geolocation service";
 static const char framelessDocumentErrorMessage[] = "Geolocation cannot be used in frameless documents";
 
-static Geoposition* createGeoposition(GeolocationPosition* position)
+static Geoposition* createGeoposition(const mojom::blink::Geoposition& position)
 {
-    if (!position)
-        return nullptr;
-
     Coordinates* coordinates = Coordinates::create(
-        position->latitude(),
-        position->longitude(),
-        position->canProvideAltitude(),
-        position->altitude(),
-        position->accuracy(),
-        position->canProvideAltitudeAccuracy(),
-        position->altitudeAccuracy(),
-        position->canProvideHeading(),
-        position->heading(),
-        position->canProvideSpeed(),
-        position->speed());
-    return Geoposition::create(coordinates, convertSecondsToDOMTimeStamp(position->timestamp()));
-}
-
-static PositionError* createPositionError(GeolocationError* error)
-{
-    PositionError::ErrorCode code = PositionError::POSITION_UNAVAILABLE;
-    switch (error->code()) {
-    case GeolocationError::PermissionDenied:
-        code = PositionError::PERMISSION_DENIED;
+        position.latitude,
+        position.longitude,
+        position.altitude > -10000.,
+        position.altitude,
+        position.accuracy,
+        position.altitude_accuracy >= 0.,
+        position.altitude_accuracy,
+        position.heading >= 0. && position.heading <= 360.,
+        position.heading,
+        position.speed >= 0.,
+        position.speed);
+    return Geoposition::create(coordinates, convertSecondsToDOMTimeStamp(position.timestamp));
+}
+
+static PositionError* createPositionError(mojom::blink::Geoposition::ErrorCode mojomErrorCode, const String& error)
+{
+    PositionError::ErrorCode errorCode = PositionError::POSITION_UNAVAILABLE;
+    switch (mojomErrorCode) {
+    case mojom::blink::Geoposition::ErrorCode::PERMISSION_DENIED:
+        errorCode = PositionError::PERMISSION_DENIED;
         break;
-    case GeolocationError::PositionUnavailable:
-        code = PositionError::POSITION_UNAVAILABLE;
+    case mojom::blink::Geoposition::ErrorCode::POSITION_UNAVAILABLE:
+        errorCode = PositionError::POSITION_UNAVAILABLE;
         break;
+    default:

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

Doesn't clang fail the build when a switch fails to handle all enum values? The
default clause should probably be removed.

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

Done.

+        ASSERT_NOT_REACHED();
     }
-
-    return PositionError::create(code, error->message());
+    return PositionError::create(errorCode, error);
 }
 
+} // namespace
+
 Geolocation* Geolocation::create(ExecutionContext* context)
 {
     Geolocation* geolocation = new Geolocation(context);
@@ -88,6 +88,7 @@ Geolocation* Geolocation::create(ExecutionContext* context)
 
 Geolocation::Geolocation(ExecutionContext* context)
     : ActiveDOMObject(context)
+    , PageLifecycleObserver(document()->page())
     , m_geolocationPermission(PermissionUnknown)
 {
 }
@@ -103,8 +104,8 @@ DEFINE_TRACE(Geolocation)
     visitor->trace(m_watchers);
     visitor->trace(m_pendingForPermissionNotifiers);
     visitor->trace(m_lastPosition);
-    visitor->trace(m_requestsAwaitingCachedPosition);
     ActiveDOMObject::trace(visitor);
+    PageLifecycleObserver::trace(visitor);
 }
 
 Document* Geolocation::document() const
@@ -119,25 +120,19 @@ LocalFrame* Geolocation::frame() const
 
 void Geolocation::stop()
 {
-    LocalFrame* frame = this->frame();
-    if (frame && m_geolocationPermission == PermissionRequested)
-        GeolocationController::from(frame)->cancelPermissionRequest(this);
+    if (m_permissionService)

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

Why not also stop m_geolocationService?

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

stopUpdating() stops it.

+        m_permissionService.reset();
 
     // The frame may be moving to a new page and we want to get the permissions from the new page's client.

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

Please update this comment, there is no client anymore.

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

Done.

     m_geolocationPermission = PermissionUnknown;
     cancelAllRequests();
     stopUpdating();
     m_pendingForPermissionNotifiers.clear();
+    m_lastPosition = nullptr;
 }
 
 Geoposition* Geolocation::lastPosition()

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

This can be in the header now, or removed entirely.

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

Done.

 {
-    LocalFrame* frame = this->frame();
-    if (!frame)
-        return 0;
-
-    m_lastPosition = createGeoposition(GeolocationController::from(frame)->lastPosition());
-
     return m_lastPosition;
 }
 
@@ -202,7 +197,7 @@ void Geolocation::startRequest(GeoNotifier *notifier)
     // the permission state can not change again in the lifetime of this page.
     if (isDenied())
         notifier->setFatalError(PositionError::create(PositionError::PERMISSION_DENIED, permissionDeniedErrorMessage));
-    else if (haveSuitableCachedPosition(notifier->options()))
+    else if (isAllowed() && haveSuitableCachedPosition(notifier->options()))

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

Why add the isAllowed call? Is it for readability? Because
haveSuitableCachedPosition should not return true if isAllowed returns false.

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

It was for readability. Removed it and added a DCHECK in
haveSuitableCachedPosition instead.

         notifier->setUseCachedPosition();
     else if (!notifier->options().timeout())
         notifier->startTimer();
@@ -210,10 +205,10 @@ void Geolocation::startRequest(GeoNotifier *notifier)
         // if we don't yet have permission, request for permission before calling startUpdating()
         m_pendingForPermissionNotifiers.add(notifier);
         requestPermission();
-    } else if (startUpdating(notifier))
+    } else {
+        startUpdating(notifier);
         notifier->startTimer();
-    else
-        notifier->setFatalError(PositionError::create(PositionError::POSITION_UNAVAILABLE, failedToStartServiceErrorMessage));
+    }
 }
 
 void Geolocation::fatalErrorOccurred(GeoNotifier* notifier)
@@ -228,47 +223,20 @@ void Geolocation::fatalErrorOccurred(GeoNotifier* notifier)
 
 void Geolocation::requestUsesCachedPosition(GeoNotifier* notifier)
 {
-    // This is called asynchronously, so the permissions could have been denied
-    // since we last checked in startRequest.
-    if (isDenied()) {
-        notifier->setFatalError(PositionError::create(PositionError::PERMISSION_DENIED, permissionDeniedErrorMessage));
-        return;
-    }
-
-    m_requestsAwaitingCachedPosition.add(notifier);
+    DCHECK(isAllowed());

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

This DCHECK is not equivalent to the isDenied() branch it replaces, which sets
an error. So in prod builds we would now fail to set the error.

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

This should never be reached unless permission has already been allowed.
Previously, GeolocationController maintained a cached last position that could
be reused by a new Geolocation for a particular frame. Now, Geolocation
communicates directly with the service in the browser so nothing is shared
between different documents.

 
-    // If permissions are allowed, make the callback
-    if (isAllowed()) {
-        makeCachedPositionCallbacks();
-        return;
-    }
-
-    // Request permissions, which may be synchronous or asynchronous.
-    requestPermission();
-}
+    notifier->runSuccessCallback(lastPosition());
 
-void Geolocation::makeCachedPositionCallbacks()
-{
-    // All modifications to m_requestsAwaitingCachedPosition are done
-    // asynchronously, so we don't need to worry about it being modified from
-    // the callbacks.
-    for (GeoNotifier* notifier : m_requestsAwaitingCachedPosition) {
-        notifier->runSuccessCallback(lastPosition());
-
-        // If this is a one-shot request, stop it. Otherwise, if the watch still
-        // exists, start the service to get updates.
-        if (m_oneShots.contains(notifier))
-            m_oneShots.remove(notifier);
-        else if (m_watchers.contains(notifier)) {
-            if (!notifier->options().timeout() || startUpdating(notifier))
-                notifier->startTimer();
-            else
-                notifier->setFatalError(PositionError::create(PositionError::POSITION_UNAVAILABLE, failedToStartServiceErrorMessage));
-        }
+    // If this is a one-shot request, stop it. Otherwise, if the watch still
+    // exists, start the service to get updates.
+    if (m_oneShots.contains(notifier)) {
+        m_oneShots.remove(notifier);
+    } else if (m_watchers.contains(notifier)) {
+        if (notifier->options().timeout())
+            startUpdating(notifier);
+        notifier->startTimer();
     }
 
-    m_requestsAwaitingCachedPosition.clear();
-
     if (!hasListeners())
         stopUpdating();
 }
@@ -306,33 +274,25 @@ void Geolocation::clearWatch(int watchID)
         stopUpdating();
 }
 
-void Geolocation::setIsAllowed(bool allowed)
+void Geolocation::onGeolocationPermissionUpdated(mojom::blink::PermissionStatus status)
 {
     // This may be due to either a new position from the service, or a cached position.
-    m_geolocationPermission = allowed ? PermissionAllowed : PermissionDenied;
+    m_geolocationPermission = status == mojom::blink::PermissionStatus::GRANTED ? PermissionAllowed : PermissionDenied;
+    m_permissionService.reset();
 
-    // Permission request was made during the startRequest process
-    if (!m_pendingForPermissionNotifiers.isEmpty()) {
-        handlePendingPermissionNotifiers();
-        m_pendingForPermissionNotifiers.clear();
-        return;
-    }
-
-    if (!isAllowed()) {
-        PositionError* error = PositionError::create(PositionError::PERMISSION_DENIED, permissionDeniedErrorMessage);
-        error->setIsFatal(true);
-        handleError(error);
-        m_requestsAwaitingCachedPosition.clear();
-        return;
+    // While we iterate through the list, we need not worry about list being modified as the permission

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

s/list/the list/

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

Done.

+    // is already set to Yes/No and no new listeners will be added to the pending list.
+    for (GeoNotifier* notifier : m_pendingForPermissionNotifiers) {
+        if (isAllowed()) {
+            // start all pending notification requests as permission granted.

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

nit: s/start/Start/

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

Done.

+            // The notifier is always ref'ed by m_oneShots or m_watchers.
+            startUpdating(notifier);
+            notifier->startTimer();
+        } else {
+            notifier->setFatalError(PositionError::create(PositionError::PERMISSION_DENIED, permissionDeniedErrorMessage));
+        }
     }
-
-    // If the service has a last position, use it to call back for all requests.
-    // If any of the requests are waiting for permission for a cached position,
-    // the position from the service will be at least as fresh.
-    if (lastPosition())
-        makeSuccessCallbacks();
-    else
-        makeCachedPositionCallbacks();
+    m_pendingForPermissionNotifiers.clear();
 }
 
 void Geolocation::sendError(GeoNotifierVector& notifiers, PositionError* error)
@@ -455,9 +415,15 @@ void Geolocation::requestPermission()
         return;
 
     m_geolocationPermission = PermissionRequested;
+    frame->serviceRegistry()->connectToRemoteService(
+        mojo::GetProxy(&m_permissionService));
+    m_permissionService.set_connection_error_handler(sameThreadBindForMojo(&Geolocation::onPermissionConnectionError, this));

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

Under what circumstances does this get called?

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

Normally, this would only happen if the browser doesn't provide a permission
service.

 
     // Ask the embedder: it maintains the geolocation challenge policy itself.
-    GeolocationController::from(frame)->requestPermission(this);
+    m_permissionService->RequestPermission(
+        mojom::blink::PermissionName::GEOLOCATION,
+        getExecutionContext()->getSecurityOrigin()->toString(),
+        sameThreadBindForMojo(&Geolocation::onGeolocationPermissionUpdated, this));
 }
 
 void Geolocation::makeSuccessCallbacks()
@@ -476,11 +442,6 @@ void Geolocation::makeSuccessCallbacks()
     // further callbacks to these notifiers.
     m_oneShots.clear();
 
-    // Also clear the set of notifiers waiting for a cached position. All the
-    // oneshots and watchers will receive a position now, and if they happen to
-    // be lingering in that set, avoid this bug: http://crbug.com/311876 .
-    m_requestsAwaitingCachedPosition.clear();
-
     sendPosition(oneShotsCopy, lastPosition());
     sendPosition(watchersCopy, lastPosition());
 
@@ -498,46 +459,81 @@ void Geolocation::positionChanged()
     makeSuccessCallbacks();
 }
 
-void Geolocation::setError(GeolocationError* error)
+void Geolocation::startUpdating(GeoNotifier* notifier)
 {
-    handleError(createPositionError(error));
+    // This may be called multiple times, though stopUpdating() is called only
+    // once.
+    m_updating = true;
+    if (notifier->options().enableHighAccuracy() && !m_enableHighAccuracy) {
+        m_enableHighAccuracy = true;
+        if (m_geolocationService)
+            m_geolocationService->SetHighAccuracy(true);
+    }
+    updateGeolocationServiceConnection();
 }
 
-bool Geolocation::startUpdating(GeoNotifier* notifier)
+void Geolocation::stopUpdating()
 {
-    LocalFrame* frame = this->frame();
-    if (!frame)
-        return false;
+    m_updating = false;
+    updateGeolocationServiceConnection();
+    m_enableHighAccuracy = false;
+}
 
-    GeolocationController::from(frame)->addObserver(this, notifier->options().enableHighAccuracy());
-    return true;
+void Geolocation::updateGeolocationServiceConnection()
+{
+    if (!page() || page()->visibilityState() != PageVisibilityStateVisible || !m_updating) {

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

nit: use page()->isPageVisible()

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

Done.

+        m_geolocationService.reset();
+        m_queryInProgress = false;
+        return;
+    }
+    if (m_geolocationService)
+        return;
+
+    frame()->serviceRegistry()->connectToRemoteService(mojo::GetProxy(&m_geolocationService));
+    m_geolocationService.set_connection_error_handler(sameThreadBindForMojo(&Geolocation::onGeolocationConnectionError, this));
+    if (m_enableHighAccuracy)
+        m_geolocationService->SetHighAccuracy(true);
+    queryNextPosition();
 }
 
-void Geolocation::stopUpdating()
+void Geolocation::queryNextPosition()
 {
-    LocalFrame* frame = this->frame();
-    if (!frame)
+    if (m_queryInProgress)
         return;
 
-    GeolocationController::from(frame)->removeObserver(this);
+    m_queryInProgress = true;
+    m_geolocationService->QueryNextPosition(
+        sameThreadBindForMojo(&Geolocation::onPositionUpdated, this));
 }
 
-void Geolocation::handlePendingPermissionNotifiers()
+void Geolocation::onPositionUpdated(mojom::blink::GeopositionPtr position)
 {
-    // While we iterate through the list, we need not worry about list being modified as the permission
-    // is already set to Yes/No and no new listeners will be added to the pending list.
-    for (GeoNotifier* notifier : m_pendingForPermissionNotifiers) {
-        if (isAllowed()) {
-            // start all pending notification requests as permission granted.
-            // The notifier is always ref'ed by m_oneShots or m_watchers.
-            if (startUpdating(notifier))
-                notifier->startTimer();
-            else
-                notifier->setFatalError(PositionError::create(PositionError::POSITION_UNAVAILABLE, failedToStartServiceErrorMessage));
-        } else {
-            notifier->setFatalError(PositionError::create(PositionError::PERMISSION_DENIED, permissionDeniedErrorMessage));
-        }
+    m_queryInProgress = false;
+    if (position->valid) {
+        m_lastPosition = createGeoposition(*position);
+        positionChanged();
+    } else {
+        handleError(createPositionError(position->error_code, position->error_message));
     }
+    if (m_geolocationService)
+        queryNextPosition();
+}
+
+void Geolocation::pageVisibilityChanged()
+{
+    updateGeolocationServiceConnection();
+}
+
+void Geolocation::onGeolocationConnectionError()
+{
+    PositionError* error = PositionError::create(PositionError::POSITION_UNAVAILABLE, failedToStartServiceErrorMessage);
+    error->setIsFatal(true);
+    handleError(error);
+}
+
+void Geolocation::onPermissionConnectionError()
+{
+    onGeolocationPermissionUpdated(mojom::blink::PermissionStatus::DENIED);
 }
 
 } // namespace blink