Changeset 136164 in webkit

Timestamp:

Nov 29, 2012, 2:18:32 PM (14 years ago)

Author:

jknotten@chromium.org

Message:

Use GeolocationController's last geoposition as cached position.
​https://bugs.webkit.org/show_bug.cgi?id=103540

Reviewed by Benjamin Poulain.

The page's GeolocationController mediates access to the
GeolocationClient for multiple frames' Geolocation instances. This
patch changes the position cache to be on the GeolocationController
rather than on the Geolocation instance.

This fixes a bug where if one frame has has received a fresh
position, then a request for a cached position from a second frame
does not succeed because the Geolocation instance in the second
frame's position cache hasn't received the position update that
went to the first frame.

Source/WebCore:

Test: fast/dom/Geolocation/cached-position-iframe.html

• Modules/geolocation/Geolocation.cpp:

(WebCore::Geolocation::makeCachedPositionCallbacks):
(WebCore::Geolocation::haveSuitableCachedPosition):
(WebCore::Geolocation::positionChanged):

• Modules/geolocation/Geolocation.h:
• Modules/geolocation/GeolocationController.h:

(GeolocationController):

LayoutTests:

• fast/dom/Geolocation/cached-position-iframe-expected.txt: Added.
• fast/dom/Geolocation/cached-position-iframe.html: Added.
• fast/dom/Geolocation/resources/cached-position-iframe-inner.html: Added.
• fast/dom/Geolocation/script-tests/cached-position-iframe.js: Added.

(window.onmessage):

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/dom/Geolocation/cached-position-iframe-expected.txt (added)
• LayoutTests/fast/dom/Geolocation/cached-position-iframe.html (added)
• LayoutTests/fast/dom/Geolocation/resources/cached-position-iframe-inner.html (added)
• LayoutTests/fast/dom/Geolocation/script-tests/cached-position-iframe.js (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/Modules/geolocation/Geolocation.cpp (modified) (4 diffs)
• Source/WebCore/Modules/geolocation/Geolocation.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2012-11-29  John Knottenbelt  <jknotten@chromium.org>
+
+        Use GeolocationController's last geoposition as cached position.
+        https://bugs.webkit.org/show_bug.cgi?id=103540
+
+        Reviewed by Benjamin Poulain.
+
+        The page's GeolocationController mediates access to the
+        GeolocationClient for multiple frames' Geolocation instances. This
+        patch changes the position cache to be on the GeolocationController
+        rather than on the Geolocation instance.
+
+        This fixes a bug where if one frame has has received a fresh
+        position, then a request for a cached position from a second frame
+        does not succeed because the Geolocation instance in the second
+        frame's position cache hasn't received the position update that
+        went to the first frame.
+
+        * fast/dom/Geolocation/cached-position-iframe-expected.txt: Added.
+        * fast/dom/Geolocation/cached-position-iframe.html: Added.
+        * fast/dom/Geolocation/resources/cached-position-iframe-inner.html: Added.
+        * fast/dom/Geolocation/script-tests/cached-position-iframe.js: Added.
+        (window.onmessage):
+
 2012-11-29  Zhenyao Mo  <zmo@google.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-11-29  John Knottenbelt  <jknotten@chromium.org>
+
+        Use GeolocationController's last geoposition as cached position.
+        https://bugs.webkit.org/show_bug.cgi?id=103540
+
+        Reviewed by Benjamin Poulain.
+
+        The page's GeolocationController mediates access to the
+        GeolocationClient for multiple frames' Geolocation instances. This
+        patch changes the position cache to be on the GeolocationController
+        rather than on the Geolocation instance.
+
+        This fixes a bug where if one frame has has received a fresh
+        position, then a request for a cached position from a second frame
+        does not succeed because the Geolocation instance in the second
+        frame's position cache hasn't received the position update that
+        went to the first frame.
+
+        Test: fast/dom/Geolocation/cached-position-iframe.html
+
+        * Modules/geolocation/Geolocation.cpp:
+        (WebCore::Geolocation::makeCachedPositionCallbacks):
+        (WebCore::Geolocation::haveSuitableCachedPosition):
+        (WebCore::Geolocation::positionChanged):
+        * Modules/geolocation/Geolocation.h:
+        * Modules/geolocation/GeolocationController.h:
+        (GeolocationController):
+
 2012-11-29  Alexei Filippov  <alph@chromium.org>
 

trunk/Source/WebCore/Modules/geolocation/Geolocation.cpp

@@ -374 +374 @@
     for (GeoNotifierSet::const_iterator iter = m_requestsAwaitingCachedPosition.begin(); iter != end; ++iter) {
         GeoNotifier* notifier = iter->get();
-        notifier->runSuccessCallback(m_cachedPosition.get());
+        notifier->runSuccessCallback(lastPosition());
 
         // If this is a one-shot request, stop it. Otherwise, if the watch still
@@ -405 +405 @@
 bool Geolocation::haveSuitableCachedPosition(PositionOptions* options)
 {
-    if (!m_cachedPosition)
+    Geoposition* cachedPosition = lastPosition();
+    if (!cachedPosition)
         return false;
     if (!options->hasMaximumAge())
@@ -412 +413 @@
         return false;
     DOMTimeStamp currentTimeMillis = convertSecondsToDOMTimeStamp(currentTime());
-    return m_cachedPosition->timestamp() > currentTimeMillis - options->maximumAge();
+    return cachedPosition->timestamp() > currentTimeMillis - options->maximumAge();
 }
 
@@ -625 +626 @@
     ASSERT(isAllowed());
 
-    m_cachedPosition = lastPosition();
-
     // Stop all currently running timers.
     stopTimers();

trunk/Source/WebCore/Modules/geolocation/Geolocation.h

@@ -174 +174 @@
     } m_allowGeolocation;
 
-    RefPtr<Geoposition> m_cachedPosition;
     GeoNotifierSet m_requestsAwaitingCachedPosition;
 };