Correctly sets default values for Geolocation PositionOptions.

This will be submitted to WebKit in bug 27254.

3 files changed, 125 insertions, 83 deletions

diff --git a/WebCore/bindings/js/JSGeolocationCustom.cpp b/WebCore/bindings/js/JSGeolocationCustom.cpp
index 493166c..5ecd227 100644
--- a/WebCore/bindings/js/JSGeolocationCustom.cpp
+++ b/WebCore/bindings/js/JSGeolocationCustom.cpp
@@ -39,109 +39,140 @@ using namespace JSC;
 
 namespace WebCore {
 
-static PassRefPtr<PositionOptions> createPositionOptions(ExecState* exec, JSValue value)
+const long PositionOptions::infinity = -1;
+
+static PassRefPtr<PositionCallback> createPositionCallback(ExecState* exec, JSValue value)
 {
-    if (!value.isObject())
+    JSObject* object = value.getObject();
+    if (!object) {
+        setDOMException(exec, TYPE_MISMATCH_ERR);
         return 0;
+    }
 
-    JSObject* object = asObject(value);
+    Frame* frame = toJSDOMWindow(exec->lexicalGlobalObject())->impl()->frame();
+    ASSERT(frame);
+    return JSCustomPositionCallback::create(object, frame);
+}
 
-    JSValue enableHighAccuracyValue = object->get(exec, Identifier(exec, "enableHighAccuracy"));
-    if (exec->hadException())
-        return 0;
-    bool enableHighAccuracy = enableHighAccuracyValue.toBoolean(exec);
-    if (exec->hadException())
+static PassRefPtr<PositionErrorCallback> createPositionErrorCallback(ExecState* exec, JSValue value)
+{
+    // No value is OK.
+    if (value.isUndefinedOrNull()) {
         return 0;
+    }
 
-    JSValue timeoutValue = object->get(exec, Identifier(exec, "timeout"));
-    if (exec->hadException())
-        return 0;
-    unsigned timeout = timeoutValue.toUInt32(exec);
-    if (exec->hadException())
+    JSObject* object = value.getObject();
+    if (!object) {
+        setDOMException(exec, TYPE_MISMATCH_ERR);
         return 0;
+    }
 
-    JSValue maximumAgeValue = object->get(exec, Identifier(exec, "maximumAge"));
-    if (exec->hadException())
-        return 0;
-    unsigned maximumAge = maximumAgeValue.toUInt32(exec);
-    if (exec->hadException())
-        return 0;
+    Frame* frame = toJSDOMWindow(exec->lexicalGlobalObject())->impl()->frame();
+    ASSERT(frame);
+    return JSCustomPositionErrorCallback::create(object, frame);
+}
 
-    return PositionOptions::create(enableHighAccuracy, timeout, maximumAge);
+// If value represents a non-negative number, the value is truncated to a long
+// and assigned to result, and the function returns true. Otherwise, result is
+// not set, and the function returns false.
+static bool getNonNegativeLong(ExecState* exec, const JSValue& value, long* result)
+{
+    if (!value.isNumber() || (value.toNumber(exec) < 0)) {
+        return false;
+    }
+    *result = value.toNumber(exec);
+    return true;
 }
 
-JSValue JSGeolocation::getCurrentPosition(ExecState* exec, const ArgList& args)
+static PassRefPtr<PositionOptions> createPositionOptions(ExecState* exec, JSValue value)
 {
-    // Arguments: PositionCallback, (optional)PositionErrorCallback, (optional)PositionOptions
-    RefPtr<PositionCallback> positionCallback;
-    JSObject* object = args.at(0).getObject();
-    if (exec->hadException())
-        return jsUndefined();
+    // Create default options.
+    RefPtr<PositionOptions> options = PositionOptions::create();
+
+    if (value.isUndefinedOrNull()) {
+        // Use default options.
+        return options;
+    }
+
+    JSObject* object = value.getObject();
     if (!object) {
         setDOMException(exec, TYPE_MISMATCH_ERR);
-        return jsUndefined();
+        return 0;
     }
 
-    if (Frame* frame = toJSDOMWindow(exec->lexicalGlobalObject())->impl()->frame())
-        positionCallback = JSCustomPositionCallback::create(object, frame);
-    
-    RefPtr<PositionErrorCallback> positionErrorCallback;
-    if (!args.at(1).isUndefinedOrNull()) {
-        JSObject* object = args.at(1).getObject();
-        if (!object) {
+    JSValue enableHighAccuracyValue = object->get(exec, Identifier(exec, "enableHighAccuracy"));
+    if (!enableHighAccuracyValue.isUndefinedOrNull()) {
+        if (!enableHighAccuracyValue.isBoolean()) {
             setDOMException(exec, TYPE_MISMATCH_ERR);
-            return jsUndefined();
+            return 0;
         }
+        options->setEnableHighAccuracy(enableHighAccuracyValue.toBoolean(exec));
+    }
 
-        if (Frame* frame = toJSDOMWindow(exec->lexicalGlobalObject())->impl()->frame())
-            positionErrorCallback = JSCustomPositionErrorCallback::create(object, frame);
+    JSValue timeoutValue = object->get(exec, Identifier(exec, "timeout"));
+    if (!timeoutValue.isUndefinedOrNull()) {
+        long timeout;
+        if (getNonNegativeLong(exec, timeoutValue, &timeout))
+            options->setTimeout(timeout);
+        else {
+            setDOMException(exec, TYPE_MISMATCH_ERR);
+            return 0;
+        }
     }
-    
-    RefPtr<PositionOptions> positionOptions;
-    if (!args.at(2).isUndefinedOrNull()) {
-        positionOptions = createPositionOptions(exec, args.at(2));
-        if (exec->hadException())
-            return jsUndefined();
+
+    JSValue maximumAgeValue = object->get(exec, Identifier(exec, "maximumAge"));
+    if (!maximumAgeValue.isUndefinedOrNull()) {
+        long maximumAge;
+        if (getNonNegativeLong(exec, maximumAgeValue, &maximumAge))
+            options->setTimeout(maximumAge);
+        else {
+            setDOMException(exec, TYPE_MISMATCH_ERR);
+            return 0;
+        }
     }
 
+    return options;
+}
+
+JSValue JSGeolocation::getCurrentPosition(ExecState* exec, const ArgList& args)
+{
+    // Arguments: PositionCallback, (optional)PositionErrorCallback, (optional)PositionOptions
+
+    RefPtr<PositionCallback> positionCallback = createPositionCallback(exec, args.at(0));
+    if (exec->hadException())
+        return jsUndefined();
+    ASSERT(positionCallback);
+
+    RefPtr<PositionErrorCallback> positionErrorCallback = createPositionErrorCallback(exec, args.at(1));
+    if (exec->hadException())
+        return jsUndefined();
+
+    RefPtr<PositionOptions> positionOptions = createPositionOptions(exec, args.at(2));
+    if (exec->hadException())
+        return jsUndefined();
+    ASSERT(positionOptions);
+
     m_impl->getCurrentPosition(positionCallback.release(), positionErrorCallback.release(), positionOptions.release());
-    
     return jsUndefined();
 }
 
 JSValue JSGeolocation::watchPosition(ExecState* exec, const ArgList& args)
 {
     // Arguments: PositionCallback, (optional)PositionErrorCallback, (optional)PositionOptions
-    RefPtr<PositionCallback> positionCallback;
-    JSObject* object = args.at(0).getObject();
+
+    RefPtr<PositionCallback> positionCallback = createPositionCallback(exec, args.at(0));
     if (exec->hadException())
         return jsUndefined();
-    if (!object) {
-        setDOMException(exec, TYPE_MISMATCH_ERR);
+    ASSERT(positionCallback);
+
+    RefPtr<PositionErrorCallback> positionErrorCallback = createPositionErrorCallback(exec, args.at(1));
+    if (exec->hadException())
         return jsUndefined();
-    }
-    
-    if (Frame* frame = toJSDOMWindow(exec->lexicalGlobalObject())->impl()->frame())
-        positionCallback = JSCustomPositionCallback::create(object, frame);
-    
-    RefPtr<PositionErrorCallback> positionErrorCallback;
-    if (!args.at(1).isUndefinedOrNull()) {
-        JSObject* object = args.at(1).getObject();
-        if (!object) {
-            setDOMException(exec, TYPE_MISMATCH_ERR);
-            return jsUndefined();
-        }
-        
-        if (Frame* frame = toJSDOMWindow(exec->lexicalGlobalObject())->impl()->frame())
-            positionErrorCallback = JSCustomPositionErrorCallback::create(object, frame);
-    }
-    
-    RefPtr<PositionOptions> positionOptions;
-    if (!args.at(2).isUndefinedOrNull()) {
-        positionOptions = createPositionOptions(exec, args.at(2));
-        if (exec->hadException())
-            return jsUndefined();
-    }
+
+    RefPtr<PositionOptions> positionOptions = createPositionOptions(exec, args.at(2));
+    if (exec->hadException())
+        return jsUndefined();
+    ASSERT(positionOptions);
 
     int watchID = m_impl->watchPosition(positionCallback.release(), positionErrorCallback.release(), positionOptions.release());
     return jsNumber(exec, watchID);
diff --git a/WebCore/page/Geolocation.cpp b/WebCore/page/Geolocation.cpp
index 8df6990..86a7f16 100644
--- a/WebCore/page/Geolocation.cpp
+++ b/WebCore/page/Geolocation.cpp
@@ -40,11 +40,15 @@ Geolocation::GeoNotifier::GeoNotifier(PassRefPtr<PositionCallback> successCallba
     , m_options(options)
     , m_timer(this, &Geolocation::GeoNotifier::timerFired)
 {
+    ASSERT(m_successCallback);
+    // If no options were supplied from JS, we should have created a default set
+    // of options in JSGeolocationCustom.cpp.
+    ASSERT(m_options);
 }
 
 void Geolocation::GeoNotifier::startTimer()
 {
-    if (m_errorCallback && m_options)
+    if (m_errorCallback && m_options->timeout() != PositionOptions::infinity)
         m_timer.startOneShot(m_options->timeout() / 1000.0);
 }
 
diff --git a/WebCore/page/PositionOptions.h b/WebCore/page/PositionOptions.h
index 10845d3..a513f61 100644
--- a/WebCore/page/PositionOptions.h
+++ b/WebCore/page/PositionOptions.h
@@ -33,26 +33,33 @@ namespace WebCore {
     
 class PositionOptions : public RefCounted<PositionOptions> {
 public:
-    static PassRefPtr<PositionOptions> create(bool highAccuracy, unsigned timeout, unsigned maximumAge) { return adoptRef(new PositionOptions(highAccuracy, timeout, maximumAge)); }
+    static const long infinity;
+    static PassRefPtr<PositionOptions> create() { return adoptRef(new PositionOptions()); }
 
     bool enableHighAccuracy() const { return m_highAccuracy; }
     void setEnableHighAccuracy(bool enable) { m_highAccuracy = enable; }
-    unsigned timeout() const { return m_timeout; }
-    void setTimeout(unsigned t) { m_timeout = t; }
-    unsigned maximumAge() const { return m_maximumAge; }
-    void setMaximumAge(unsigned a) { m_maximumAge = a; }
+    long timeout() const { return m_timeout; }
+    void setTimeout(long t) {
+        ASSERT(t == infinity || t >= 0);
+        m_timeout = t;
+    }
+    long maximumAge() const { return m_maximumAge; }
+    void setMaximumAge(long a) {
+        ASSERT(a == infinity || a >= 0);
+        m_maximumAge = a;
+    }
     
 private:
-    PositionOptions(bool highAccuracy, unsigned timeout, unsigned maximumAge)
-        : m_highAccuracy(highAccuracy)
-        , m_timeout(timeout)
-        , m_maximumAge(maximumAge)
+    PositionOptions()
+        : m_highAccuracy(false)
+        , m_timeout(infinity)
+        , m_maximumAge(0)
     {
     }
     
     bool m_highAccuracy;
-    unsigned m_timeout;
-    unsigned m_maximumAge;
+    long m_timeout;
+    long m_maximumAge;
 };
     
 } // namespace WebCore