Changeset 243533 in webkit

Timestamp:

Mar 27, 2019, 2:36:29 AM (7 years ago)

Author:

Carlos Garcia Campos

Message:

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.

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):

Location:

trunk

Files:

• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/UIProcess/WebGeolocationManagerProxy.cpp (modified) (2 diffs)
• Source/WebKit/UIProcess/WebGeolocationManagerProxy.h (modified) (3 diffs)
• Source/WebKit/WebProcess/WebCoreSupport/WebGeolocationClient.cpp (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKit/Geolocation.cpp (modified) (4 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebKit/UIProcess/WebGeolocationManagerProxy.cpp

@@ -89 +89 @@
 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()));
 }
 
@@ -117 +119 @@
         m_provider->setEnableHighAccuracy(*this, isHighAccuracyEnabled());
         m_provider->startUpdating(*this);
-    }
+    } else if (m_lastPosition)
+        connection.send(Messages::WebGeolocationManager::DidChangePosition(m_lastPosition.value()), 0);
 }
 

trunk/Source/WebKit/UIProcess/WebGeolocationManagerProxy.h

@@ -30 +30 @@
 #include "MessageReceiver.h"
 #include "WebContextSupplement.h"
+#include <WebCore/GeolocationPosition.h>
 #include <wtf/HashSet.h>
 #include <wtf/text/WTFString.h>
@@ -55 +56 @@
     void resetPermissions();
 #endif
+    const Optional<WebCore::GeolocationPosition>& lastPosition() const { return m_lastPosition; }
 
     using API::Object::ref;
@@ -83 +85 @@
 
     std::unique_ptr<API::GeolocationProvider> m_provider;
+    Optional<WebCore::GeolocationPosition> m_lastPosition;
 };
 

trunk/Source/WebKit/WebProcess/WebCoreSupport/WebGeolocationClient.cpp

@@ -66 +66 @@
 Optional<GeolocationPosition> WebGeolocationClient::lastPosition()
 {
-    // FIXME: Implement this.
     return WTF::nullopt;
 }

trunk/Tools/ChangeLog

@@ +1 @@
+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>
 

trunk/Tools/TestWebKitAPI/Tests/WebKit/Geolocation.cpp

@@ -316 +316 @@
 };
 
+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);
 }
 
@@ -337 +344 @@
     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;
@@ -349 +356 @@
     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"));
@@ -362 +370 @@
 }
 
+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