Title: [192839] trunk/Source/WebCore
Revision
192839
Author
da...@apple.com
Date
2015-11-30 15:57:31 -0800 (Mon, 30 Nov 2015)

Log Message

Use Optional instead of isNull out argument for nullable getters
https://bugs.webkit.org/show_bug.cgi?id=151676

Reviewed by Anders Carlsson.

No behavior change, just cleaner code.

* Modules/geolocation/Coordinates.cpp:
(WebCore::Coordinates::altitude): Return an Optional.
(WebCore::Coordinates::altitudeAccuracy): Ditto.
(WebCore::Coordinates::heading): Ditto.
(WebCore::Coordinates::speed): Ditto.
* Modules/geolocation/Coordinates.h: Ditto.

* Modules/indexeddb/IDBVersionChangeEvent.cpp:
(WebCore::IDBVersionChangeEvent::create): Added. The code before was calling
through to Event::create, which is clearly not what was wanted. Also removed
unneeded explicit destructor.
* Modules/indexeddb/IDBVersionChangeEvent.h: Changed return type of newVersion
to Optional and updated for above change.

* Modules/indexeddb/client/IDBVersionChangeEventImpl.cpp:
(WebCore::IDBClient::IDBVersionChangeEvent::newVersion): Changed to return
an Optional.
* Modules/indexeddb/client/IDBVersionChangeEventImpl.h: Removed unused
default argument values; the event type one, at least, was clearly incorrect.
Made more things private, got rid of unneeded destructor, marked class final
instead of marking all functions final.

* Modules/indexeddb/legacy/LegacyVersionChangeEvent.cpp:
(WebCore::LegacyVersionChangeEvent::newVersion): Same as above.
* Modules/indexeddb/legacy/LegacyVersionChangeEvent.h: Ditto.

* Modules/mediastream/MediaTrackConstraints.cpp:
(WebCore::MediaTrackConstraints::optional): Removed bogus bool value. If we
come back to finish later we will have to implement optional return values
for arrays in the _javascript_ bindings generator, which should be straightforward.
* Modules/mediastream/MediaTrackConstraints.h: Ditto.

* bindings/js/JSDOMBinding.h:
(WebCore::toNullableJSNumber): Added. This function template is used for
return values that are nullable numbers.

* bindings/scripts/CodeGeneratorGObject.pm:
(GenerateFunction): Replaced some existing bogus code to handle nullables with
new equally-bogus code that should be no worse and will compile.

* bindings/scripts/CodeGeneratorJS.pm:
(GenerateImplementation): Removed old support for nullables.
(NativeToJSValue): Added new support for nullable numbers.

* bindings/scripts/CodeGeneratorObjC.pm:
(GenerateImplementation): Removed support for nullables. We almost certainly
won't need it for Objective-C bindings.

* bindings/scripts/test/GObject/WebKitDOMTestObj.cpp: Updated.
* bindings/scripts/test/JS/JSTestObj.cpp: Updated.
* bindings/scripts/test/ObjC/DOMTestObj.mm: Updated.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (192838 => 192839)


--- trunk/Source/WebCore/ChangeLog	2015-11-30 23:48:42 UTC (rev 192838)
+++ trunk/Source/WebCore/ChangeLog	2015-11-30 23:57:31 UTC (rev 192839)
@@ -1,3 +1,64 @@
+2015-11-30  Darin Adler  <da...@apple.com>
+
+        Use Optional instead of isNull out argument for nullable getters
+        https://bugs.webkit.org/show_bug.cgi?id=151676
+
+        Reviewed by Anders Carlsson.
+
+        No behavior change, just cleaner code.
+
+        * Modules/geolocation/Coordinates.cpp:
+        (WebCore::Coordinates::altitude): Return an Optional.
+        (WebCore::Coordinates::altitudeAccuracy): Ditto.
+        (WebCore::Coordinates::heading): Ditto.
+        (WebCore::Coordinates::speed): Ditto.
+        * Modules/geolocation/Coordinates.h: Ditto.
+
+        * Modules/indexeddb/IDBVersionChangeEvent.cpp:
+        (WebCore::IDBVersionChangeEvent::create): Added. The code before was calling
+        through to Event::create, which is clearly not what was wanted. Also removed
+        unneeded explicit destructor.
+        * Modules/indexeddb/IDBVersionChangeEvent.h: Changed return type of newVersion
+        to Optional and updated for above change.
+
+        * Modules/indexeddb/client/IDBVersionChangeEventImpl.cpp:
+        (WebCore::IDBClient::IDBVersionChangeEvent::newVersion): Changed to return
+        an Optional.
+        * Modules/indexeddb/client/IDBVersionChangeEventImpl.h: Removed unused
+        default argument values; the event type one, at least, was clearly incorrect.
+        Made more things private, got rid of unneeded destructor, marked class final
+        instead of marking all functions final.
+
+        * Modules/indexeddb/legacy/LegacyVersionChangeEvent.cpp:
+        (WebCore::LegacyVersionChangeEvent::newVersion): Same as above.
+        * Modules/indexeddb/legacy/LegacyVersionChangeEvent.h: Ditto.
+
+        * Modules/mediastream/MediaTrackConstraints.cpp:
+        (WebCore::MediaTrackConstraints::optional): Removed bogus bool value. If we
+        come back to finish later we will have to implement optional return values
+        for arrays in the _javascript_ bindings generator, which should be straightforward.
+        * Modules/mediastream/MediaTrackConstraints.h: Ditto.
+
+        * bindings/js/JSDOMBinding.h:
+        (WebCore::toNullableJSNumber): Added. This function template is used for
+        return values that are nullable numbers.
+
+        * bindings/scripts/CodeGeneratorGObject.pm:
+        (GenerateFunction): Replaced some existing bogus code to handle nullables with
+        new equally-bogus code that should be no worse and will compile.
+
+        * bindings/scripts/CodeGeneratorJS.pm:
+        (GenerateImplementation): Removed old support for nullables.
+        (NativeToJSValue): Added new support for nullable numbers.
+
+        * bindings/scripts/CodeGeneratorObjC.pm:
+        (GenerateImplementation): Removed support for nullables. We almost certainly
+        won't need it for Objective-C bindings.
+
+        * bindings/scripts/test/GObject/WebKitDOMTestObj.cpp: Updated.
+        * bindings/scripts/test/JS/JSTestObj.cpp: Updated.
+        * bindings/scripts/test/ObjC/DOMTestObj.mm: Updated.
+
 2015-11-30  Wenson Hsieh  <wenson_hs...@apple.com>
 
         Split platform-independent logic in AVCaptureDeviceManager out into a new class

Modified: trunk/Source/WebCore/Modules/geolocation/Coordinates.cpp (192838 => 192839)


--- trunk/Source/WebCore/Modules/geolocation/Coordinates.cpp	2015-11-30 23:48:42 UTC (rev 192838)
+++ trunk/Source/WebCore/Modules/geolocation/Coordinates.cpp	2015-11-30 23:57:31 UTC (rev 192839)
@@ -28,40 +28,32 @@
 
 namespace WebCore {
 
-double Coordinates::altitude(bool& isNull) const
+Optional<double> Coordinates::altitude() const
 {
-    if (m_canProvideAltitude)
-        return m_altitude;
-
-    isNull = true;
-    return 0;
+    if (!m_canProvideAltitude)
+        return Nullopt;
+    return m_altitude;
 }
 
-double Coordinates::altitudeAccuracy(bool& isNull) const
+Optional<double> Coordinates::altitudeAccuracy() const
 {
-    if (m_canProvideAltitudeAccuracy)
-        return m_altitudeAccuracy;
-
-    isNull = true;
-    return 0;
+    if (!m_canProvideAltitudeAccuracy)
+        return Nullopt;
+    return m_altitudeAccuracy;
 }
 
-double Coordinates::heading(bool& isNull) const
+Optional<double> Coordinates::heading() const
 {
-    if (m_canProvideHeading)
-        return m_heading;
-
-    isNull = true;
-    return 0;
+    if (!m_canProvideHeading)
+        return Nullopt;
+    return m_heading;
 }
 
-double Coordinates::speed(bool& isNull) const
+Optional<double> Coordinates::speed() const
 {
-    if (m_canProvideSpeed)
-        return m_speed;
-
-    isNull = true;
-    return 0;
+    if (!m_canProvideSpeed)
+        return Nullopt;
+    return m_speed;
 }
     
 } // namespace WebCore

Modified: trunk/Source/WebCore/Modules/geolocation/Coordinates.h (192838 => 192839)


--- trunk/Source/WebCore/Modules/geolocation/Coordinates.h	2015-11-30 23:48:42 UTC (rev 192838)
+++ trunk/Source/WebCore/Modules/geolocation/Coordinates.h	2015-11-30 23:57:31 UTC (rev 192839)
@@ -27,6 +27,7 @@
 #define Coordinates_h
 
 #include "Event.h"
+#include <wtf/Optional.h>
 #include <wtf/RefCounted.h>
 
 namespace WebCore {
@@ -42,11 +43,11 @@
 
     double latitude() const { return m_latitude; }
     double longitude() const { return m_longitude; }
-    double altitude(bool& isNull) const;
+    Optional<double> altitude() const;
     double accuracy() const { return m_accuracy; }
-    double altitudeAccuracy(bool& isNull) const;
-    double heading(bool& isNull) const;
-    double speed(bool& isNull) const;
+    Optional<double> altitudeAccuracy() const;
+    Optional<double> heading() const;
+    Optional<double> speed() const;
     
 private:
     Coordinates(double latitude, double longitude, bool providesAltitude, double altitude, double accuracy, bool providesAltitudeAccuracy, double altitudeAccuracy, bool providesHeading, double heading, bool providesSpeed, double speed)

Modified: trunk/Source/WebCore/Modules/indexeddb/IDBVersionChangeEvent.cpp (192838 => 192839)


--- trunk/Source/WebCore/Modules/indexeddb/IDBVersionChangeEvent.cpp	2015-11-30 23:48:42 UTC (rev 192838)
+++ trunk/Source/WebCore/Modules/indexeddb/IDBVersionChangeEvent.cpp	2015-11-30 23:57:31 UTC (rev 192839)
@@ -28,6 +28,9 @@
 
 #if ENABLE(INDEXED_DATABASE)
 
+#include "EventNames.h"
+#include "IDBVersionChangeEventImpl.h"
+
 namespace WebCore {
 
 IDBVersionChangeEvent::IDBVersionChangeEvent(const AtomicString& name)
@@ -35,6 +38,13 @@
 {
 }
 
+Ref<IDBVersionChangeEvent> IDBVersionChangeEvent::create()
+{
+    // FIXME: This is called only by document.createEvent. I don't see how it's valuable to create an event with
+    // read-only oldVersion attribute of 0 and newVersion of null; preserving that behavior for now.
+    return IDBClient::IDBVersionChangeEvent::create(0, 0, eventNames().versionchangeEvent);
+}
+
 } // namespace WebCore
 
 #endif // ENABLE(INDEXED_DATABASE)

Modified: trunk/Source/WebCore/Modules/indexeddb/IDBVersionChangeEvent.h (192838 => 192839)


--- trunk/Source/WebCore/Modules/indexeddb/IDBVersionChangeEvent.h	2015-11-30 23:48:42 UTC (rev 192838)
+++ trunk/Source/WebCore/Modules/indexeddb/IDBVersionChangeEvent.h	2015-11-30 23:57:31 UTC (rev 192839)
@@ -29,21 +29,19 @@
 #if ENABLE(INDEXED_DATABASE)
 
 #include "Event.h"
-#include <wtf/PassRefPtr.h>
-#include <wtf/RefPtr.h>
-#include <wtf/text/WTFString.h>
+#include <wtf/Optional.h>
 
 namespace WebCore {
 
 class IDBVersionChangeEvent : public Event {
 public:
-    virtual ~IDBVersionChangeEvent() { }
+    static Ref<IDBVersionChangeEvent> create();
 
     virtual uint64_t oldVersion() const = 0;
-    virtual uint64_t newVersion(bool& isNull) const = 0;
+    virtual Optional<uint64_t> newVersion() const = 0;
 
 protected:
-    IDBVersionChangeEvent(const AtomicString&);
+    explicit IDBVersionChangeEvent(const AtomicString& type);
 };
 
 } // namespace WebCore

Modified: trunk/Source/WebCore/Modules/indexeddb/client/IDBVersionChangeEventImpl.cpp (192838 => 192839)


--- trunk/Source/WebCore/Modules/indexeddb/client/IDBVersionChangeEventImpl.cpp	2015-11-30 23:48:42 UTC (rev 192838)
+++ trunk/Source/WebCore/Modules/indexeddb/client/IDBVersionChangeEventImpl.cpp	2015-11-30 23:57:31 UTC (rev 192839)
@@ -38,9 +38,10 @@
 {
 }
 
-uint64_t IDBVersionChangeEvent::newVersion(bool& isNull) const
+Optional<uint64_t> IDBVersionChangeEvent::newVersion() const
 {
-    isNull = !m_newVersion;
+    if (!m_newVersion)
+        return Nullopt;
     return m_newVersion;
 }
 

Modified: trunk/Source/WebCore/Modules/indexeddb/client/IDBVersionChangeEventImpl.h (192838 => 192839)


--- trunk/Source/WebCore/Modules/indexeddb/client/IDBVersionChangeEventImpl.h	2015-11-30 23:48:42 UTC (rev 192838)
+++ trunk/Source/WebCore/Modules/indexeddb/client/IDBVersionChangeEventImpl.h	2015-11-30 23:57:31 UTC (rev 192839)
@@ -33,23 +33,22 @@
 namespace WebCore {
 namespace IDBClient {
 
-class IDBVersionChangeEvent : public WebCore::IDBVersionChangeEvent {
+class IDBVersionChangeEvent final : public WebCore::IDBVersionChangeEvent {
 public:
-    static Ref<IDBVersionChangeEvent> create(uint64_t oldVersion = 0, uint64_t newVersion = 0, const AtomicString& eventType = AtomicString())
+    static Ref<IDBVersionChangeEvent> create(uint64_t oldVersion, uint64_t newVersion, const AtomicString& eventType)
     {
         return adoptRef(*new IDBVersionChangeEvent(oldVersion, newVersion, eventType));
     }
 
-    virtual uint64_t oldVersion() const override final { return m_oldVersion; }
-    virtual uint64_t newVersion(bool& isNull) const override final;
-
-    virtual EventInterface eventInterface() const override final;
-
 private:
     IDBVersionChangeEvent(uint64_t oldVersion, uint64_t newVersion, const AtomicString& eventType);
 
-    uint64_t m_oldVersion { 0 };
-    uint64_t m_newVersion { 0 };
+    virtual uint64_t oldVersion() const override { return m_oldVersion; }
+    virtual Optional<uint64_t> newVersion() const override;
+    virtual EventInterface eventInterface() const override;
+
+    uint64_t m_oldVersion;
+    uint64_t m_newVersion;
 };
 
 } // namespace IDBClient

Modified: trunk/Source/WebCore/Modules/indexeddb/legacy/LegacyVersionChangeEvent.cpp (192838 => 192839)


--- trunk/Source/WebCore/Modules/indexeddb/legacy/LegacyVersionChangeEvent.cpp	2015-11-30 23:48:42 UTC (rev 192838)
+++ trunk/Source/WebCore/Modules/indexeddb/legacy/LegacyVersionChangeEvent.cpp	2015-11-30 23:57:31 UTC (rev 192839)
@@ -37,13 +37,8 @@
 {
 }
 
-LegacyVersionChangeEvent::~LegacyVersionChangeEvent()
+Optional<uint64_t> LegacyVersionChangeEvent::newVersion() const
 {
-}
-
-uint64_t LegacyVersionChangeEvent::newVersion(bool& isNull) const
-{
-    isNull = false;
     return m_newVersion;
 }
 

Modified: trunk/Source/WebCore/Modules/indexeddb/legacy/LegacyVersionChangeEvent.h (192838 => 192839)


--- trunk/Source/WebCore/Modules/indexeddb/legacy/LegacyVersionChangeEvent.h	2015-11-30 23:48:42 UTC (rev 192838)
+++ trunk/Source/WebCore/Modules/indexeddb/legacy/LegacyVersionChangeEvent.h	2015-11-30 23:57:31 UTC (rev 192839)
@@ -33,23 +33,20 @@
 
 namespace WebCore {
 
-class LegacyVersionChangeEvent : public IDBVersionChangeEvent {
+class LegacyVersionChangeEvent final : public IDBVersionChangeEvent {
 public:
-    static Ref<LegacyVersionChangeEvent> create(unsigned long long oldVersion = 0, unsigned long long newVersion = 0, const AtomicString& eventType = AtomicString())
+    static Ref<LegacyVersionChangeEvent> create(unsigned long long oldVersion, unsigned long long newVersion, const AtomicString& eventType)
     {
         return adoptRef(*new LegacyVersionChangeEvent(oldVersion, newVersion, eventType));
     }
 
-    virtual ~LegacyVersionChangeEvent();
-
-    virtual uint64_t oldVersion() const override final { return m_oldVersion; }
-    virtual uint64_t newVersion(bool& isNull) const override final;
-
-    virtual EventInterface eventInterface() const override final;
-
 private:
     LegacyVersionChangeEvent(unsigned long long oldVersion, unsigned long long newVersion, const AtomicString& eventType);
 
+    virtual uint64_t oldVersion() const override { return m_oldVersion; }
+    virtual Optional<uint64_t> newVersion() const override;
+    virtual EventInterface eventInterface() const override;
+
     uint64_t m_oldVersion;
     uint64_t m_newVersion;
 };

Modified: trunk/Source/WebCore/Modules/mediastream/MediaTrackConstraints.cpp (192838 => 192839)


--- trunk/Source/WebCore/Modules/mediastream/MediaTrackConstraints.cpp	2015-11-30 23:48:42 UTC (rev 192838)
+++ trunk/Source/WebCore/Modules/mediastream/MediaTrackConstraints.cpp	2015-11-30 23:57:31 UTC (rev 192839)
@@ -48,7 +48,7 @@
 {
 }
 
-Vector<PassRefPtr<MediaTrackConstraint>> MediaTrackConstraints::optional(bool) const
+Vector<PassRefPtr<MediaTrackConstraint>> MediaTrackConstraints::optional() const
 {
     // https://bugs.webkit.org/show_bug.cgi?id=121954
     notImplemented();

Modified: trunk/Source/WebCore/Modules/mediastream/MediaTrackConstraints.h (192838 => 192839)


--- trunk/Source/WebCore/Modules/mediastream/MediaTrackConstraints.h	2015-11-30 23:48:42 UTC (rev 192838)
+++ trunk/Source/WebCore/Modules/mediastream/MediaTrackConstraints.h	2015-11-30 23:57:31 UTC (rev 192839)
@@ -44,7 +44,7 @@
 
     static Ref<MediaTrackConstraints> create(PassRefPtr<MediaConstraintsImpl>);
 
-    Vector<PassRefPtr<MediaTrackConstraint>> optional(bool) const;
+    Vector<PassRefPtr<MediaTrackConstraint>> optional() const;
     MediaTrackConstraintSet* mandatory() const;
 
 private:

Modified: trunk/Source/WebCore/bindings/js/JSDOMBinding.h (192838 => 192839)


--- trunk/Source/WebCore/bindings/js/JSDOMBinding.h	2015-11-30 23:48:42 UTC (rev 192838)
+++ trunk/Source/WebCore/bindings/js/JSDOMBinding.h	2015-11-30 23:57:31 UTC (rev 192839)
@@ -51,6 +51,9 @@
 #include <wtf/Noncopyable.h>
 #include <wtf/Vector.h>
 
+// FIXME: We could make this file a lot easier to read by putting all function declarations at the top,
+// and function definitions below, even for template and inline functions.
+
 namespace JSC {
 class HashEntry;
 class JSFunction;
@@ -283,6 +286,7 @@
 void reportCurrentException(JSC::ExecState*);
 
 JSC::JSValue createDOMException(JSC::ExecState*, ExceptionCode);
+
 // Convert a DOM implementation exception code into a _javascript_ exception in the execution state.
 WEBCORE_EXPORT void setDOMException(JSC::ExecState*, ExceptionCode);
 
@@ -305,6 +309,8 @@
 String valueToStringWithNullCheck(JSC::ExecState*, JSC::JSValue); // null if the value is null
 String valueToStringWithUndefinedOrNullCheck(JSC::ExecState*, JSC::JSValue); // null if the value is null or undefined
 
+template<typename T> JSC::JSValue toNullableJSNumber(Optional<T>); // null if the optional is null
+
 inline int32_t finiteInt32Value(JSC::JSValue value, JSC::ExecState* exec, bool& okay)
 {
     double number = value.toNumber(exec);
@@ -350,10 +356,12 @@
 WEBCORE_EXPORT int64_t toInt64(JSC::ExecState*, JSC::JSValue, IntegerConversionConfiguration);
 WEBCORE_EXPORT uint64_t toUInt64(JSC::ExecState*, JSC::JSValue, IntegerConversionConfiguration);
 
-// Returns a Date instnace for the specified value, or NaN if the date is not a number.
+// Returns a Date instance for the specified value, or NaN if the date is not a number.
 JSC::JSValue jsDateOrNaN(JSC::ExecState*, double);
+
 // Returns a Date instance for the specified value, or null if the value is NaN or infinity.
 JSC::JSValue jsDateOrNull(JSC::ExecState*, double);
+
 // NaN if the value can't be converted to a date.
 double valueToDate(JSC::ExecState*, JSC::JSValue);
 
@@ -685,7 +693,12 @@
     JSC::PutPropertySlot propertySlot(&object);
     JSC::JSObject::put(&object, &exec, JSC::Identifier::fromString(&exec, name), value, propertySlot);
 }
-    
+
+template<typename T> inline JSC::JSValue toNullableJSNumber(Optional<T> optionalNumber)
+{
+    return optionalNumber ? JSC::jsNumber(optionalNumber.value()) : JSC::jsNull();
+}
+
 } // namespace WebCore
 
 #endif // JSDOMBinding_h

Modified: trunk/Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm (192838 => 192839)


--- trunk/Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm	2015-11-30 23:48:42 UTC (rev 192838)
+++ trunk/Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm	2015-11-30 23:57:31 UTC (rev 192839)
@@ -575,8 +575,6 @@
 
     push(@txtGetProps, "    case ${propEnum}:\n");
 
-    # FIXME: Should we return a default value when isNull == true?
-
     my $postConvertFunction = "";
     if ($gtype eq "string") {
         push(@txtGetProps, "        g_value_take_string(value, " . $getterFunctionName . "(" . join(", ", @getterArguments) . "));\n");
@@ -1239,6 +1237,12 @@
             $assignPost = ")";
         } else {
             $assign = "${returnType} result = ";
+            if ($function->signature->isNullable) {
+                # FIXME: Returning 0 is probably not right for all nullable attribute values.
+                # We may want to handle this the way we do in the Objective-C bindings: not
+                # handle it at all, and not expose any nullables.
+                $assignPost = ".valueOr(0)";
+            }
         }
 
         if ($functionSigType eq "SerializedScriptValue") {
@@ -1247,12 +1251,6 @@
         }
     }
 
-    # FIXME: Should we return a default value when isNull == true?
-    if ($function->signature->isNullable) {
-        push(@cBody, "    bool isNull = false;\n");
-        push(@callImplParams, "isNull");
-    }
-
     if ($raisesException) {
         push(@cBody, "    WebCore::ExceptionCode ec = 0;\n");
         push(@callImplParams, "ec");

Modified: trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm (192838 => 192839)


--- trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm	2015-11-30 23:48:42 UTC (rev 192838)
+++ trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm	2015-11-30 23:57:31 UTC (rev 192839)
@@ -2278,8 +2278,6 @@
 
             my $name = $attribute->signature->name;
             my $type = $attribute->signature->type;
-            # Nullable wrapper types do not need any special handling as the implementation can return a null pointer.
-            my $isNullable = $attribute->signature->isNullable && !$codeGenerator->IsWrapperType($type);
             $codeGenerator->AssertNotSequenceType($type);
             my $getFunctionName = GetAttributeGetterName($interfaceName, $className, $interface, $attribute);
             my $implGetterFunctionName = $codeGenerator->WK_lcfirst($attribute->signature->extendedAttributes->{"ImplementedAs"} || $name);
@@ -2411,8 +2409,6 @@
                     push(@implContent, "    return JSValue::encode(JS" . $constructorType . "::getConstructor(state->vm(), castedThis->globalObject()));\n");
                 }
             } elsif (!$attribute->signature->extendedAttributes->{"GetterRaisesException"}) {
-                push(@implContent, "    bool isNull = false;\n") if $isNullable;
-
                 my $cacheIndex = 0;
                 if ($attribute->signature->extendedAttributes->{"CachedAttribute"}) {
                     $cacheIndex = $currentCachedAttribute;
@@ -2435,7 +2431,6 @@
                     }
                 } else {
                     my ($functionName, @arguments) = $codeGenerator->GetterExpression(\%implIncludes, $interfaceName, $attribute);
-                    push(@arguments, "isNull") if $isNullable;
                     if ($attribute->signature->extendedAttributes->{"ImplementedBy"}) {
                         my $implementedBy = $attribute->signature->extendedAttributes->{"ImplementedBy"};
                         $implIncludes{"${implementedBy}.h"} = 1;
@@ -2456,22 +2451,12 @@
                     } else {
                         push(@implContent, "    JSValue result = $jsType;\n");
                     }
-
-                    if ($isNullable) {
-                        push(@implContent, "    if (isNull)\n");
-                        push(@implContent, "        return JSValue::encode(jsNull());\n");
-                    }
                 }
 
                 push(@implContent, "    castedThis->m_" . $attribute->signature->name . ".set(state->vm(), castedThis, result);\n") if ($attribute->signature->extendedAttributes->{"CachedAttribute"});
                 push(@implContent, "    return JSValue::encode(result);\n");
 
             } else {
-                if ($isNullable) {
-                    push(@implContent, "    bool isNull = false;\n");
-                    unshift(@arguments, "isNull");
-                }
-
                 unshift(@arguments, GenerateCallWith($attribute->signature->extendedAttributes->{"CallWith"}, \@implContent, "JSValue::encode(jsUndefined())"));
 
                 if ($svgPropertyOrListPropertyType) {
@@ -2484,11 +2469,6 @@
 
                 push(@implContent, "    setDOMException(state, ec);\n");
 
-                if ($isNullable) {
-                    push(@implContent, "    if (isNull)\n");
-                    push(@implContent, "        return JSValue::encode(jsNull());\n");
-                }
-
                 push(@implContent, "    return JSValue::encode(result);\n");
             }
 
@@ -4161,13 +4141,15 @@
         return "Symbol::create(state->vm(), *($value).uid())";
     }
 
-    if ($signature->extendedAttributes->{"Reflect"} and ($type eq "unsigned long" or $type eq "unsigned short")) {
-        $value =~ s/getUnsignedIntegralAttribute/getIntegralAttribute/g;
-        return "jsNumber(std::max(0, " . $value . "))";
-    }
-
     if ($codeGenerator->IsPrimitiveType($type) or $type eq "DOMTimeStamp") {
-        return "jsNumber($value)";
+        # We could instead overload a function to work with optional as well as non-optional numbers, but this
+        # is slightly better because it guarantees we will fail to compile if the IDL file doesn't match the C++.
+        my $function = $signature->isNullable ? "toNullableJSNumber" : "jsNumber";
+        if ($signature->extendedAttributes->{"Reflect"} and ($type eq "unsigned long" or $type eq "unsigned short")) {
+            $value =~ s/getUnsignedIntegralAttribute/getIntegralAttribute/g;
+            return "$function(std::max(0, " . $value . "))";
+        }
+        return "$function($value)";
     }
 
     if ($codeGenerator->IsEnumType($type)) {

Modified: trunk/Source/WebCore/bindings/scripts/CodeGeneratorObjC.pm (192838 => 192839)


--- trunk/Source/WebCore/bindings/scripts/CodeGeneratorObjC.pm	2015-11-30 23:48:42 UTC (rev 192838)
+++ trunk/Source/WebCore/bindings/scripts/CodeGeneratorObjC.pm	2015-11-30 23:57:31 UTC (rev 192839)
@@ -105,7 +105,6 @@
 
 # Constants
 my $shouldUseCGColor = defined $ENV{PLATFORM_NAME} && $ENV{PLATFORM_NAME} ne "macosx";
-my $nullableInit = "bool isNull = false;";
 my $exceptionInit = "WebCore::ExceptionCode ec = 0;";
 my $jsContextSetter = "WebCore::JSMainThreadNullState state;";
 my $exceptionRaiseOnError = "WebCore::raiseOnDOMError(ec);";
@@ -1360,14 +1359,14 @@
                 $getterContentTail .= "))";
             }
 
+            # It would be easy to add nullable support here if we ever need it, but we probably never
+            # will since we are unlikely to add Objective-C bindings that require it in the future.
+            # In particular, we'd have to decide what return type to use for "number or null".
+
             my $getterContent;
-            if ($hasGetterException || $attribute->signature->isNullable) {
+            if ($hasGetterException) {
                 $getterContent = $getterContentHead;
                 my $getterWithoutAttributes = $getterContentHead =~ /\($|, $/ ? "ec" : ", ec";
-                if ($attribute->signature->isNullable) {
-                    $getterContent .= $getterWithoutAttributes ? "isNull" : ", isNull";
-                    $getterWithoutAttributes = 0;
-                }
                 if ($hasGetterException) {
                     $getterContent .= $getterWithoutAttributes ? "ec" : ", ec";
                 }
@@ -1383,11 +1382,6 @@
             push(@implContent, "    $jsContextSetter\n");
             push(@implContent, @customGetterContent);
 
-            # FIXME: Should we return a default value when isNull == true?
-            if ($attribute->signature->isNullable) {
-                push(@implContents, "    $nullableInit\n");
-            }
-
             if ($hasGetterException) {
                 # Differentiated between when the return type is a pointer and
                 # not for white space issue (ie. Foo *result vs. int result).

Modified: trunk/Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp (192838 => 192839)


--- trunk/Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp	2015-11-30 23:48:42 UTC (rev 192838)
+++ trunk/Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp	2015-11-30 23:57:31 UTC (rev 192839)
@@ -2415,8 +2415,7 @@
     WebCore::JSMainThreadNullState state;
     g_return_val_if_fail(WEBKIT_DOM_IS_TEST_OBJ(self), 0);
     WebCore::TestObj* item = WebKit::core(self);
-    bool isNull = false;
-    gdouble result = item->nullableDoubleAttribute(isNull);
+    gdouble result = item->nullableDoubleAttribute().valueOr(0);
     return result;
 }
 
@@ -2425,8 +2424,7 @@
     WebCore::JSMainThreadNullState state;
     g_return_val_if_fail(WEBKIT_DOM_IS_TEST_OBJ(self), 0);
     WebCore::TestObj* item = WebKit::core(self);
-    bool isNull = false;
-    glong result = item->nullableLongAttribute(isNull);
+    glong result = item->nullableLongAttribute().valueOr(0);
     return result;
 }
 
@@ -2435,8 +2433,7 @@
     WebCore::JSMainThreadNullState state;
     g_return_val_if_fail(WEBKIT_DOM_IS_TEST_OBJ(self), FALSE);
     WebCore::TestObj* item = WebKit::core(self);
-    bool isNull = false;
-    gboolean result = item->nullableBooleanAttribute(isNull);
+    gboolean result = item->nullableBooleanAttribute().valueOr(0);
     return result;
 }
 
@@ -2445,8 +2442,7 @@
     WebCore::JSMainThreadNullState state;
     g_return_val_if_fail(WEBKIT_DOM_IS_TEST_OBJ(self), 0);
     WebCore::TestObj* item = WebKit::core(self);
-    bool isNull = false;
-    gchar* result = convertToUTF8String(item->nullableStringAttribute(isNull));
+    gchar* result = convertToUTF8String(item->nullableStringAttribute());
     return result;
 }
 
@@ -2455,8 +2451,7 @@
     WebCore::JSMainThreadNullState state;
     g_return_val_if_fail(WEBKIT_DOM_IS_TEST_OBJ(self), 0);
     WebCore::TestObj* item = WebKit::core(self);
-    bool isNull = false;
-    glong result = item->nullableLongSettableAttribute(isNull);
+    glong result = item->nullableLongSettableAttribute().valueOr(0);
     return result;
 }
 
@@ -2474,9 +2469,8 @@
     g_return_val_if_fail(WEBKIT_DOM_IS_TEST_OBJ(self), 0);
     g_return_val_if_fail(!error || !*error, 0);
     WebCore::TestObj* item = WebKit::core(self);
-    bool isNull = false;
     WebCore::ExceptionCode ec = 0;
-    glong result = item->nullableStringValue(isNull, ec);
+    glong result = item->nullableStringValue(ec).valueOr(0);
     if (ec) {
         WebCore::ExceptionCodeDescription ecdesc(ec);
         g_set_error_literal(error, g_quark_from_string("WEBKIT_DOM"), ecdesc.code, ecdesc.name);
@@ -2515,8 +2509,7 @@
     WebCore::JSMainThreadNullState state;
     g_return_val_if_fail(WEBKIT_DOM_IS_TEST_OBJ(self), 0);
     WebCore::TestObj* item = WebKit::core(self);
-    bool isNull = false;
-    RefPtr<WebCore::TestNode> gobjectResult = WTF::getPtr(item->putForwardsNullableAttribute(isNull));
+    RefPtr<WebCore::TestNode> gobjectResult = WTF::getPtr(item->putForwardsNullableAttribute());
     return WebKit::kit(gobjectResult.get());
 }
 

Modified: trunk/Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp (192838 => 192839)


--- trunk/Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp	2015-11-30 23:48:42 UTC (rev 192838)
+++ trunk/Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp	2015-11-30 23:57:31 UTC (rev 192839)
@@ -1824,11 +1824,8 @@
             return reportDeprecatedGetterError(*state, "TestObj", "nullableDoubleAttribute");
         return throwGetterTypeError(*state, "TestObj", "nullableDoubleAttribute");
     }
-    bool isNull = false;
     auto& impl = castedThis->wrapped();
-    JSValue result = jsNumber(impl.nullableDoubleAttribute(isNull));
-    if (isNull)
-        return JSValue::encode(jsNull());
+    JSValue result = toNullableJSNumber(impl.nullableDoubleAttribute());
     return JSValue::encode(result);
 }
 
@@ -1844,11 +1841,8 @@
             return reportDeprecatedGetterError(*state, "TestObj", "nullableLongAttribute");
         return throwGetterTypeError(*state, "TestObj", "nullableLongAttribute");
     }
-    bool isNull = false;
     auto& impl = castedThis->wrapped();
-    JSValue result = jsNumber(impl.nullableLongAttribute(isNull));
-    if (isNull)
-        return JSValue::encode(jsNull());
+    JSValue result = toNullableJSNumber(impl.nullableLongAttribute());
     return JSValue::encode(result);
 }
 
@@ -1864,11 +1858,8 @@
             return reportDeprecatedGetterError(*state, "TestObj", "nullableBooleanAttribute");
         return throwGetterTypeError(*state, "TestObj", "nullableBooleanAttribute");
     }
-    bool isNull = false;
     auto& impl = castedThis->wrapped();
-    JSValue result = jsBoolean(impl.nullableBooleanAttribute(isNull));
-    if (isNull)
-        return JSValue::encode(jsNull());
+    JSValue result = jsBoolean(impl.nullableBooleanAttribute());
     return JSValue::encode(result);
 }
 
@@ -1884,11 +1875,8 @@
             return reportDeprecatedGetterError(*state, "TestObj", "nullableStringAttribute");
         return throwGetterTypeError(*state, "TestObj", "nullableStringAttribute");
     }
-    bool isNull = false;
     auto& impl = castedThis->wrapped();
-    JSValue result = jsStringWithCache(state, impl.nullableStringAttribute(isNull));
-    if (isNull)
-        return JSValue::encode(jsNull());
+    JSValue result = jsStringWithCache(state, impl.nullableStringAttribute());
     return JSValue::encode(result);
 }
 
@@ -1904,11 +1892,8 @@
             return reportDeprecatedGetterError(*state, "TestObj", "nullableLongSettableAttribute");
         return throwGetterTypeError(*state, "TestObj", "nullableLongSettableAttribute");
     }
-    bool isNull = false;
     auto& impl = castedThis->wrapped();
-    JSValue result = jsNumber(impl.nullableLongSettableAttribute(isNull));
-    if (isNull)
-        return JSValue::encode(jsNull());
+    JSValue result = toNullableJSNumber(impl.nullableLongSettableAttribute());
     return JSValue::encode(result);
 }
 
@@ -1925,12 +1910,9 @@
         return throwGetterTypeError(*state, "TestObj", "nullableStringValue");
     }
     ExceptionCode ec = 0;
-    bool isNull = false;
     auto& impl = castedThis->wrapped();
-    JSValue result = jsNumber(impl.nullableStringValue(isNull, ec));
+    JSValue result = toNullableJSNumber(impl.nullableStringValue(ec));
     setDOMException(state, ec);
-    if (isNull)
-        return JSValue::encode(jsNull());
     return JSValue::encode(result);
 }
 

Modified: trunk/Source/WebCore/bindings/scripts/test/ObjC/DOMTestObj.mm (192838 => 192839)


--- trunk/Source/WebCore/bindings/scripts/test/ObjC/DOMTestObj.mm	2015-11-30 23:48:42 UTC (rev 192838)
+++ trunk/Source/WebCore/bindings/scripts/test/ObjC/DOMTestObj.mm	2015-11-30 23:57:31 UTC (rev 192839)
@@ -800,31 +800,31 @@
 - (double)nullableDoubleAttribute
 {
     WebCore::JSMainThreadNullState state;
-    return IMPL->nullableDoubleAttribute(isNull);
+    return IMPL->nullableDoubleAttribute();
 }
 
 - (int)nullableLongAttribute
 {
     WebCore::JSMainThreadNullState state;
-    return IMPL->nullableLongAttribute(isNull);
+    return IMPL->nullableLongAttribute();
 }
 
 - (BOOL)nullableBooleanAttribute
 {
     WebCore::JSMainThreadNullState state;
-    return IMPL->nullableBooleanAttribute(isNull);
+    return IMPL->nullableBooleanAttribute();
 }
 
 - (NSString *)nullableStringAttribute
 {
     WebCore::JSMainThreadNullState state;
-    return IMPL->nullableStringAttribute(isNull);
+    return IMPL->nullableStringAttribute();
 }
 
 - (int)nullableLongSettableAttribute
 {
     WebCore::JSMainThreadNullState state;
-    return IMPL->nullableLongSettableAttribute(isNull);
+    return IMPL->nullableLongSettableAttribute();
 }
 
 - (void)setNullableLongSettableAttribute:(int)newNullableLongSettableAttribute
@@ -837,7 +837,7 @@
 {
     WebCore::JSMainThreadNullState state;
     WebCore::ExceptionCode ec = 0;
-    int result = IMPL->nullableStringValue(isNull, ec);
+    int result = IMPL->nullableStringValue(ec);
     WebCore::raiseOnDOMError(ec);
     return result;
 }
@@ -863,7 +863,7 @@
 - (DOMTestNode *)putForwardsNullableAttribute
 {
     WebCore::JSMainThreadNullState state;
-    return kit(WTF::getPtr(IMPL->putForwardsNullableAttribute(isNull)));
+    return kit(WTF::getPtr(IMPL->putForwardsNullableAttribute()));
 }
 
 - (void)voidMethod
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to