Changeset 238165 in webkit

Timestamp:

Nov 13, 2018 10:14:58 PM (5 years ago)

Author:

rniwa@webkit.org

Message:

WebKit.GeolocationTransitionToLowAccuracy API crashes when enabling PSON
​https://bugs.webkit.org/show_bug.cgi?id=191616

Reviewed by Chris Dumez.

The crash was caused by WKView in autorelease pool invoking stopUpdatingCallback after
GeolocationTransitionToLowAccuracyStateTracker in the stack had been destroyed,
resulting in the use-after-free.

Made the tests more robust by clearing geolocation provider before exiting each test
since we can't really prevent WKView from entering an autorelease pool.

Also made WebKit.GeolocationTransitionToLowAccuracy wait for the success callback
instead of simply the end of the navigation so that the test would continue to work
even if a web content process was created for the second web view (lowAccuracyWebView)

• TestWebKitAPI/Tests/WebKit/Geolocation.cpp:

(TestWebKitAPI::setupGeolocationProvider): Moved "*" to match the WebKit coding style guideline.
(TestWebKitAPI::clearGeolocationProvider): Added.
(TestWebKitAPI::runJavaScriptAlert): Added.
(TestWebKitAPI::didFinishNavigation): Deleted.

• TestWebKitAPI/Tests/WebKit/geolocationWatchPosition.html:

Location:

trunk/Tools

Files:

• ChangeLog (modified) (1 diff)
• TestWebKitAPI/Tests/WebKit/Geolocation.cpp (modified) (9 diffs)
• TestWebKitAPI/Tests/WebKit/geolocationWatchPosition.html (modified) (1 diff)

trunk/Tools/ChangeLog

@@ +1 @@
+2018-11-13  Ryosuke Niwa  <rniwa@webkit.org>
+
+        WebKit.GeolocationTransitionToLowAccuracy API crashes when enabling PSON
+        https://bugs.webkit.org/show_bug.cgi?id=191616
+
+        Reviewed by Chris Dumez.
+
+        The crash was caused by WKView in autorelease pool invoking stopUpdatingCallback after
+        GeolocationTransitionToLowAccuracyStateTracker in the stack had been destroyed,
+        resulting in the use-after-free.
+
+        Made the tests more robust by clearing geolocation provider before exiting each test
+        since we can't really prevent WKView from entering an autorelease pool.
+
+        Also made WebKit.GeolocationTransitionToLowAccuracy wait for the success callback
+        instead of simply the end of the navigation so that the test would continue to work
+        even if a web content process was created for the second web view (lowAccuracyWebView)
+
+        * TestWebKitAPI/Tests/WebKit/Geolocation.cpp:
+        (TestWebKitAPI::setupGeolocationProvider): Moved "*" to match the WebKit coding style guideline.
+        (TestWebKitAPI::clearGeolocationProvider): Added.
+        (TestWebKitAPI::runJavaScriptAlert): Added.
+        (TestWebKitAPI::didFinishNavigation): Deleted.
+        * TestWebKitAPI/Tests/WebKit/geolocationWatchPosition.html:
+
 2018-11-13  Chris Dumez  <cdumez@apple.com>
 

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

@@ -105 +105 @@
 }
 
-void setupGeolocationProvider(WKContextRef context, void *clientInfo)
+void setupGeolocationProvider(WKContextRef context, void* clientInfo)
 {
     WKGeolocationProviderV1 providerCallback;
@@ -118 +118 @@
     WKGeolocationManagerSetProvider(WKContextGetGeolocationManager(context), &providerCallback.base);
 }
+    
+void clearGeolocationProvider(WKContextRef context)
+{
+    WKGeolocationManagerSetProvider(WKContextGetGeolocationManager(context), nullptr);
+}
 
 void setupView(PlatformWebView& webView)
@@ -169 +174 @@
 
     Util::run(&stateTracker.finished);
+    clearGeolocationProvider(context.get());
 }
 
@@ -210 +216 @@
 
     Util::run(&stateTracker.finished);
+    clearGeolocationProvider(context.get());
 }
 
@@ -273 +280 @@
     WKPageLoadURL(lowAccuracyWebView.page(), resetUrl.get());
     Util::run(&stateTracker.finished);
+
+    clearGeolocationProvider(context.get());
 }
 
@@ -308 +317 @@
 };
 
-static void didFinishNavigation(WKPageRef page, WKNavigationRef, WKTypeRef userData, const void* clientInfo)
+static void runJavaScriptAlert(WKPageRef page, WKStringRef alertText, WKFrameRef frame, const void* clientInfo)
 {
     *static_cast<bool*>(const_cast<void*>(clientInfo)) = true;
@@ -316 +325 @@
 {
     WKRetainPtr<WKContextRef> context(AdoptWK, WKContextCreate());
-    WKContextSetMaximumNumberOfProcesses(context.get(), 1);
 
     GeolocationTransitionToLowAccuracyStateTracker stateTracker;
@@ -332 +340 @@
     bool finishedSecondStep = false;
 
-    WKPageNavigationClientV0 loaderClient;
-    memset(&loaderClient, 0, sizeof(loaderClient));
-
-    loaderClient.base.version = 0;
-    loaderClient.base.clientInfo = &finishedSecondStep;
-    loaderClient.didFinishNavigation = didFinishNavigation;
-
-    WKPageSetPageNavigationClient(lowAccuracyWebView.page(), &loaderClient.base);
+    WKPageUIClientV2 uiClient;
+    memset(&uiClient, 0, sizeof(uiClient));
+    uiClient.base.version = 2;
+    uiClient.base.clientInfo = &finishedSecondStep;
+    uiClient.decidePolicyForGeolocationPermissionRequest = decidePolicyForGeolocationPermissionRequestCallBack;
+    uiClient.runJavaScriptAlert = runJavaScriptAlert;
+    WKPageSetPageUIClient(lowAccuracyWebView.page(), &uiClient.base);
 
     WKRetainPtr<WKURLRef> lowAccuracyURL(AdoptWK, Util::createURLForResource("geolocationWatchPosition", "html"));
@@ -347 +354 @@
     WKRetainPtr<WKURLRef> resetUrl = adoptWK(WKURLCreateWithUTF8CString("about:blank"));
     WKPageLoadURL(highAccuracyWebView.page(), resetUrl.get());
+
     Util::run(&stateTracker.disabledHighAccuracy);
+
     WKPageLoadURL(lowAccuracyWebView.page(), resetUrl.get());
     Util::run(&stateTracker.finished);
+
+    clearGeolocationProvider(context.get());
 }
 

trunk/Tools/TestWebKitAPI/Tests/WebKit/geolocationWatchPosition.html

@@ -1 +1 @@
 <script>
-navigator.geolocation.watchPosition(function() { });
+navigator.geolocation.watchPosition(function() { setTimeout(() => alert("SUCCESS"), 0); }, function() { setTimeout(() => alert("FAIL"), 0); }, {timeout: 100});
 </script>