Get rid of the useless flag PREEMPT_GEOLOCATION_PERMISSION

https://bugs.webkit.org/show_bug.cgi?id=83325

Patch by Benjamin Poulain <bpoulain@apple.com> on 2012-04-06
Reviewed by Ryosuke Niwa.

Source/WebCore:

The flag WTF_USE_PREEMPT_GEOLOCATION_PERMISSION was added in r63742 but
was never disabled by anyone. Supporting this feature added complexity by
introducing two authorization scheme.

This patch removes WTF_USE_PREEMPT_GEOLOCATION_PERMISSION and the code supporting
granting the authorization after startUpdating().

* Modules/geolocation/Geolocation.cpp:
(WebCore::Geolocation::GeoNotifier::runSuccessCallback): With the simplified
authorization code, we ensure stronger constraint on GeoNotifier::runSuccessCallback().
(WebCore::Geolocation::stop):
(WebCore::Geolocation::startRequest):
(WebCore::Geolocation::clearWatch):
(WebCore::Geolocation::setIsAllowed):
(WebCore::Geolocation::positionChanged): The case (!isAllowed()) was there
to support granting the authorization for WTF_USE_PREEMPT_GEOLOCATION_PERMISSION.
(WebCore::Geolocation::handlePendingPermissionNotifiers):
* Modules/geolocation/Geolocation.h:
(Geolocation):

Source/WebKit/blackberry:

* WebCoreSupport/AboutDataUseFeatures.in:

Source/WTF:

* wtf/Platform.h: Remove the flag.

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

diff --git a/Source/WTF/ChangeLog b/Source/WTF/ChangeLog
index ccf241b..a841d4f 100644 (file)
--- a/Source/WTF/ChangeLog
+++ b/Source/WTF/ChangeLog
@@ -1,3 +1,12 @@
+2012-04-06  Benjamin Poulain  <bpoulain@apple.com>
+
+        Get rid of the useless flag PREEMPT_GEOLOCATION_PERMISSION
+        https://bugs.webkit.org/show_bug.cgi?id=83325
+
+        Reviewed by Ryosuke Niwa.
+
+        * wtf/Platform.h: Remove the flag.
+
 2012-04-06  Darin Adler  <darin@apple.com>
 
         Streamline strtod and fix some related problems

diff --git a/Source/WTF/wtf/Platform.h b/Source/WTF/wtf/Platform.h
index 0c2a473..883a781 100644 (file)
--- a/Source/WTF/wtf/Platform.h
+++ b/Source/WTF/wtf/Platform.h
@@ -1108,9 +1108,6 @@
 #define WTF_USE_ACCESSIBILITY_CONTEXT_MENUS 1
 #endif
 
-/* Geolocation request policy. pre-emptive policy is to acquire user permission before acquiring location. */
-#define WTF_USE_PREEMPT_GEOLOCATION_PERMISSION 1
-
 #if CPU(ARM_THUMB2)
 #define ENABLE_BRANCH_COMPACTION 1
 #endif

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 574fb82..5ca8a65 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,30 @@
+2012-04-06  Benjamin Poulain  <bpoulain@apple.com>
+
+        Get rid of the useless flag PREEMPT_GEOLOCATION_PERMISSION
+        https://bugs.webkit.org/show_bug.cgi?id=83325
+
+        Reviewed by Ryosuke Niwa.
+
+        The flag WTF_USE_PREEMPT_GEOLOCATION_PERMISSION was added in r63742 but
+        was never disabled by anyone. Supporting this feature added complexity by
+        introducing two authorization scheme.
+
+        This patch removes WTF_USE_PREEMPT_GEOLOCATION_PERMISSION and the code supporting
+        granting the authorization after startUpdating().
+
+        * Modules/geolocation/Geolocation.cpp:
+        (WebCore::Geolocation::GeoNotifier::runSuccessCallback): With the simplified
+        authorization code, we ensure stronger constraint on GeoNotifier::runSuccessCallback().
+        (WebCore::Geolocation::stop):
+        (WebCore::Geolocation::startRequest):
+        (WebCore::Geolocation::clearWatch):
+        (WebCore::Geolocation::setIsAllowed):
+        (WebCore::Geolocation::positionChanged): The case (!isAllowed()) was there
+        to support granting the authorization for WTF_USE_PREEMPT_GEOLOCATION_PERMISSION.
+        (WebCore::Geolocation::handlePendingPermissionNotifiers):
+        * Modules/geolocation/Geolocation.h:
+        (Geolocation):
+
 2012-04-06  Tom Sepez  <tsepez@chromium.org>
 
         Block cross-origin iframe scroll to fragment.

diff --git a/Source/WebCore/Modules/geolocation/Geolocation.cpp b/Source/WebCore/Modules/geolocation/Geolocation.cpp
index e2cc89c..e1c2c9f 100644 (file)
--- a/Source/WebCore/Modules/geolocation/Geolocation.cpp
+++ b/Source/WebCore/Modules/geolocation/Geolocation.cpp
@@ -122,10 +122,8 @@ void Geolocation::GeoNotifier::runSuccessCallback(Geoposition* position)
 {
     // If we are here and the Geolocation permission is not approved, something has
     // gone horribly wrong.
-    // We bail out to avoid any privacy issue.
-    ASSERT(m_geolocation->isAllowed());
     if (!m_geolocation->isAllowed())
-        return;
+        CRASH();
 
     m_successCallback->handleEvent(position);
 }
@@ -280,9 +278,7 @@ void Geolocation::stop()
     m_allowGeolocation = Unknown;
     cancelAllRequests();
     stopUpdating();
-#if USE(PREEMPT_GEOLOCATION_PERMISSION)
     m_pendingForPermissionNotifiers.clear();
-#endif
 }
 
 Geoposition* Geolocation::lastPosition()
@@ -335,14 +331,11 @@ PassRefPtr<Geolocation::GeoNotifier> Geolocation::startRequest(PassRefPtr<Positi
         notifier->setUseCachedPosition();
     else if (notifier->hasZeroTimeout())
         notifier->startTimerIfNeeded();
-#if USE(PREEMPT_GEOLOCATION_PERMISSION)
     else if (!isAllowed()) {
         // if we don't yet have permission, request for permission before calling startUpdating()
         m_pendingForPermissionNotifiers.add(notifier);
         requestPermission();
-    }
-#endif
-    else if (startUpdating(notifier.get()))
+    } else if (startUpdating(notifier.get()))
         notifier->startTimerIfNeeded();
     else
         notifier->setFatalError(PositionError::create(PositionError::POSITION_UNAVAILABLE, failedToStartServiceErrorMessage));
@@ -435,10 +428,8 @@ void Geolocation::clearWatch(int watchId)
     if (watchId < firstAvailableWatchId)
         return;
 
-#if USE(PREEMPT_GEOLOCATION_PERMISSION)
     if (GeoNotifier* notifier = m_watchers.find(watchId))
         m_pendingForPermissionNotifiers.remove(notifier);
-#endif
     m_watchers.remove(watchId);
     
     if (!hasListeners())
@@ -454,14 +445,12 @@ void Geolocation::setIsAllowed(bool allowed)
     // position.
     m_allowGeolocation = allowed ? Yes : No;
     
-#if USE(PREEMPT_GEOLOCATION_PERMISSION)
     // Permission request was made during the startRequest process
     if (!m_pendingForPermissionNotifiers.isEmpty()) {
         handlePendingPermissionNotifiers();
         m_pendingForPermissionNotifiers.clear();
         return;
     }
-#endif
 
     if (!isAllowed()) {
         RefPtr<PositionError> error = PositionError::create(PositionError::PERMISSION_DENIED, permissionDeniedErrorMessage);
@@ -642,20 +631,13 @@ void Geolocation::makeSuccessCallbacks()
 
 void Geolocation::positionChanged()
 {
+    ASSERT(isAllowed());
+
     m_cachedPosition = lastPosition();
 
     // Stop all currently running timers.
     stopTimers();
 
-    if (!isAllowed()) {
-        // requestPermission() will ask the chrome for permission. This may be
-        // implemented synchronously or asynchronously. In both cases,
-        // makeSuccessCallbacks() will be called if permission is granted, so
-        // there's nothing more to do here.
-        requestPermission();
-        return;
-    }
-
     makeSuccessCallbacks();
 }
 
@@ -684,7 +666,6 @@ void Geolocation::stopUpdating()
     GeolocationController::from(page)->removeObserver(this);
 }
 
-#if USE(PREEMPT_GEOLOCATION_PERMISSION)
 void Geolocation::handlePendingPermissionNotifiers()
 {
     // While we iterate through the list, we need not worry about list being modified as the permission 
@@ -704,7 +685,6 @@ void Geolocation::handlePendingPermissionNotifiers()
             notifier->setFatalError(PositionError::create(PositionError::PERMISSION_DENIED, permissionDeniedErrorMessage));
     }
 }
-#endif
 
 } // namespace WebCore
                                                         

diff --git a/Source/WebCore/Modules/geolocation/Geolocation.h b/Source/WebCore/Modules/geolocation/Geolocation.h
index 8b63c7f..7f3cf93 100644 (file)
--- a/Source/WebCore/Modules/geolocation/Geolocation.h
+++ b/Source/WebCore/Modules/geolocation/Geolocation.h
@@ -150,9 +150,7 @@ private:
     bool startUpdating(GeoNotifier*);
     void stopUpdating();
 
-#if USE(PREEMPT_GEOLOCATION_PERMISSION)
     void handlePendingPermissionNotifiers();
-#endif
 
     PassRefPtr<GeoNotifier> startRequest(PassRefPtr<PositionCallback>, PassRefPtr<PositionErrorCallback>, PassRefPtr<PositionOptions>);
 
@@ -164,9 +162,7 @@ private:
 
     GeoNotifierSet m_oneShots;
     Watchers m_watchers;
-#if USE(PREEMPT_GEOLOCATION_PERMISSION)
     GeoNotifierSet m_pendingForPermissionNotifiers;
-#endif
     RefPtr<Geoposition> m_lastPosition;
 
     enum {

diff --git a/Source/WebKit/blackberry/ChangeLog b/Source/WebKit/blackberry/ChangeLog
index b079cd5..676c9d6 100644 (file)
--- a/Source/WebKit/blackberry/ChangeLog
+++ b/Source/WebKit/blackberry/ChangeLog
@@ -1,3 +1,12 @@
+2012-04-06  Benjamin Poulain  <bpoulain@apple.com>
+
+        Get rid of the useless flag PREEMPT_GEOLOCATION_PERMISSION
+        https://bugs.webkit.org/show_bug.cgi?id=83325
+
+        Reviewed by Ryosuke Niwa.
+
+        * WebCoreSupport/AboutDataUseFeatures.in:
+
 2012-04-06  Sean Wang  <Xuewen.Wang@torchmobile.com.cn>
 
         [BlackBerry] Text selection - Can't select texts

diff --git a/Source/WebKit/blackberry/WebCoreSupport/AboutDataUseFeatures.in b/Source/WebKit/blackberry/WebCoreSupport/AboutDataUseFeatures.in
index e1a7ced..3fcbf5e 100644 (file)
--- a/Source/WebKit/blackberry/WebCoreSupport/AboutDataUseFeatures.in
+++ b/Source/WebKit/blackberry/WebCoreSupport/AboutDataUseFeatures.in
@@ -49,7 +49,6 @@ OS_RANDOMNESS
 PANGO
 PLATFORM_STRATEGIES
 PLUGIN_HOST_PROCESS
-PREEMPT_GEOLOCATION_PERMISSION
 PROTECTION_SPACE_AUTH_CALLBACK
 PTHREADS
 PTHREAD_GETSPECIFIC_DIRECT