- Revision
- 124724
- Author
- [email protected]
- Date
- 2012-08-05 18:36:00 -0700 (Sun, 05 Aug 2012)
Log Message
Assert in checkValidity() in hashtable.h from WebGeolocationManager::didFailToDeterminePosition() when fetching http://html5demos.com/geo
https://bugs.webkit.org/show_bug.cgi?id=80386
Patch by Benjamin Poulain <[email protected]> on 2012-08-05
Reviewed by Alexey Proskuryakov.
Source/WebKit2:
WebKit2's WebGeolocationManager was delivering events without accounting that each event
can modify the list of page that need delivery.
Any page can remove itself by invoking clearWatch() from the callback function. A page can also cause another
page to be removed.
This patch solves the issue by taking a copy of the list of page before delivery. Each page is
referenced as it can be deleted during the delivery.
Unfortunately, this cannot be tested due to missing features of WebKitTestRunner.
* WebProcess/Geolocation/WebGeolocationManager.cpp:
(WebKit::WebGeolocationManager::didChangePosition):
(WebKit::WebGeolocationManager::didFailToDeterminePosition):
LayoutTests:
Add tests for clearing the Geolocation as a listener from a callback originated from the GeolocationController.
* fast/dom/Geolocation/error-clear-watch-expected.txt: Added.
* fast/dom/Geolocation/error-clear-watch.html: Added.
* fast/dom/Geolocation/success-clear-watch-expected.txt: Added.
* fast/dom/Geolocation/success-clear-watch.html: Added.
* platform/wk2/Skipped:
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (124723 => 124724)
--- trunk/LayoutTests/ChangeLog 2012-08-06 01:22:40 UTC (rev 124723)
+++ trunk/LayoutTests/ChangeLog 2012-08-06 01:36:00 UTC (rev 124724)
@@ -1,3 +1,18 @@
+2012-08-05 Benjamin Poulain <[email protected]>
+
+ Assert in checkValidity() in hashtable.h from WebGeolocationManager::didFailToDeterminePosition() when fetching http://html5demos.com/geo
+ https://bugs.webkit.org/show_bug.cgi?id=80386
+
+ Reviewed by Alexey Proskuryakov.
+
+ Add tests for clearing the Geolocation as a listener from a callback originated from the GeolocationController.
+
+ * fast/dom/Geolocation/error-clear-watch-expected.txt: Added.
+ * fast/dom/Geolocation/error-clear-watch.html: Added.
+ * fast/dom/Geolocation/success-clear-watch-expected.txt: Added.
+ * fast/dom/Geolocation/success-clear-watch.html: Added.
+ * platform/wk2/Skipped:
+
2012-08-05 Luke Macpherson <[email protected]>
Fix null pointer dereference when CSSParser::sinkFloatingValueList() returns null and is passed to storeVariableDeclaration().
Added: trunk/LayoutTests/fast/dom/Geolocation/error-clear-watch-expected.txt (0 => 124724)
--- trunk/LayoutTests/fast/dom/Geolocation/error-clear-watch-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/dom/Geolocation/error-clear-watch-expected.txt 2012-08-06 01:36:00 UTC (rev 124724)
@@ -0,0 +1,9 @@
+This tests removing the watcher from an error callback does not causes assertions.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/dom/Geolocation/error-clear-watch.html (0 => 124724)
--- trunk/LayoutTests/fast/dom/Geolocation/error-clear-watch.html (rev 0)
+++ trunk/LayoutTests/fast/dom/Geolocation/error-clear-watch.html 2012-08-06 01:36:00 UTC (rev 124724)
@@ -0,0 +1,32 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<script>
+description("This tests removing the watcher from an error callback does not causes assertions.");
+
+var mockCode = 2;
+var mockMessage = "debug";
+
+if (window.testRunner) {
+ testRunner.setGeolocationPermission(true);
+ testRunner.setMockGeolocationError(mockCode, mockMessage);
+} else
+ debug('This test can not be run without the LayoutTestController');
+
+var watchId = navigator.geolocation.watchPosition(function() {
+ navigator.geolocation.clearWatch(watchId);
+ finishJSTest();
+}, function(e) {
+ navigator.geolocation.clearWatch(watchId);
+ finishJSTest();
+});
+
+
+window.jsTestIsAsync = true;
+</script>
+<script src=""
+</body>
+</html>
Added: trunk/LayoutTests/fast/dom/Geolocation/success-clear-watch-expected.txt (0 => 124724)
--- trunk/LayoutTests/fast/dom/Geolocation/success-clear-watch-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/dom/Geolocation/success-clear-watch-expected.txt 2012-08-06 01:36:00 UTC (rev 124724)
@@ -0,0 +1,9 @@
+This tests removing the watcher from a position callback does not causes assertions.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/dom/Geolocation/success-clear-watch.html (0 => 124724)
--- trunk/LayoutTests/fast/dom/Geolocation/success-clear-watch.html (rev 0)
+++ trunk/LayoutTests/fast/dom/Geolocation/success-clear-watch.html 2012-08-06 01:36:00 UTC (rev 124724)
@@ -0,0 +1,35 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<script>
+description("This tests removing the watcher from a position callback does not causes assertions.");
+
+var mockLatitude = 51.478;
+var mockLongitude = -0.166;
+var mockAccuracy = 100;
+
+if (window.testRunner) {
+ testRunner.setGeolocationPermission(true);
+ testRunner.setMockGeolocationPosition(mockLatitude,
+ mockLongitude,
+ mockAccuracy);
+} else
+ debug('This test can not be run without the LayoutTestController');
+
+var watchId = navigator.geolocation.watchPosition(function() {
+ navigator.geolocation.clearWatch(watchId);
+ finishJSTest();
+}, function(e) {
+ navigator.geolocation.clearWatch(watchId);
+ finishJSTest();
+});
+
+
+window.jsTestIsAsync = true;
+</script>
+<script src=""
+</body>
+</html>
Modified: trunk/LayoutTests/platform/wk2/Skipped (124723 => 124724)
--- trunk/LayoutTests/platform/wk2/Skipped 2012-08-06 01:22:40 UTC (rev 124723)
+++ trunk/LayoutTests/platform/wk2/Skipped 2012-08-06 01:36:00 UTC (rev 124724)
@@ -504,6 +504,7 @@
fast/dom/Geolocation/disconnected-frame-already.html
fast/dom/Geolocation/disconnected-frame-permission-denied.html
fast/dom/Geolocation/disconnected-frame.html
+fast/dom/Geolocation/error-clear-watch.html
fast/dom/Geolocation/error.html
fast/dom/Geolocation/maximum-age.html
fast/dom/Geolocation/multiple-requests.html
@@ -516,6 +517,7 @@
fast/dom/Geolocation/reentrant-error.html
fast/dom/Geolocation/reentrant-permission-denied.html
fast/dom/Geolocation/reentrant-success.html
+fast/dom/Geolocation/success-clear-watch.html
fast/dom/Geolocation/success.html
fast/dom/Geolocation/timeout.html
fast/dom/Geolocation/timeout-clear-watch.html
Modified: trunk/Source/WebKit2/ChangeLog (124723 => 124724)
--- trunk/Source/WebKit2/ChangeLog 2012-08-06 01:22:40 UTC (rev 124723)
+++ trunk/Source/WebKit2/ChangeLog 2012-08-06 01:36:00 UTC (rev 124724)
@@ -1,3 +1,24 @@
+2012-08-05 Benjamin Poulain <[email protected]>
+
+ Assert in checkValidity() in hashtable.h from WebGeolocationManager::didFailToDeterminePosition() when fetching http://html5demos.com/geo
+ https://bugs.webkit.org/show_bug.cgi?id=80386
+
+ Reviewed by Alexey Proskuryakov.
+
+ WebKit2's WebGeolocationManager was delivering events without accounting that each event
+ can modify the list of page that need delivery.
+ Any page can remove itself by invoking clearWatch() from the callback function. A page can also cause another
+ page to be removed.
+
+ This patch solves the issue by taking a copy of the list of page before delivery. Each page is
+ referenced as it can be deleted during the delivery.
+
+ Unfortunately, this cannot be tested due to missing features of WebKitTestRunner.
+
+ * WebProcess/Geolocation/WebGeolocationManager.cpp:
+ (WebKit::WebGeolocationManager::didChangePosition):
+ (WebKit::WebGeolocationManager::didFailToDeterminePosition):
+
2012-08-04 No'am Rosenthal <[email protected]>
[Qt] UI_SIDE_COMPOSITING code has confusing names
Modified: trunk/Source/WebKit2/WebProcess/Geolocation/WebGeolocationManager.cpp (124723 => 124724)
--- trunk/Source/WebKit2/WebProcess/Geolocation/WebGeolocationManager.cpp 2012-08-06 01:22:40 UTC (rev 124723)
+++ trunk/Source/WebKit2/WebProcess/Geolocation/WebGeolocationManager.cpp 2012-08-06 01:36:00 UTC (rev 124724)
@@ -76,10 +76,10 @@
#if ENABLE(GEOLOCATION)
RefPtr<GeolocationPosition> position = GeolocationPosition::create(data.timestamp, data.latitude, data.longitude, data.accuracy);
- HashSet<WebPage*>::const_iterator it = m_pageSet.begin();
- HashSet<WebPage*>::const_iterator end = m_pageSet.end();
- for (; it != end; ++it) {
- WebPage* page = *it;
+ Vector<RefPtr<WebPage> > webPageCopy;
+ copyToVector(m_pageSet, webPageCopy);
+ for (size_t i = 0; i < webPageCopy.size(); ++i) {
+ WebPage* page = webPageCopy[i].get();
if (page->corePage())
GeolocationController::from(page->corePage())->positionChanged(position.get());
}
@@ -92,10 +92,10 @@
// FIXME: Add localized error string.
RefPtr<GeolocationError> error = GeolocationError::create(GeolocationError::PositionUnavailable, /* Localized error string */ String(""));
- HashSet<WebPage*>::const_iterator it = m_pageSet.begin();
- HashSet<WebPage*>::const_iterator end = m_pageSet.end();
- for (; it != end; ++it) {
- WebPage* page = *it;
+ Vector<RefPtr<WebPage> > webPageCopy;
+ copyToVector(m_pageSet, webPageCopy);
+ for (size_t i = 0; i < webPageCopy.size(); ++i) {
+ WebPage* page = webPageCopy[i].get();
if (page->corePage())
GeolocationController::from(page->corePage())->errorOccurred(error.get());
}
