Modernize Geolocation code

https://bugs.webkit.org/show_bug.cgi?id=178148

Reviewed by Ryosuke Niwa.

Source/WebCore:

Modernize Geolocation code:
- Use std::optional<> instead of separate boolean members
- Make GeolocationPosition a simple struct that can be passed via IPC
- Replace WebGeolocationPosition::Data with GeolocationPosition
- Move logic to construct a GeolocationPosition from a CLLocation on iOS
  in one place to avoid code duplication.

* Modules/geolocation/Coordinates.cpp:
(WebCore::Coordinates::Coordinates):
* Modules/geolocation/Coordinates.h:
(WebCore::Coordinates::create):
(WebCore::Coordinates::isolatedCopy const):
(WebCore::Coordinates::latitude const):
(WebCore::Coordinates::longitude const):
(WebCore::Coordinates::altitude const):
(WebCore::Coordinates::accuracy const):
(WebCore::Coordinates::altitudeAccuracy const):
(WebCore::Coordinates::heading const):
(WebCore::Coordinates::speed const):
* Modules/geolocation/Geolocation.cpp:
(WebCore::createGeoposition):
(WebCore::Geolocation::lastPosition):
* Modules/geolocation/GeolocationClient.h:
* Modules/geolocation/GeolocationController.cpp:
(WebCore::GeolocationController::positionChanged):
(WebCore::GeolocationController::lastPosition):
* Modules/geolocation/GeolocationController.h:
* Modules/geolocation/GeolocationPosition.h:
(WebCore::GeolocationPosition::GeolocationPosition):
The default constructor is only needed by our IPC decoding code.

(WebCore::GeolocationPosition::encode const):
(WebCore::GeolocationPosition::decode):
* Modules/geolocation/ios/GeolocationPositionIOS.mm: Copied from Source/WebCore/Modules/geolocation/Coordinates.cpp.
(WebCore::GeolocationPosition::GeolocationPosition):
* WebCore.xcodeproj/project.pbxproj:
* platform/mock/GeolocationClientMock.cpp:
(WebCore::GeolocationClientMock::lastPosition):
(WebCore::GeolocationClientMock::controllerTimerFired):
* platform/mock/GeolocationClientMock.h:

Source/WebKit:

* Shared/WebGeolocationPosition.cpp:
(WebKit::WebGeolocationPosition::create):
(WebKit::WebGeolocationPosition::~WebGeolocationPosition):
* Shared/WebGeolocationPosition.h:
(WebKit::WebGeolocationPosition::timestamp const):
(WebKit::WebGeolocationPosition::latitude const):
(WebKit::WebGeolocationPosition::longitude const):
(WebKit::WebGeolocationPosition::accuracy const):
(WebKit::WebGeolocationPosition::altitude const):
(WebKit::WebGeolocationPosition::altitudeAccuracy const):
(WebKit::WebGeolocationPosition::heading const):
(WebKit::WebGeolocationPosition::speed const):
(WebKit::WebGeolocationPosition::corePosition const):
(WebKit::WebGeolocationPosition::WebGeolocationPosition):
* UIProcess/API/C/WKGeolocationPosition.cpp:
(WKGeolocationPositionCreate_b):
* UIProcess/API/Cocoa/_WKGeolocationPosition.mm:
(WebKit::wrapper):
* UIProcess/API/glib/WebKitGeolocationProvider.cpp:
(WebKit::WebKitGeolocationProvider::notifyPositionChanged):
* UIProcess/WebGeolocationManagerProxy.cpp:
(WebKit::WebGeolocationManagerProxy::providerDidChangePosition):
* UIProcess/ios/WKGeolocationProviderIOS.mm:
(-[WKLegacyCoreLocationProvider positionChanged:]):
* WebProcess/Geolocation/WebGeolocationManager.cpp:
(WebKit::WebGeolocationManager::didChangePosition):
* WebProcess/Geolocation/WebGeolocationManager.h:
* WebProcess/Geolocation/WebGeolocationManager.messages.in:
* WebProcess/WebCoreSupport/WebGeolocationClient.cpp:
(WebKit::WebGeolocationClient::lastPosition):
* WebProcess/WebCoreSupport/WebGeolocationClient.h:

Source/WebKitLegacy/ios:

* Misc/WebGeolocationCoreLocationProvider.h:
* Misc/WebGeolocationCoreLocationProvider.mm:
(-[WebGeolocationCoreLocationProvider sendLocation:]):
* Misc/WebGeolocationProviderIOS.mm:
(-[_WebCoreLocationUpdateThreadingProxy positionChanged:]):

Source/WebKitLegacy/mac:

* WebCoreSupport/WebGeolocationClient.h:
* WebCoreSupport/WebGeolocationClient.mm:
(WebGeolocationClient::lastPosition):
* WebView/WebGeolocationPosition.mm:
(-[WebGeolocationPositionInternal initWithCoreGeolocationPosition:]):
(core):
(-[WebGeolocationPosition initWithTimestamp:latitude:longitude:accuracy:]):
(-[WebGeolocationPosition initWithGeolocationPosition:]):
* WebView/WebGeolocationPositionInternal.h:

Source/WebKitLegacy/win:

* WebCoreSupport/WebGeolocationClient.cpp:
(WebGeolocationClient::lastPosition):
* WebCoreSupport/WebGeolocationClient.h:
* WebGeolocationPosition.cpp:
(WebGeolocationPosition::initWithTimestamp):
(core):
* WebGeolocationPosition.h:
(WebGeolocationPosition::impl const):

Tools:

* DumpRenderTree/mac/TestRunnerMac.mm:
(TestRunner::setMockGeolocationPosition):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@223192 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Author: [email protected]

Source/WebCore/ChangeLog

@@ -1,3 +1,51 @@
+2017-10-11  Chris Dumez  <[email protected]>
+
+        Modernize Geolocation code
+        https://bugs.webkit.org/show_bug.cgi?id=178148
+
+        Reviewed by Ryosuke Niwa.
+
+        Modernize Geolocation code:
+        - Use std::optional<> instead of separate boolean members
+        - Make GeolocationPosition a simple struct that can be passed via IPC
+        - Replace WebGeolocationPosition::Data with GeolocationPosition
+        - Move logic to construct a GeolocationPosition from a CLLocation on iOS
+          in one place to avoid code duplication.
+
+        * Modules/geolocation/Coordinates.cpp:
+        (WebCore::Coordinates::Coordinates):
+        * Modules/geolocation/Coordinates.h:
+        (WebCore::Coordinates::create):
+        (WebCore::Coordinates::isolatedCopy const):
+        (WebCore::Coordinates::latitude const):
+        (WebCore::Coordinates::longitude const):
+        (WebCore::Coordinates::altitude const):
+        (WebCore::Coordinates::accuracy const):
+        (WebCore::Coordinates::altitudeAccuracy const):
+        (WebCore::Coordinates::heading const):
+        (WebCore::Coordinates::speed const):
+        * Modules/geolocation/Geolocation.cpp:
+        (WebCore::createGeoposition):
+        (WebCore::Geolocation::lastPosition):
+        * Modules/geolocation/GeolocationClient.h:
+        * Modules/geolocation/GeolocationController.cpp:
+        (WebCore::GeolocationController::positionChanged):
+        (WebCore::GeolocationController::lastPosition):
+        * Modules/geolocation/GeolocationController.h:
+        * Modules/geolocation/GeolocationPosition.h:
+        (WebCore::GeolocationPosition::GeolocationPosition):
+        The default constructor is only needed by our IPC decoding code.
+
+        (WebCore::GeolocationPosition::encode const):
+        (WebCore::GeolocationPosition::decode):
+        * Modules/geolocation/ios/GeolocationPositionIOS.mm: Copied from Source/WebCore/Modules/geolocation/Coordinates.cpp.
+        (WebCore::GeolocationPosition::GeolocationPosition):
+        * WebCore.xcodeproj/project.pbxproj:
+        * platform/mock/GeolocationClientMock.cpp:
+        (WebCore::GeolocationClientMock::lastPosition):
+        (WebCore::GeolocationClientMock::controllerTimerFired):
+        * platform/mock/GeolocationClientMock.h:
+
 2017-10-11  Brady Eidson  <[email protected]>
 
         Add a SW context process (where SW scripts will actually execute).

Source/WebCore/Modules/geolocation/Coordinates.cpp

@@ -28,32 +28,10 @@
 
 namespace WebCore {
 
-std::optional<double> Coordinates::altitude() const
+Coordinates::Coordinates(GeolocationPosition&& position)
+    : m_position(WTFMove(position))
 {
-    if (!m_canProvideAltitude)
-        return std::nullopt;
-    return m_altitude;
+    ASSERT(m_position.isValid());
 }
 
-std::optional<double> Coordinates::altitudeAccuracy() const
-{
-    if (!m_canProvideAltitudeAccuracy)
-        return std::nullopt;
-    return m_altitudeAccuracy;
-}
-
-std::optional<double> Coordinates::heading() const
-{
-    if (!m_canProvideHeading)
-        return std::nullopt;
-    return m_heading;
-}
-
-std::optional<double> Coordinates::speed() const
-{
-    if (!m_canProvideSpeed)
-        return std::nullopt;
-    return m_speed;
-}
-    
 } // namespace WebCore

Source/WebCore/Modules/geolocation/Coordinates.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2009 Apple Inc. All Rights Reserved.
+ * Copyright (C) 2009-2017 Apple Inc. All Rights Reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -25,6 +25,7 @@
 
 #pragma once
 
+#include "GeolocationPosition.h"
 #include <wtf/Optional.h>
 #include <wtf/Ref.h>
 #include <wtf/RefCounted.h>
@@ -33,49 +34,28 @@ namespace WebCore {
 
 class Coordinates : public RefCounted<Coordinates> {
 public:
-    static Ref<Coordinates> create(double latitude, double longitude, bool providesAltitude, double altitude, double accuracy, bool providesAltitudeAccuracy, double altitudeAccuracy, bool providesHeading, double heading, bool providesSpeed, double speed) { return adoptRef(*new Coordinates(latitude, longitude, providesAltitude, altitude, accuracy, providesAltitudeAccuracy, altitudeAccuracy, providesHeading, heading, providesSpeed, speed)); }
+    static Ref<Coordinates> create(GeolocationPosition&& position)
+    {
+        return adoptRef(*new Coordinates(WTFMove(position)));
+    }
 
     Ref<Coordinates> isolatedCopy() const
     {
-        return Coordinates::create(m_latitude, m_longitude, m_canProvideAltitude, m_altitude, m_accuracy, m_canProvideAltitudeAccuracy, m_altitudeAccuracy, m_canProvideHeading, m_heading, m_canProvideSpeed, m_speed);
+        return Coordinates::create( GeolocationPosition { m_position });
     }
 
-    double latitude() const { return m_latitude; }
-    double longitude() const { return m_longitude; }
-    std::optional<double> altitude() const;
-    double accuracy() const { return m_accuracy; }
-    std::optional<double> altitudeAccuracy() const;
-    std::optional<double> heading() const;
-    std::optional<double> speed() const;
+    double latitude() const { return m_position.latitude; }
+    double longitude() const { return m_position.longitude; }
+    std::optional<double> altitude() const { return m_position.altitude; }
+    double accuracy() const { return m_position.accuracy; }
+    std::optional<double> altitudeAccuracy() const { return m_position.altitudeAccuracy; }
+    std::optional<double> heading() const { return m_position.heading; }
+    std::optional<double> speed() const { return m_position.speed; }
 
 private:
-    Coordinates(double latitude, double longitude, bool providesAltitude, double altitude, double accuracy, bool providesAltitudeAccuracy, double altitudeAccuracy, bool providesHeading, double heading, bool providesSpeed, double speed)
-        : m_latitude(latitude)
-        , m_longitude(longitude)
-        , m_altitude(altitude)
-        , m_accuracy(accuracy)
-        , m_altitudeAccuracy(altitudeAccuracy)
-        , m_heading(heading)
-        , m_speed(speed)
-        , m_canProvideAltitude(providesAltitude)
-        , m_canProvideAltitudeAccuracy(providesAltitudeAccuracy)
-        , m_canProvideHeading(providesHeading)
-        , m_canProvideSpeed(providesSpeed)
-    {
-    }
+    explicit Coordinates(GeolocationPosition&&);
 
-    double m_latitude;
-    double m_longitude;
-    double m_altitude;
-    double m_accuracy;
-    double m_altitudeAccuracy;
-    double m_heading;
-    double m_speed;
-    
-    bool m_canProvideAltitude;
-    bool m_canProvideAltitudeAccuracy;
-    bool m_canProvideHeading;
-    bool m_canProvideSpeed;
+    GeolocationPosition m_position;
 };
 
 } // namespace WebCore

Source/WebCore/Modules/geolocation/Geolocation.cpp

@@ -53,16 +53,13 @@ static const char failedToStartServiceErrorMessage[] = "Failed to start Geolocat
 static const char framelessDocumentErrorMessage[] = "Geolocation cannot be used in frameless documents";
 static const char originCannotRequestGeolocationErrorMessage[] = "Origin does not have permission to use Geolocation service";
 
-static RefPtr<Geoposition> createGeoposition(GeolocationPosition* position)
+static RefPtr<Geoposition> createGeoposition(std::optional<GeolocationPosition>&& position)
 {
     if (!position)
         return nullptr;
 
-    auto 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(WTFMove(coordinates), convertSecondsToDOMTimeStamp(position->timestamp()));
+    DOMTimeStamp timestamp = convertSecondsToDOMTimeStamp(position->timestamp);
+    return Geoposition::create(Coordinates::create(WTFMove(position.value())), timestamp);
 }
 
 static Ref<PositionError> createPositionError(GeolocationError& error)
@@ -296,7 +293,7 @@ Geoposition* Geolocation::lastPosition()
 {
     Page* page = this->page();
     if (!page)
-        return 0;
+        return nullptr;
 
     m_lastPosition = createGeoposition(GeolocationController::from(page)->lastPosition());
 

Source/WebCore/Modules/geolocation/GeolocationClient.h

@@ -25,6 +25,8 @@
 
 #pragma once
 
+#include <wtf/Optional.h>
+
 namespace WebCore {
 
 class Geolocation;
@@ -42,7 +44,7 @@ class GeolocationClient {
     // We should update WebKit to reflect this if and when the V2 specification
     // is published.
     virtual void setEnableHighAccuracy(bool) = 0;
-    virtual GeolocationPosition* lastPosition() = 0;
+    virtual std::optional<GeolocationPosition> lastPosition() = 0;
 
     virtual void requestPermission(Geolocation&) = 0;
     virtual void cancelPermissionRequest(Geolocation&) = 0;

Source/WebCore/Modules/geolocation/GeolocationController.cpp

@@ -99,7 +99,7 @@ void GeolocationController::cancelPermissionRequest(Geolocation& geolocation)
     m_client.cancelPermissionRequest(geolocation);
 }
 
-void GeolocationController::positionChanged(GeolocationPosition* position)
+void GeolocationController::positionChanged(const std::optional<GeolocationPosition>& position)
 {
     m_lastPosition = position;
     Vector<Ref<Geolocation>> observersVector;
@@ -120,10 +120,10 @@ void GeolocationController::errorOccurred(GeolocationError& error)
         observer->setError(error);
 }
 
-GeolocationPosition* GeolocationController::lastPosition()
+std::optional<GeolocationPosition> GeolocationController::lastPosition()
 {
-    if (m_lastPosition.get())
-        return m_lastPosition.get();
+    if (m_lastPosition)
+        return m_lastPosition.value();
 
     return m_client.lastPosition();
 }

Source/WebCore/Modules/geolocation/GeolocationController.h

@@ -53,10 +53,10 @@ class GeolocationController : public Supplement<Page>, private ActivityStateChan
     void requestPermission(Geolocation&);
     void cancelPermissionRequest(Geolocation&);
 
-    WEBCORE_EXPORT void positionChanged(GeolocationPosition*);
+    WEBCORE_EXPORT void positionChanged(const std::optional<GeolocationPosition>&);
     WEBCORE_EXPORT void errorOccurred(GeolocationError&);
 
-    GeolocationPosition* lastPosition();
+    std::optional<GeolocationPosition> lastPosition();
 
     GeolocationClient& client() { return m_client; }
 
@@ -69,7 +69,7 @@ class GeolocationController : public Supplement<Page>, private ActivityStateChan
 
     void activityStateDidChange(ActivityState::Flags oldActivityState, ActivityState::Flags newActivityState) override;
 
-    RefPtr<GeolocationPosition> m_lastPosition;
+    std::optional<GeolocationPosition> m_lastPosition;
 
     typedef HashSet<Ref<Geolocation>> ObserversSet;
     // All observers; both those requesting high accuracy and those not.