[WTR] Crash in WebGeolocationManagerProxy::~WebGeolocationManagerProxy() when running...
authorcarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 13 Jun 2017 17:48:50 +0000 (17:48 +0000)
committercarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 13 Jun 2017 17:48:50 +0000 (17:48 +0000)
https://bugs.webkit.org/show_bug.cgi?id=173315

Reviewed by Darin Adler.

This has started to happen after r218165, but I don't think it's a regression, but that r218165 revealed the bug
somehow in WTR. The problem is that GeolocationProviderMock keeps a pointer to the WKGeolocationManagerRef
returned by WKContextGetGeolocationManager. But in TestController::generatePageConfiguration() the context is
freed before the GeolocationProviderMock. When the GeolocationProviderMock is then destroyed, it calls
WKGeolocationManagerSetProvider(m_geolocationManager, 0); but the WKGeolocationManagerRef has already been
destroyed. GeolocationProviderMock should keep a reference to the WKContext to ensure the
WKGeolocationManagerRef is not destroyed.

* WebKitTestRunner/GeolocationProviderMock.cpp:
(WTR::GeolocationProviderMock::GeolocationProviderMock):
* WebKitTestRunner/GeolocationProviderMock.h:

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

Tools/ChangeLog
Tools/WebKitTestRunner/GeolocationProviderMock.cpp
Tools/WebKitTestRunner/GeolocationProviderMock.h

index 7e509e4..fe7456a 100644 (file)
@@ -1,3 +1,22 @@
+2017-06-13  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [WTR] Crash in WebGeolocationManagerProxy::~WebGeolocationManagerProxy() when running several tests
+        https://bugs.webkit.org/show_bug.cgi?id=173315
+
+        Reviewed by Darin Adler.
+
+        This has started to happen after r218165, but I don't think it's a regression, but that r218165 revealed the bug
+        somehow in WTR. The problem is that GeolocationProviderMock keeps a pointer to the WKGeolocationManagerRef
+        returned by WKContextGetGeolocationManager. But in TestController::generatePageConfiguration() the context is
+        freed before the GeolocationProviderMock. When the GeolocationProviderMock is then destroyed, it calls
+        WKGeolocationManagerSetProvider(m_geolocationManager, 0); but the WKGeolocationManagerRef has already been
+        destroyed. GeolocationProviderMock should keep a reference to the WKContext to ensure the
+        WKGeolocationManagerRef is not destroyed.
+
+        * WebKitTestRunner/GeolocationProviderMock.cpp:
+        (WTR::GeolocationProviderMock::GeolocationProviderMock):
+        * WebKitTestRunner/GeolocationProviderMock.h:
+
 2017-06-13  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         Unable to paste text that was copied from a page into the universal search field
index ad41649..3a18d0b 100644 (file)
@@ -46,11 +46,9 @@ static void stopUpdatingCallback(WKGeolocationManagerRef geolocationManager, con
 }
 
 GeolocationProviderMock::GeolocationProviderMock(WKContextRef context)
-    : m_isActive(false)
-    , m_hasError(false)
+    : m_context(context)
+    , m_geolocationManager(WKContextGetGeolocationManager(context))
 {
-    m_geolocationManager = WKContextGetGeolocationManager(context);
-
     WKGeolocationProviderV1 providerCallback;
     memset(&providerCallback, 0, sizeof(WKGeolocationProviderV1));
     providerCallback.base.version = 1;
index 802223e..950fc69 100644 (file)
@@ -47,12 +47,13 @@ private:
     void sendPositionIfNeeded();
     void sendErrorIfNeeded();
 
+    WKRetainPtr<WKContextRef> m_context;
     WKGeolocationManagerRef m_geolocationManager;
-    bool m_isActive;
+    bool m_isActive { false };
 
     WKRetainPtr<WKGeolocationPositionRef> m_position;
 
-    bool m_hasError;
+    bool m_hasError { false };
     WKRetainPtr<WKStringRef> m_errorMessage;
 };