Title: [99161] trunk
Revision
99161
Author
[email protected]
Date
2011-11-03 03:23:10 -0700 (Thu, 03 Nov 2011)

Log Message

Fixed wrong implementation of doubleValue % 2^{64}.
https://bugs.webkit.org/show_bug.cgi?id=67980

Reviewed by Hajime Morita.

Source/_javascript_Core:

fast/events/constructors/progress-event-constructor.html was failing
because of the wrong implementation of conversion from an ECMAScript value
to an IDL unsigned long long value (Spec: http://www.w3.org/TR/WebIDL/#es-unsigned-long-long).
In particular, the calculation of doubleValue % 2^{64} was wrong.
This patch implemented it correctly in doubleToInteger() in wtf/MathExtras.h.

* wtf/MathExtras.h:
(doubleToInteger): Implemented the spec correctly.

Source/WebCore:

fast/events/constructors/progress-event-constructor.html was failing
because of the wrong implementation of conversion from an ECMAScript value
to an IDL unsigned long long value (Spec: http://www.w3.org/TR/WebIDL/#es-unsigned-long-long).
In particular, the calculation of doubleValue % 2^{64} was wrong.
This patch implemented it correctly in doubleToInteger() in wtf/MathExtras.h.

* bindings/js/JSDictionary.cpp:
(WebCore::JSDictionary::convertValue): Uses doubleToInteger().
* bindings/v8/OptionsObject.cpp:
(WebCore::OptionsObject::getKeyValue): Ditto.

LayoutTests:

Enabled progress-event-constructor.html on Chromium-Mac.

* fast/events/constructors/progress-event-constructor-expected.txt:
* fast/events/constructors/progress-event-constructor.html: The change from "12345678901234567168" to "12345678901234567890" is not related to this patch. Just to fix typo. The reason for whether "12345678901234567168" or "12345678901234567890" does not matter is that both values are toStringed to the same value "12345678901234567000".
* platform/chromium/test_expectations.txt:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (99160 => 99161)


--- trunk/LayoutTests/ChangeLog	2011-11-03 10:21:49 UTC (rev 99160)
+++ trunk/LayoutTests/ChangeLog	2011-11-03 10:23:10 UTC (rev 99161)
@@ -1,3 +1,16 @@
+2011-11-03  Kentaro Hara  <[email protected]>
+
+        Fixed wrong implementation of doubleValue % 2^{64}.
+        https://bugs.webkit.org/show_bug.cgi?id=67980
+
+        Reviewed by Hajime Morita.
+
+        Enabled progress-event-constructor.html on Chromium-Mac.
+
+        * fast/events/constructors/progress-event-constructor-expected.txt:
+        * fast/events/constructors/progress-event-constructor.html: The change from "12345678901234567168" to "12345678901234567890" is not related to this patch. Just to fix typo. The reason for whether "12345678901234567168" or "12345678901234567890" does not matter is that both values are toStringed to the same value "12345678901234567000".
+        * platform/chromium/test_expectations.txt:
+
 2011-11-03  Philippe Normand  <[email protected]>
 
         Unreviewed, skip fast/css/webkit-mask-crash-table.html crasher on

Modified: trunk/LayoutTests/fast/events/constructors/progress-event-constructor-expected.txt (99160 => 99161)


--- trunk/LayoutTests/fast/events/constructors/progress-event-constructor-expected.txt	2011-11-03 10:21:49 UTC (rev 99160)
+++ trunk/LayoutTests/fast/events/constructors/progress-event-constructor-expected.txt	2011-11-03 10:23:10 UTC (rev 99161)
@@ -19,7 +19,7 @@
 PASS new ProgressEvent('eventType', { loaded: 9007199254740990 }).loaded is 9007199254740990
 PASS new ProgressEvent('eventType', { loaded: 9007199254740991 }).loaded is 9007199254740991
 PASS new ProgressEvent('eventType', { loaded: 18446744073709551615 }).loaded is 0
-PASS new ProgressEvent('eventType', { loaded: 12345678901234567890 }).loaded is 12345678901234567168
+PASS new ProgressEvent('eventType', { loaded: 12345678901234567890 }).loaded is 12345678901234567890
 PASS new ProgressEvent('eventType', { loaded: -1 }).loaded is 18446744073709551615
 PASS new ProgressEvent('eventType', { loaded: NaN }).loaded is 0
 PASS new ProgressEvent('eventType', { loaded: 123.45 }).loaded is 123
@@ -40,7 +40,7 @@
 PASS new ProgressEvent('eventType', { total: 9007199254740990 }).total is 9007199254740990
 PASS new ProgressEvent('eventType', { total: 9007199254740991 }).total is 9007199254740991
 PASS new ProgressEvent('eventType', { total: 18446744073709551615 }).total is 0
-PASS new ProgressEvent('eventType', { total: 12345678901234567890 }).total is 12345678901234567168
+PASS new ProgressEvent('eventType', { total: 12345678901234567890 }).total is 12345678901234567890
 PASS new ProgressEvent('eventType', { total: -1 }).total is 18446744073709551615
 PASS new ProgressEvent('eventType', { total: NaN }).total is 0
 PASS new ProgressEvent('eventType', { total: 123.45 }).total is 123

Modified: trunk/LayoutTests/fast/events/constructors/progress-event-constructor.html (99160 => 99161)


--- trunk/LayoutTests/fast/events/constructors/progress-event-constructor.html	2011-11-03 10:21:49 UTC (rev 99160)
+++ trunk/LayoutTests/fast/events/constructors/progress-event-constructor.html	2011-11-03 10:23:10 UTC (rev 99161)
@@ -38,7 +38,7 @@
     // [2^53, 2^64 - 1]. A value that is in the unsigned long long range but cannot be represented as a double.
     // Spec: http://www.w3.org/TR/WebIDL/#es-unsigned-long-long
     shouldBe("new ProgressEvent('eventType', { " + attr + ": 18446744073709551615 })." + attr, "0");
-    shouldBe("new ProgressEvent('eventType', { " + attr + ": 12345678901234567890 })." + attr, "12345678901234567168");
+    shouldBe("new ProgressEvent('eventType', { " + attr + ": 12345678901234567890 })." + attr, "12345678901234567890");
 
     // A negative number.
     shouldBe("new ProgressEvent('eventType', { " + attr + ": -1 })." + attr, "18446744073709551615");

Modified: trunk/LayoutTests/platform/chromium/test_expectations.txt (99160 => 99161)


--- trunk/LayoutTests/platform/chromium/test_expectations.txt	2011-11-03 10:21:49 UTC (rev 99160)
+++ trunk/LayoutTests/platform/chromium/test_expectations.txt	2011-11-03 10:23:10 UTC (rev 99161)
@@ -3590,7 +3590,6 @@
 
 // Failing after r94946
 BUGWK67926 WIN : fast/events/constructors/progress-event-constructor.html = TEXT
-BUGWK67980 MAC CPU-CG : fast/events/constructors/progress-event-constructor.html = TEXT
 
 BUG_LEVIN SNOWLEOPARD DEBUG : http/tests/loading/307-after-303-after-post.html = PASS TEXT
 BUG_LEVIN SNOWLEOPARD DEBUG : http/tests/security/mixedContent/insecure-image-in-main-frame.html = PASS TEXT
@@ -3601,7 +3600,6 @@
 BUGWK68372 SNOWLEOPARD : svg/animations/svglength-animation-px-to-exs.html = PASS CRASH
 
 // Tests that are known to fail on Mac10.6 with CPU-Skia graphics.
-BUGWK68436 SNOWLEOPARD CPU : fast/events/constructors/progress-event-constructor.html = TEXT
 BUGWK68436 SNOWLEOPARD CPU DEBUG : fast/repaint/background-clip-text.html = TIMEOUT
 BUGWK68436 SNOWLEOPARD CPU : fast/repaint/text-emphasis-h.html = TIMEOUT IMAGE
 BUGWK68436 SNOWLEOPARD CPU : svg/W3C-SVG-1.1/paths-data-12-t.svg = IMAGE+TEXT

Modified: trunk/Source/_javascript_Core/ChangeLog (99160 => 99161)


--- trunk/Source/_javascript_Core/ChangeLog	2011-11-03 10:21:49 UTC (rev 99160)
+++ trunk/Source/_javascript_Core/ChangeLog	2011-11-03 10:23:10 UTC (rev 99161)
@@ -1,3 +1,19 @@
+2011-11-03  Kentaro Hara  <[email protected]>
+
+        Fixed wrong implementation of doubleValue % 2^{64}.
+        https://bugs.webkit.org/show_bug.cgi?id=67980
+
+        Reviewed by Hajime Morita.
+
+        fast/events/constructors/progress-event-constructor.html was failing
+        because of the wrong implementation of conversion from an ECMAScript value
+        to an IDL unsigned long long value (Spec: http://www.w3.org/TR/WebIDL/#es-unsigned-long-long).
+        In particular, the calculation of doubleValue % 2^{64} was wrong.
+        This patch implemented it correctly in doubleToInteger() in wtf/MathExtras.h.
+
+        * wtf/MathExtras.h:
+        (doubleToInteger): Implemented the spec correctly.
+
 2011-11-03  Sheriff Bot  <[email protected]>
 
         Unreviewed, rolling out r99089.

Modified: trunk/Source/_javascript_Core/wtf/MathExtras.h (99160 => 99161)


--- trunk/Source/_javascript_Core/wtf/MathExtras.h	2011-11-03 10:21:49 UTC (rev 99160)
+++ trunk/Source/_javascript_Core/wtf/MathExtras.h	2011-11-03 10:23:10 UTC (rev 99161)
@@ -293,4 +293,27 @@
         mantissa |= 0x10000000000000ull;
 }
 
+// Calculate d % 2^{64}.
+inline void doubleToInteger(double d, unsigned long long& value)
+{
+    if (isnan(d) || isinf(d))
+        value = 0;
+    else {
+        // -2^{64} < fmodValue < 2^{64}.
+        double fmodValue = fmod(trunc(d), std::numeric_limits<unsigned long long>::max() + 1.0);
+        if (fmodValue >= 0) {
+            // 0 <= fmodValue < 2^{64}.
+            // 0 <= value < 2^{64}. This cast causes no loss.
+            value = static_cast<unsigned long long>(fmodValue);
+        } else {
+            // -2^{64} < fmodValue < 0.
+            // 0 < fmodValueInUnsignedLongLong < 2^{64}. This cast causes no loss.
+            unsigned long long fmodValueInUnsignedLongLong = static_cast<unsigned long long>(-fmodValue);
+            // -1 < (std::numeric_limits<unsigned long long>::max() - fmodValueInUnsignedLongLong) < 2^{64} - 1.
+            // 0 < value < 2^{64}.
+            value = std::numeric_limits<unsigned long long>::max() - fmodValueInUnsignedLongLong + 1;
+        }
+    }
+}
+
 #endif // #ifndef WTF_MathExtras_h

Modified: trunk/Source/WebCore/ChangeLog (99160 => 99161)


--- trunk/Source/WebCore/ChangeLog	2011-11-03 10:21:49 UTC (rev 99160)
+++ trunk/Source/WebCore/ChangeLog	2011-11-03 10:23:10 UTC (rev 99161)
@@ -1,3 +1,21 @@
+2011-11-03  Kentaro Hara  <[email protected]>
+
+        Fixed wrong implementation of doubleValue % 2^{64}.
+        https://bugs.webkit.org/show_bug.cgi?id=67980
+
+        Reviewed by Hajime Morita.
+
+        fast/events/constructors/progress-event-constructor.html was failing
+        because of the wrong implementation of conversion from an ECMAScript value
+        to an IDL unsigned long long value (Spec: http://www.w3.org/TR/WebIDL/#es-unsigned-long-long).
+        In particular, the calculation of doubleValue % 2^{64} was wrong.
+        This patch implemented it correctly in doubleToInteger() in wtf/MathExtras.h.
+
+        * bindings/js/JSDictionary.cpp:
+        (WebCore::JSDictionary::convertValue): Uses doubleToInteger().
+        * bindings/v8/OptionsObject.cpp:
+        (WebCore::OptionsObject::getKeyValue): Ditto.
+
 2011-11-03  Alexander Pavlov  <[email protected]>
 
         Web Inspector: Introduce SuggestBox for TextPrompt

Modified: trunk/Source/WebCore/bindings/js/JSDictionary.cpp (99160 => 99161)


--- trunk/Source/WebCore/bindings/js/JSDictionary.cpp	2011-11-03 10:21:49 UTC (rev 99160)
+++ trunk/Source/WebCore/bindings/js/JSDictionary.cpp	2011-11-03 10:23:10 UTC (rev 99161)
@@ -32,6 +32,7 @@
 #include "JSNode.h"
 #include "SerializedScriptValue.h"
 #include "ScriptValue.h"
+#include <wtf/MathExtras.h>
 
 using namespace JSC;
 
@@ -78,10 +79,7 @@
 void JSDictionary::convertValue(ExecState* exec, JSValue value, unsigned long long& result)
 {
     double d = value.toNumber(exec);
-    if (isnan(d) || isinf(d))
-        result = 0;
-    else
-        result = static_cast<unsigned long long>(fmod(trunc(d), std::numeric_limits<unsigned long long>::max() + 1.0));
+    doubleToInteger(d, result);
 }
 
 void JSDictionary::convertValue(ExecState* exec, JSValue value, double& result)

Modified: trunk/Source/WebCore/bindings/v8/OptionsObject.cpp (99160 => 99161)


--- trunk/Source/WebCore/bindings/v8/OptionsObject.cpp	2011-11-03 10:21:49 UTC (rev 99160)
+++ trunk/Source/WebCore/bindings/v8/OptionsObject.cpp	2011-11-03 10:23:10 UTC (rev 99161)
@@ -30,7 +30,7 @@
 #include "V8Binding.h"
 #include "V8DOMWindow.h"
 #include "V8MessagePortCustom.h"
-#include <limits>
+#include <wtf/MathExtras.h>
 
 #if ENABLE(INDEXED_DATABASE)
 #include "IDBKeyRange.h"
@@ -180,10 +180,7 @@
     if (v8Number.IsEmpty())
         return false;
     double d = v8Number->Value();
-    if (isnan(d) || isinf(d))
-        value = 0;
-    else
-        value = static_cast<unsigned long long>(fmod(trunc(d), std::numeric_limits<unsigned long long>::max() + 1.0));
+    doubleToInteger(d, value);
     return true;
 }
 
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to