Changeset 72396 in webkit

Timestamp:

Nov 19, 2010 7:06:41 AM (13 years ago)

Author:

commit-queue@webkit.org

Message:

2010-11-19 John Knottenbelt <​jknotten@chromium.org>

Reviewed by Jeremy Orlow.

Reentrant Geolocation tests crash with an assertion.
​https://bugs.webkit.org/show_bug.cgi?id=39908

PositionErrors should not be sent to Geolocation watches or one
shots that are due to receive a valid cached position. Re-enable
fast/dom/Geolocation/maximum-age.html on Mac WebKit.

• platform/mac/Skipped:

2010-11-19 John Knottenbelt <​jknotten@chromium.org>

Reviewed by Jeremy Orlow.

Reentrant Geolocation tests crash with an assertion.
​https://bugs.webkit.org/show_bug.cgi?id=39908

PositionErrors should not be sent to Geolocation watches or one shots
that are due to receive a valid cached position.

Test: fast/dom/Geolocation/maximum-age.html

• page/Geolocation.cpp: (WebCore::Geolocation::sendError): (WebCore::Geolocation::copyCachedNotifiers): (WebCore::Geolocation::handleError):
• page/Geolocation.h:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/mac/Skipped (modified) (1 diff)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/page/Geolocation.cpp (modified) (2 diffs)
• WebCore/page/Geolocation.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2010-11-19  John Knottenbelt  <jknotten@chromium.org>
+
+        Reviewed by Jeremy Orlow.
+
+        Reentrant Geolocation tests crash with an assertion.
+        https://bugs.webkit.org/show_bug.cgi?id=39908
+
+        PositionErrors should not be sent to Geolocation watches or one
+        shots that are due to receive a valid cached position. Re-enable
+        fast/dom/Geolocation/maximum-age.html on Mac WebKit.
+
+        * platform/mac/Skipped:
+
 2010-11-19  Anton Muhin  <antonm@chromium.org>
 

trunk/LayoutTests/platform/mac/Skipped

@@ -182 +182 @@
 http/tests/misc/prefetch-purpose.html
 
-# https://bugs.webkit.org/show_bug.cgi?id=39908
-fast/dom/Geolocation/maximum-age.html
-
 # Filenames aren't filtered out from edit drags yet, see https://bugs.wekit.org/show_bug.cgi?id=38826
 editing/pasteboard/file-drag-to-editable.html

trunk/WebCore/ChangeLog

@@ +1 @@
+2010-11-19  John Knottenbelt  <jknotten@chromium.org>
+
+        Reviewed by Jeremy Orlow.
+
+        Reentrant Geolocation tests crash with an assertion.
+        https://bugs.webkit.org/show_bug.cgi?id=39908
+
+        PositionErrors should not be sent to Geolocation watches or one shots
+        that are due to receive a valid cached position.
+
+        Test: fast/dom/Geolocation/maximum-age.html
+
+        * page/Geolocation.cpp:
+        (WebCore::Geolocation::sendError):
+        (WebCore::Geolocation::copyCachedNotifiers):
+        (WebCore::Geolocation::handleError):
+        * page/Geolocation.h:
+
 2010-11-19  Sam Magnuson  <smagnuso@gmail.com>
 

trunk/WebCore/page/Geolocation.cpp

@@ -523 +523 @@
 }
 
+void Geolocation::extractNotifiersWithCachedPosition(GeoNotifierVector& notifiers, GeoNotifierVector* cached)
+{
+    GeoNotifierVector nonCached;
+    GeoNotifierVector::iterator end = notifiers.end();
+    for (GeoNotifierVector::const_iterator it = notifiers.begin(); it != end; ++it) {
+        GeoNotifier* notifier = it->get();
+        if (notifier->m_useCachedPosition) {
+            if (cached)
+                cached->append(notifier);
+        } else
+            nonCached.append(notifier);
+    }
+    notifiers.swap(nonCached);
+}
+
+void Geolocation::copyToSet(const GeoNotifierVector& src, GeoNotifierSet& dest)
+{
+     GeoNotifierVector::const_iterator end = src.end();
+     for (GeoNotifierVector::const_iterator it = src.begin(); it != end; ++it) {
+         GeoNotifier* notifier = it->get();
+         dest.add(notifier);
+     }
+}
+
 void Geolocation::handleError(PositionError* error)
 {
@@ -536 +560 @@
     // added by calls to Geolocation methods from the callbacks, and to prevent
     // further callbacks to these notifiers.
+    GeoNotifierVector oneShotsWithCachedPosition;
     m_oneShots.clear();
     if (error->isFatal())
         m_watchers.clear();
+    else {
+        // Don't send non-fatal errors to notifiers due to receive a cached position.
+        extractNotifiersWithCachedPosition(oneShotsCopy, &oneShotsWithCachedPosition);
+        extractNotifiersWithCachedPosition(watchersCopy, 0);
+    }
 
     sendError(oneShotsCopy, error);
     sendError(watchersCopy, error);
 
+    // hasListeners() doesn't distinguish between notifiers due to receive a
+    // cached position and those requiring a fresh position. Perform the check
+    // before restoring the notifiers below.
     if (!hasListeners())
         stopUpdating();
+
+    // Maintain a reference to the cached notifiers until their timer fires.
+    copyToSet(oneShotsWithCachedPosition, m_oneShots);
 }
 

trunk/WebCore/page/Geolocation.h

@@ -134 +134 @@
     void sendPosition(GeoNotifierVector&, Geoposition*);
 
+    static void extractNotifiersWithCachedPosition(GeoNotifierVector& notifiers, GeoNotifierVector* cached);
+    static void copyToSet(const GeoNotifierVector&, GeoNotifierSet&);
+
     static void stopTimer(GeoNotifierVector&);
     void stopTimersForOneShots();