Fix crash in ~WebProcessPool when using Geolocation with useNetworkProcess=true
authorachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 23 Nov 2015 18:39:16 +0000 (18:39 +0000)
committerachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 23 Nov 2015 18:39:16 +0000 (18:39 +0000)
https://bugs.webkit.org/show_bug.cgi?id=151532

Reviewed by Benjamin Poulain.

Source/WebKit2:

* UIProcess/WebGeolocationManagerProxy.cpp:
(WebKit::WebGeolocationManagerProxy::processPoolDestroyed):
(WebKit::WebGeolocationManagerProxy::processDidClose):
When a WebProcessPool is destroyed, only call stopUpdating if m_updateRequesters.clear()
stopped the updating, like we do in WebGeolocationManagerProxy::removeRequester.
Otherwise, call setEnableHighAccuracy if needed, also like we do in WebGeolocationManagerProxy::removeRequester.

Tools:

* TestWebKitAPI/Tests/WebKit2/Geolocation.cpp:
(TestWebKitAPI::GeolocationTransitionToHighAccuracyStateTracker::eventsChanged):
(TestWebKitAPI::TEST):
(TestWebKitAPI::GeolocationTransitionToLowAccuracyStateTracker::eventsChanged):
(TestWebKitAPI::GeolocationTransitionToHighAccuracyStateTracker::GeolocationTransitionToHighAccuracyStateTracker): Deleted.
(TestWebKitAPI::GeolocationTransitionToLowAccuracyStateTracker::GeolocationTransitionToLowAccuracyStateTracker): Deleted.
Properly load about:blank in all WebViews to clean up.  Without this change, we had a
Geolocation provider stopping after its state tracker was destroyed with its stack frame,
so it was calling a function on a test object that had gone out of scope.
Also, call WKContextSetUsesNetworkProcess(context, true) to show what crash this fixed,
but that will become the default soon and that call will be removed.

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

Source/WebKit2/ChangeLog
Source/WebKit2/UIProcess/WebGeolocationManagerProxy.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKit2/Geolocation.cpp

index d9a1e21..15800f9 100644 (file)
@@ -1,3 +1,17 @@
+2015-11-23  Alex Christensen  <achristensen@webkit.org>
+
+        Fix crash in ~WebProcessPool when using Geolocation with useNetworkProcess=true
+        https://bugs.webkit.org/show_bug.cgi?id=151532
+
+        Reviewed by Benjamin Poulain.
+
+        * UIProcess/WebGeolocationManagerProxy.cpp:
+        (WebKit::WebGeolocationManagerProxy::processPoolDestroyed):
+        (WebKit::WebGeolocationManagerProxy::processDidClose):
+        When a WebProcessPool is destroyed, only call stopUpdating if m_updateRequesters.clear()
+        stopped the updating, like we do in WebGeolocationManagerProxy::removeRequester.
+        Otherwise, call setEnableHighAccuracy if needed, also like we do in WebGeolocationManagerProxy::removeRequester.
+
 2015-11-23  Brian Burg  <bburg@apple.com>
 
         Web Inspector: when inspecting the inspector, add the inspection level to the title bar
index d77e90e..740be46 100644 (file)
@@ -57,8 +57,12 @@ void WebGeolocationManagerProxy::initializeProvider(const WKGeolocationProviderB
 
 void WebGeolocationManagerProxy::processPoolDestroyed()
 {
+    bool wasUpdating = isUpdating();
     m_updateRequesters.clear();
-    m_provider.stopUpdating(this);
+
+    ASSERT(!isUpdating());
+    if (wasUpdating)
+        m_provider.stopUpdating(this);
 }
 
 void WebGeolocationManagerProxy::processDidClose(WebProcessProxy* webProcessProxy)
index d2db228..75f9f46 100644 (file)
@@ -1,3 +1,22 @@
+2015-11-23  Alex Christensen  <achristensen@webkit.org>
+
+        Fix crash in ~WebProcessPool when using Geolocation with useNetworkProcess=true
+        https://bugs.webkit.org/show_bug.cgi?id=151532
+
+        Reviewed by Benjamin Poulain.
+
+        * TestWebKitAPI/Tests/WebKit2/Geolocation.cpp:
+        (TestWebKitAPI::GeolocationTransitionToHighAccuracyStateTracker::eventsChanged):
+        (TestWebKitAPI::TEST):
+        (TestWebKitAPI::GeolocationTransitionToLowAccuracyStateTracker::eventsChanged):
+        (TestWebKitAPI::GeolocationTransitionToHighAccuracyStateTracker::GeolocationTransitionToHighAccuracyStateTracker): Deleted.
+        (TestWebKitAPI::GeolocationTransitionToLowAccuracyStateTracker::GeolocationTransitionToLowAccuracyStateTracker): Deleted.
+        Properly load about:blank in all WebViews to clean up.  Without this change, we had a 
+        Geolocation provider stopping after its state tracker was destroyed with its stack frame,
+        so it was calling a function on a test object that had gone out of scope.
+        Also, call WKContextSetUsesNetworkProcess(context, true) to show what crash this fixed,
+        but that will become the default soon and that call will be removed.
+
 2015-11-22  David Kilzer  <ddkilzer@apple.com>
 
         run-webkit-tests: http server for imported W3C tests doesn't work with --layout-tests-directory switch
index b6d872f..fea4126 100644 (file)
@@ -30,6 +30,7 @@
 #include "PlatformUtilities.h"
 #include "PlatformWebView.h"
 #include "Test.h"
+#include <WebKit/WKContextPrivate.h>
 #include <WebKit/WKRetainPtr.h>
 #include <string.h>
 #include <vector>
@@ -212,14 +213,9 @@ TEST(WebKit2, GeolocationBasicWithHighAccuracy)
 
 // Geolocation start without High Accuracy, then requires High Accuracy.
 struct GeolocationTransitionToHighAccuracyStateTracker : GeolocationStateTracker {
-    bool finishedFirstStep;
-    bool finished;
-
-    GeolocationTransitionToHighAccuracyStateTracker()
-        : finishedFirstStep(false)
-        , finished(false)
-    {
-    }
+    bool finishedFirstStep { false };
+    bool enabledHighAccuracy { false };
+    bool finished { false };
 
     virtual void eventsChanged()
     {
@@ -233,11 +229,19 @@ struct GeolocationTransitionToHighAccuracyStateTracker : GeolocationStateTracker
             break;
         case 3:
             EXPECT_EQ(GeolocationEvent::EnableHighAccuracy, events[2]);
+            enabledHighAccuracy = true;
+            break;
+        case 4:
+            EXPECT_EQ(GeolocationEvent::DisableHighAccuracy, events[3]);
+            break;
+        case 5:
+            EXPECT_EQ(GeolocationEvent::StopUpdating, events[4]);
             finished = true;
             break;
         default:
             EXPECT_TRUE(false);
             finishedFirstStep = true;
+            enabledHighAccuracy = true;
             finished = true;
         }
     }
@@ -246,6 +250,7 @@ struct GeolocationTransitionToHighAccuracyStateTracker : GeolocationStateTracker
 TEST(WebKit2, GeolocationTransitionToHighAccuracy)
 {
     WKRetainPtr<WKContextRef> context(AdoptWK, WKContextCreate());
+    WKContextSetUsesNetworkProcess(context.get(), true);
 
     GeolocationTransitionToHighAccuracyStateTracker stateTracker;
     setupGeolocationProvider(context.get(), &stateTracker);
@@ -260,19 +265,20 @@ TEST(WebKit2, GeolocationTransitionToHighAccuracy)
     setupView(highAccuracyWebView);
     WKRetainPtr<WKURLRef> highAccuracyURL(AdoptWK, Util::createURLForResource("geolocationWatchPositionWithHighAccuracy", "html"));
     WKPageLoadURL(highAccuracyWebView.page(), highAccuracyURL.get());
+    Util::run(&stateTracker.enabledHighAccuracy);
+    
+    WKRetainPtr<WKURLRef> resetUrl = adoptWK(WKURLCreateWithUTF8CString("about:blank"));
+    WKPageLoadURL(highAccuracyWebView.page(), resetUrl.get());
+    Util::run(&stateTracker.enabledHighAccuracy);
+    WKPageLoadURL(lowAccuracyWebView.page(), resetUrl.get());
     Util::run(&stateTracker.finished);
 }
 
 // Geolocation start with High Accuracy, then should fall back to low accuracy.
 struct GeolocationTransitionToLowAccuracyStateTracker : GeolocationStateTracker {
-    bool finishedFirstStep;
-    bool finished;
-
-    GeolocationTransitionToLowAccuracyStateTracker()
-        : finishedFirstStep(false)
-        , finished(false)
-    {
-    }
+    bool finishedFirstStep { false };
+    bool disabledHighAccuracy { false };
+    bool finished { false };
 
     virtual void eventsChanged()
     {
@@ -286,11 +292,16 @@ struct GeolocationTransitionToLowAccuracyStateTracker : GeolocationStateTracker
             break;
         case 3:
             EXPECT_EQ(GeolocationEvent::DisableHighAccuracy, events[2]);
+            disabledHighAccuracy = true;
+            break;
+        case 4:
+            EXPECT_EQ(GeolocationEvent::StopUpdating, events[3]);
             finished = true;
             break;
         default:
             EXPECT_TRUE(false);
             finishedFirstStep = true;
+            disabledHighAccuracy = true;
             finished = true;
         }
     }
@@ -334,6 +345,8 @@ TEST(WebKit2, GeolocationTransitionToLowAccuracy)
 
     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);
 }