Geolocation request not complete when watch request was started in a different web...
authorcarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 27 Mar 2019 09:36:29 +0000 (09:36 +0000)
committercarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 27 Mar 2019 09:36:29 +0000 (09:36 +0000)
https://bugs.webkit.org/show_bug.cgi?id=195996

Reviewed by Alex Christensen.

Source/WebKit:

In WebGeolocationManagerProxy::startUpdating() we do nothing when the provider is already updating. We should
reply with a DidChangePosition using the last known position, if available. If we are updating, but we still
don't have a known position, the request will be completed when
WebGeolocationManagerProxy::providerDidChangePosition() is called since it always notifies all web
processes.

* UIProcess/WebGeolocationManagerProxy.cpp:
(WebKit::WebGeolocationManagerProxy::providerDidChangePosition): Cache the position.
(WebKit::WebGeolocationManagerProxy::startUpdating): Reply using cached position if already known.
* UIProcess/WebGeolocationManagerProxy.h:
(WebKit::WebGeolocationManagerProxy::lastPosition const): Return cached position.
* WebProcess/WebCoreSupport/WebGeolocationClient.cpp:
(WebKit::WebGeolocationClient::lastPosition): Remove the FIXME since we don't want this feature.

Tools:

Add a test case.

* TestWebKitAPI/Tests/WebKit/Geolocation.cpp:
(TestWebKitAPI::runJavaScriptAlert):
(TestWebKitAPI::TEST):

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

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/WebGeolocationManagerProxy.cpp
Source/WebKit/UIProcess/WebGeolocationManagerProxy.h
Source/WebKit/WebProcess/WebCoreSupport/WebGeolocationClient.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKit/Geolocation.cpp

index 3940ed6..65b3ee3 100644 (file)
@@ -1,3 +1,24 @@
+2019-03-27  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        Geolocation request not complete when watch request was started in a different web process
+        https://bugs.webkit.org/show_bug.cgi?id=195996
+
+        Reviewed by Alex Christensen.
+
+        In WebGeolocationManagerProxy::startUpdating() we do nothing when the provider is already updating. We should
+        reply with a DidChangePosition using the last known position, if available. If we are updating, but we still
+        don't have a known position, the request will be completed when
+        WebGeolocationManagerProxy::providerDidChangePosition() is called since it always notifies all web
+        processes.
+
+        * UIProcess/WebGeolocationManagerProxy.cpp:
+        (WebKit::WebGeolocationManagerProxy::providerDidChangePosition): Cache the position.
+        (WebKit::WebGeolocationManagerProxy::startUpdating): Reply using cached position if already known.
+        * UIProcess/WebGeolocationManagerProxy.h:
+        (WebKit::WebGeolocationManagerProxy::lastPosition const): Return cached position.
+        * WebProcess/WebCoreSupport/WebGeolocationClient.cpp:
+        (WebKit::WebGeolocationClient::lastPosition): Remove the FIXME since we don't want this feature.
+
 2019-03-26  Brent Fulgham  <bfulgham@apple.com>
 
         [macOS] Correct kerberos-related sandbox violations
index 57f0136..99d6ea3 100644 (file)
@@ -88,10 +88,12 @@ void WebGeolocationManagerProxy::derefWebContextSupplement()
 
 void WebGeolocationManagerProxy::providerDidChangePosition(WebGeolocationPosition* position)
 {
+    m_lastPosition = position->corePosition();
+
     if (!processPool())
         return;
 
-    processPool()->sendToAllProcesses(Messages::WebGeolocationManager::DidChangePosition(position->corePosition()));
+    processPool()->sendToAllProcesses(Messages::WebGeolocationManager::DidChangePosition(m_lastPosition.value()));
 }
 
 void WebGeolocationManagerProxy::providerDidFailToDeterminePosition(const String& errorMessage)
@@ -116,7 +118,8 @@ void WebGeolocationManagerProxy::startUpdating(IPC::Connection& connection)
     if (!wasUpdating) {
         m_provider->setEnableHighAccuracy(*this, isHighAccuracyEnabled());
         m_provider->startUpdating(*this);
-    }
+    } else if (m_lastPosition)
+        connection.send(Messages::WebGeolocationManager::DidChangePosition(m_lastPosition.value()), 0);
 }
 
 void WebGeolocationManagerProxy::stopUpdating(IPC::Connection& connection)
index d16389b..17c0869 100644 (file)
@@ -29,6 +29,7 @@
 #include "Connection.h"
 #include "MessageReceiver.h"
 #include "WebContextSupplement.h"
+#include <WebCore/GeolocationPosition.h>
 #include <wtf/HashSet.h>
 #include <wtf/text/WTFString.h>
 
@@ -54,6 +55,7 @@ public:
 #if PLATFORM(IOS_FAMILY)
     void resetPermissions();
 #endif
+    const Optional<WebCore::GeolocationPosition>& lastPosition() const { return m_lastPosition; }
 
     using API::Object::ref;
     using API::Object::deref;
@@ -82,6 +84,7 @@ private:
     HashSet<const IPC::Connection::Client*> m_highAccuracyRequesters;
 
     std::unique_ptr<API::GeolocationProvider> m_provider;
+    Optional<WebCore::GeolocationPosition> m_lastPosition;
 };
 
 } // namespace WebKit
index 826b7ac..a24e0ff 100644 (file)
@@ -65,7 +65,6 @@ void WebGeolocationClient::setEnableHighAccuracy(bool enabled)
 
 Optional<GeolocationPosition> WebGeolocationClient::lastPosition()
 {
-    // FIXME: Implement this.
     return WTF::nullopt;
 }
 
index 99dbe54..9f1455f 100644 (file)
@@ -1,3 +1,16 @@
+2019-03-27  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        Geolocation request not complete when watch request was started in a different web process
+        https://bugs.webkit.org/show_bug.cgi?id=195996
+
+        Reviewed by Alex Christensen.
+
+        Add a test case.
+
+        * TestWebKitAPI/Tests/WebKit/Geolocation.cpp:
+        (TestWebKitAPI::runJavaScriptAlert):
+        (TestWebKitAPI::TEST):
+
 2019-03-26  Keith Rollin  <krollin@apple.com>
 
         Update the way generate-xcfilelists returns strings from functions
index 58dd74b..d729e26 100644 (file)
@@ -315,9 +315,16 @@ struct GeolocationTransitionToLowAccuracyStateTracker : GeolocationStateTracker
     }
 };
 
+struct JavaScriptAlertContext {
+    bool didRun { false };
+    std::string alertText;
+};
+
 static void runJavaScriptAlert(WKPageRef page, WKStringRef alertText, WKFrameRef frame, const void* clientInfo)
 {
-    *static_cast<bool*>(const_cast<void*>(clientInfo)) = true;
+    auto* context = static_cast<JavaScriptAlertContext*>(const_cast<void*>(clientInfo));
+    context->didRun = true;
+    context->alertText = Util::toSTD(alertText);
 }
 
 TEST(WebKit, GeolocationTransitionToLowAccuracy)
@@ -336,19 +343,20 @@ TEST(WebKit, GeolocationTransitionToLowAccuracy)
     PlatformWebView lowAccuracyWebView(context.get());
     setupView(lowAccuracyWebView);
 
-    bool finishedSecondStep = false;
+    JavaScriptAlertContext secondStepContext;
 
     WKPageUIClientV2 uiClient;
     memset(&uiClient, 0, sizeof(uiClient));
     uiClient.base.version = 2;
-    uiClient.base.clientInfo = &finishedSecondStep;
+    uiClient.base.clientInfo = &secondStepContext;
     uiClient.decidePolicyForGeolocationPermissionRequest = decidePolicyForGeolocationPermissionRequestCallBack;
     uiClient.runJavaScriptAlert = runJavaScriptAlert;
     WKPageSetPageUIClient(lowAccuracyWebView.page(), &uiClient.base);
 
     WKRetainPtr<WKURLRef> lowAccuracyURL(AdoptWK, Util::createURLForResource("geolocationWatchPosition", "html"));
     WKPageLoadURL(lowAccuracyWebView.page(), lowAccuracyURL.get());
-    Util::run(&finishedSecondStep);
+    Util::run(&secondStepContext.didRun);
+    EXPECT_EQ(secondStepContext.alertText, "SUCCESS");
 
     WKRetainPtr<WKURLRef> resetUrl = adoptWK(WKURLCreateWithUTF8CString("about:blank"));
     WKPageLoadURL(highAccuracyWebView.page(), resetUrl.get());
@@ -361,6 +369,42 @@ TEST(WebKit, GeolocationTransitionToLowAccuracy)
     clearGeolocationProvider(context.get());
 }
 
+TEST(WebKit, GeolocationWatchMultiprocess)
+{
+    WKRetainPtr<WKContextRef> context(AdoptWK, WKContextCreateWithConfiguration(nullptr));
+
+    GeolocationStateTracker stateTracker;
+    setupGeolocationProvider(context.get(), &stateTracker);
+
+    JavaScriptAlertContext testContext;
+
+    WKPageUIClientV2 uiClient;
+    memset(&uiClient, 0, sizeof(uiClient));
+    uiClient.base.version = 2;
+    uiClient.base.clientInfo = &testContext;
+    uiClient.decidePolicyForGeolocationPermissionRequest = decidePolicyForGeolocationPermissionRequestCallBack;
+    uiClient.runJavaScriptAlert = runJavaScriptAlert;
+
+    PlatformWebView view1(context.get());
+    WKPageSetPageUIClient(view1.page(), &uiClient.base);
+    WKRetainPtr<WKURLRef> url(AdoptWK, Util::createURLForResource("geolocationWatchPosition", "html"));
+    WKPageLoadURL(view1.page(), url.get());
+    Util::run(&testContext.didRun);
+    EXPECT_EQ(testContext.alertText, "SUCCESS");
+    WKPageSetPageUIClient(view1.page(), nullptr);
+
+    testContext.didRun = false;
+    testContext.alertText = { };
+
+    PlatformWebView view2(context.get());
+    WKPageSetPageUIClient(view2.page(), &uiClient.base);
+    WKPageLoadURL(view2.page(), url.get());
+    Util::run(&testContext.didRun);
+    EXPECT_EQ(testContext.alertText, "SUCCESS");
+
+    clearGeolocationProvider(context.get());
+}
+
 } // namespace TestWebKitAPI
 
 #endif