Diff
Modified: trunk/JSTests/ChangeLog (215776 => 215777)
--- trunk/JSTests/ChangeLog 2017-04-26 00:42:32 UTC (rev 215776)
+++ trunk/JSTests/ChangeLog 2017-04-26 00:52:35 UTC (rev 215777)
@@ -1,3 +1,25 @@
+2017-04-25 Saam Barati <sbar...@apple.com>
+
+ JSArray::isArrayPrototypeIteratorProtocolFastAndNonObservable is wrong because it does not do the necessary checks on the base object
+ https://bugs.webkit.org/show_bug.cgi?id=171150
+ <rdar://problem/31771880>
+
+ Reviewed by Sam Weinig.
+
+ * stress/spread-optimized-properly.js: Added.
+ (assert):
+ (test):
+ (shallowEq):
+ (makeArrayIterator):
+ (test.bar):
+ (test.callback):
+ (test.arr.__proto__.Symbol.iterator):
+ (test.arr.Symbol.iterator):
+ (test.get bar):
+ (test.hackedNext):
+ (test.test):
+ (test.):
+
2017-04-25 Mark Lam <mark....@apple.com>
[Follow up] Array.prototype.slice() should ensure that end >= begin.
Added: trunk/JSTests/stress/spread-optimized-properly.js (0 => 215777)
--- trunk/JSTests/stress/spread-optimized-properly.js (rev 0)
+++ trunk/JSTests/stress/spread-optimized-properly.js 2017-04-26 00:52:35 UTC (rev 215777)
@@ -0,0 +1,155 @@
+function assert(b) {
+ if (!b)
+ throw new Error("Bad assertion");
+}
+
+function test(f) { f(); }
+
+function shallowEq(a, b) {
+ if (a.length !== b.length)
+ return false;
+ for (let i = 0; i < a.length; ++i) {
+ if (a[i] !== b[i])
+ return false;
+ }
+ return true;
+}
+
+function makeArrayIterator(arr, f) {
+ let i = 0;
+ return {
+ next() {
+ f();
+ if (i >= arr.length)
+ return {done: true};
+ return {value: arr[i++], done: false};
+ }
+ };
+}
+
+test(function() {
+ let arr = [10, 20];
+ arr.__proto__ = {[Symbol.iterator]: Array.prototype[Symbol.iterator]};
+ function bar(a) {
+ a.x;
+ return [...a];
+ }
+ noInline(bar);
+ for (let i = 0; i < 10000; i++) {
+ assert(shallowEq(bar(arr), arr));
+ }
+});
+
+test(function() {
+ let arr = [10, 20];
+ let count = 0;
+ function callback() {
+ count++;
+ }
+
+ arr.__proto__ = {
+ [Symbol.iterator]: function() {
+ return makeArrayIterator(this, callback);
+ }
+ };
+
+ function bar(a) {
+ a.x;
+ return [...a];
+ }
+ noInline(bar);
+ for (let i = 0; i < 10000; i++) {
+ let t = bar(arr);
+ assert(count === 3);
+ count = 0;
+ assert(shallowEq(t, arr));
+ }
+});
+
+test(function() {
+ let arr = [10, 20];
+ let count = 0;
+ function callback() {
+ count++;
+ }
+
+ arr[Symbol.iterator] = function() {
+ return makeArrayIterator(this, callback);
+ };
+
+ function bar(a) {
+ a.x;
+ return [...a];
+ }
+ noInline(bar);
+ for (let i = 0; i < 10000; i++) {
+ let t = bar(arr);
+ assert(count === 3);
+ count = 0;
+ assert(shallowEq(t, arr));
+ }
+});
+
+test(function() {
+ let arr = [10, 20];
+ arr[Symbol.iterator] = Array.prototype[Symbol.iterator];
+ function bar(a) {
+ a.x;
+ return [...a];
+ }
+ noInline(bar);
+ for (let i = 0; i < 10000; i++) {
+ assert(shallowEq(bar(arr), arr));
+ }
+});
+
+test(function() {
+ let arr = [, 20];
+ let callCount = 0;
+ Object.defineProperty(arr, 0, {get() { ++callCount; return 10; }});
+ function bar(a) {
+ a.x;
+ return [...a];
+ }
+ noInline(bar);
+ for (let i = 0; i < 10000; i++) {
+ let t = bar(arr);
+ assert(callCount === 1);
+ assert(shallowEq(t, arr));
+ assert(callCount === 2);
+ callCount = 0;
+ }
+});
+
+// We run this test last since it fires watchpoints for the protocol.
+test(function() {
+ let iter = [][Symbol.iterator]();
+ let iterProto = Object.getPrototypeOf(iter);
+ let oldNext = iterProto.next;
+
+ function hackedNext() {
+ let val = oldNext.call(this);
+ if ("value" in val) {
+ val.value++;
+ }
+ return val;
+ }
+
+ function test(a) {
+ a.x;
+ return [...a];
+ }
+
+ for (let i = 0; i < 10000; ++i) {
+ let arr = [1,,3];
+ let callCount = 0;
+ Object.defineProperty(arr, 1, { get: function() { ++callCount; iterProto.next = hackedNext; return 2; } });
+ let t = test(arr);
+ assert(callCount === 1);
+ assert(t.length === 3);
+ assert(t[0] === 1);
+ assert(t[1] === 2);
+ assert(t[2] === 4);
+ iterProto.next = oldNext;
+ }
+});
Modified: trunk/LayoutTests/ChangeLog (215776 => 215777)
--- trunk/LayoutTests/ChangeLog 2017-04-26 00:42:32 UTC (rev 215776)
+++ trunk/LayoutTests/ChangeLog 2017-04-26 00:52:35 UTC (rev 215777)
@@ -1,3 +1,16 @@
+2017-04-25 Saam Barati <sbar...@apple.com>
+
+ JSArray::isArrayPrototypeIteratorProtocolFastAndNonObservable is wrong because it does not do the necessary checks on the base object
+ https://bugs.webkit.org/show_bug.cgi?id=171150
+ <rdar://problem/31771880>
+
+ Reviewed by Sam Weinig.
+
+ * js/sequence-iterator-protocol-2-expected.txt: Added.
+ * js/sequence-iterator-protocol-2.html: Added.
+ * js/sequence-iterator-protocol-expected.txt: Added.
+ * js/sequence-iterator-protocol.html: Added.
+
2017-04-25 Ryan Haddad <ryanhad...@apple.com>
Mark media/modern-media-controls/pip-support/pip-support-click.html as flaky.
Added: trunk/LayoutTests/js/sequence-iterator-protocol-2-expected.txt (0 => 215777)
--- trunk/LayoutTests/js/sequence-iterator-protocol-2-expected.txt (rev 0)
+++ trunk/LayoutTests/js/sequence-iterator-protocol-2-expected.txt 2017-04-26 00:52:35 UTC (rev 215777)
@@ -0,0 +1,11 @@
+CONSOLE MESSAGE: line 28: Using an array but with objects with valueOf()
+CONSOLE MESSAGE: line 29: 1,2,3,1,2,3
+CONSOLE MESSAGE: line 33: 1,2,3,1,2,3
+CONSOLE MESSAGE: line 38: callCount: 3
+CONSOLE MESSAGE: line 66: 1,2,4,1,2,4
+CONSOLE MESSAGE: line 71: 1,2,4,1,2,4
+CONSOLE MESSAGE: line 75: callCount: 2
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/js/sequence-iterator-protocol-2.html (0 => 215777)
--- trunk/LayoutTests/js/sequence-iterator-protocol-2.html (rev 0)
+++ trunk/LayoutTests/js/sequence-iterator-protocol-2.html 2017-04-26 00:52:35 UTC (rev 215777)
@@ -0,0 +1,84 @@
+<!DOCTYPE html>
+<html>
+ <head>
+ <meta charset="utf-8">
+ <script src=""
+ </head>
+ <body>
+ <pre id='console'></pre>
+ <canvas></canvas>
+
+ <script>
+ "use strict";
+
+ const log = console.log.bind(console);
+ const canvas = document.querySelector("canvas");
+ const ctx = canvas.getContext("2d");
+
+ let callCount = 0;
+ function makeValue(x) {
+ return {
+ valueOf() { ++callCount; return x; }
+ };
+ }
+
+ let array = [makeValue(1), makeValue(2), makeValue(3)];
+ ctx.setLineDash(array);
+ let a = ctx.getLineDash();
+ log("Using an array but with objects with valueOf()");
+ log(a);
+
+ ctx.setLineDash([1,2,3]);
+ let b = ctx.getLineDash();
+ log(b);
+
+ if (a.toString() !== b.toString())
+ throw new Error("Bad result. They should be equal.");
+
+ log(`callCount: ${callCount}`);
+ if (callCount !== 3)
+ throw new Error("Bad result. callCount should be 3.");
+
+
+ let iter = [][Symbol.iterator]();
+ let iterProto = Object.getPrototypeOf(iter);
+ let oldNext = iterProto.next;
+
+ callCount = 0;
+ function hackedNext() {
+ ++callCount;
+
+ let val = oldNext.call(this);
+ if ("value" in val) {
+ val.value++;
+ }
+ return val;
+ }
+ const obj = {
+ valueOf: function() {
+ iterProto.next = hackedNext;
+ return 2;
+ }
+ };
+ array = [1, obj, 3];
+ ctx.setLineDash(array);
+ b = ctx.getLineDash();
+ log(`${b}`);
+
+ iterProto.next = oldNext;
+ ctx.setLineDash([1,2,4]);
+ a = ctx.getLineDash();
+ log(a);
+ if (a.toString() !== b.toString())
+ throw new Error("Bad result. They should be equal.");
+
+ log(`callCount: ${callCount}`);
+ if (callCount !== 2)
+ throw new Error("Bad result. callCount should be 2.");
+
+ </script>
+
+ <script src=""
+
+ </body>
+</html>
Added: trunk/LayoutTests/js/sequence-iterator-protocol-expected.txt (0 => 215777)
--- trunk/LayoutTests/js/sequence-iterator-protocol-expected.txt (rev 0)
+++ trunk/LayoutTests/js/sequence-iterator-protocol-expected.txt 2017-04-26 00:52:35 UTC (rev 215777)
@@ -0,0 +1,10 @@
+CONSOLE MESSAGE: line 19:
+CONSOLE MESSAGE: line 22: Using an array: 1,2,3,1,2,3
+CONSOLE MESSAGE: line 33: Using an iterator: 10,11,12,10,11,12
+CONSOLE MESSAGE: line 41: Using an array but with customized iteration (should be same as iterator): 10,11,12,10,11,12
+The second and third results should be the same.
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/js/sequence-iterator-protocol.html (0 => 215777)
--- trunk/LayoutTests/js/sequence-iterator-protocol.html (rev 0)
+++ trunk/LayoutTests/js/sequence-iterator-protocol.html 2017-04-26 00:52:35 UTC (rev 215777)
@@ -0,0 +1,51 @@
+<!DOCTYPE html>
+<html>
+ <head>
+ <meta charset="utf-8">
+ <script src=""
+ </head>
+ <body>
+ <p>The second and third results should be the same.</p>
+ <pre id='console'></pre>
+ <canvas></canvas>
+
+ <script>
+ "use strict";
+
+ const log = console.log.bind(console);
+
+ const canvas = document.querySelector("canvas");
+ const ctx = canvas.getContext("2d");
+ log(ctx.getLineDash());
+
+ ctx.setLineDash([1, 2, 3]);
+ log("Using an array: " + ctx.getLineDash());
+
+ function* generator() {
+ yield 10;
+ yield 11;
+ yield 12;
+ }
+
+ const iterator = generator();
+ ctx.setLineDash(iterator);
+ let a = ctx.getLineDash();
+ log("Using an iterator: " + a);
+
+ const array = [1, 2, 3];
+ Object.defineProperty(array, Symbol.iterator, {
+ value: () => generator()
+ });
+ ctx.setLineDash(array);
+ let b = ctx.getLineDash();
+ log("Using an array but with customized iteration (should be same as iterator): " + b);
+
+ if (a.toString() !== b.toString())
+ throw new Error("Bad result. They should be equal.");
+
+ </script>
+
+ <script src=""
+
+ </body>
+</html>
Modified: trunk/Source/_javascript_Core/ChangeLog (215776 => 215777)
--- trunk/Source/_javascript_Core/ChangeLog 2017-04-26 00:42:32 UTC (rev 215776)
+++ trunk/Source/_javascript_Core/ChangeLog 2017-04-26 00:52:35 UTC (rev 215777)
@@ -1,3 +1,43 @@
+2017-04-25 Saam Barati <sbar...@apple.com>
+
+ JSArray::isArrayPrototypeIteratorProtocolFastAndNonObservable is wrong because it does not do the necessary checks on the base object
+ https://bugs.webkit.org/show_bug.cgi?id=171150
+ <rdar://problem/31771880>
+
+ Reviewed by Sam Weinig.
+
+ This patch fixes a huge oversight from the patch that introduced
+ op_spread/Spread. The original patch did not account for the
+ base object having Symbol.iterator or getters that could
+ change the iterator protocol. This patch fixes the oversight both
+ in the C code, as well as the DFG/FTL backends. We only perform
+ the memcpy version of spread if we've proven that it's guaranteed
+ to be side-effect free (no indexed getters), and if the iterator
+ protocol is guaranteed to be the original protocol. To do this, we
+ must prove that:
+ 1. The protocol on Array.prototype hasn't changed (this is the same as the
+ introductory patch for op_spread).
+ 2. The base object's __proto__ is Array.prototype
+ 3. The base object does not have indexed getters
+ 4. The base object does not have Symbol.iterator property.
+
+ * dfg/DFGGraph.cpp:
+ (JSC::DFG::Graph::canDoFastSpread):
+ * dfg/DFGGraph.h:
+ * dfg/DFGSpeculativeJIT.cpp:
+ (JSC::DFG::SpeculativeJIT::compileSpread):
+ * ftl/FTLLowerDFGToB3.cpp:
+ (JSC::FTL::DFG::LowerDFGToB3::compileSpread):
+ * runtime/JSArray.cpp:
+ (JSC::JSArray::isIteratorProtocolFastAndNonObservable):
+ * runtime/JSArray.h:
+ * runtime/JSArrayInlines.h:
+ (JSC::JSArray::isIteratorProtocolFastAndNonObservable): Deleted.
+ * runtime/JSGlobalObject.h:
+ * runtime/JSGlobalObjectInlines.h:
+ (JSC::JSGlobalObject::isArrayPrototypeIteratorProtocolFastAndNonObservable):
+ (JSC::JSGlobalObject::isArrayIteratorProtocolFastAndNonObservable): Deleted.
+
2017-04-25 Mark Lam <mark....@apple.com>
Array.prototype.slice() should ensure that end >= begin.
Modified: trunk/Source/_javascript_Core/dfg/DFGGraph.cpp (215776 => 215777)
--- trunk/Source/_javascript_Core/dfg/DFGGraph.cpp 2017-04-26 00:42:32 UTC (rev 215776)
+++ trunk/Source/_javascript_Core/dfg/DFGGraph.cpp 2017-04-26 00:52:35 UTC (rev 215777)
@@ -1723,6 +1723,37 @@
RELEASE_ASSERT_NOT_REACHED();
}
+bool Graph::canDoFastSpread(Node* node, const AbstractValue& value)
+{
+ // The parameter 'value' is the AbstractValue for child1 (the thing being spread).
+ ASSERT(node->op() == Spread);
+
+ if (node->child1().useKind() != ArrayUse) {
+ // Note: we only speculate on ArrayUse when we've set up the necessary watchpoints
+ // to prove that the iteration protocol is non-observable starting from ArrayPrototype.
+ return false;
+ }
+
+ // FIXME: We should add profiling of the incoming operand to Spread
+ // so we can speculate in such a way that we guarantee that this
+ // function would return true:
+ // https://bugs.webkit.org/show_bug.cgi?id=171198
+
+ if (!value.m_structure.isFinite())
+ return false;
+
+ ArrayPrototype* arrayPrototype = globalObjectFor(node->child1()->origin.semantic)->arrayPrototype();
+ bool allGood = true;
+ value.m_structure.forEach([&] (RegisteredStructure structure) {
+ allGood &= structure->storedPrototype() == arrayPrototype
+ && !structure->isDictionary()
+ && structure->getConcurrently(m_vm.propertyNames->iteratorSymbol.impl()) == invalidOffset
+ && !structure->mayInterceptIndexedAccesses();
+ });
+
+ return allGood;
+}
+
} } // namespace JSC::DFG
#endif // ENABLE(DFG_JIT)
Modified: trunk/Source/_javascript_Core/dfg/DFGGraph.h (215776 => 215777)
--- trunk/Source/_javascript_Core/dfg/DFGGraph.h 2017-04-26 00:42:32 UTC (rev 215776)
+++ trunk/Source/_javascript_Core/dfg/DFGGraph.h 2017-04-26 00:52:35 UTC (rev 215777)
@@ -884,6 +884,8 @@
JSArrayBufferView* tryGetFoldableView(JSValue);
JSArrayBufferView* tryGetFoldableView(JSValue, ArrayMode arrayMode);
+
+ bool canDoFastSpread(Node*, const AbstractValue&);
void registerFrozenValues();
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (215776 => 215777)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2017-04-26 00:42:32 UTC (rev 215776)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2017-04-26 00:52:35 UTC (rev 215777)
@@ -7033,11 +7033,10 @@
SpeculateCellOperand operand(this, node->child1());
GPRReg argument = operand.gpr();
- if (node->child1().useKind() == ArrayUse) {
- // Note: we only speculate on ArrayUse when we've set up the necessary watchpoints
- // to prove that the iteration protocol is non-observable.
+ if (node->child1().useKind() == ArrayUse)
speculateArray(node->child1(), argument);
+ if (m_jit.graph().canDoFastSpread(node, m_state.forNode(node->child1()))) {
#if USE(JSVALUE64)
GPRTemporary result(this);
GPRTemporary scratch1(this);
Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (215776 => 215777)
--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2017-04-26 00:42:32 UTC (rev 215776)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2017-04-26 00:52:35 UTC (rev 215777)
@@ -4676,9 +4676,11 @@
LValue argument = lowCell(m_node->child1());
LValue result;
- if (m_node->child1().useKind() == ArrayUse) {
+
+ if (m_node->child1().useKind() == ArrayUse)
speculateArray(m_node->child1());
+ if (m_graph.canDoFastSpread(m_node, m_state.forNode(m_node->child1()))) {
LBasicBlock preLoop = m_out.newBlock();
LBasicBlock loopSelection = m_out.newBlock();
LBasicBlock contiguousLoopStart = m_out.newBlock();
Modified: trunk/Source/_javascript_Core/runtime/JSArray.cpp (215776 => 215777)
--- trunk/Source/_javascript_Core/runtime/JSArray.cpp 2017-04-26 00:42:32 UTC (rev 215776)
+++ trunk/Source/_javascript_Core/runtime/JSArray.cpp 2017-04-26 00:52:35 UTC (rev 215777)
@@ -1403,4 +1403,27 @@
}
}
+bool JSArray::isIteratorProtocolFastAndNonObservable()
+{
+ JSGlobalObject* globalObject = this->globalObject();
+ if (!globalObject->isArrayPrototypeIteratorProtocolFastAndNonObservable())
+ return false;
+
+ Structure* structure = this->structure();
+ // This is the fast case. Many arrays will be an original array.
+ if (globalObject->isOriginalArrayStructure(structure))
+ return true;
+
+ if (structure->mayInterceptIndexedAccesses())
+ return false;
+
+ if (structure->storedPrototype() != globalObject->arrayPrototype())
+ return false;
+
+ if (getDirectOffset(globalObject->vm(), globalObject->vm().propertyNames->iteratorSymbol) != invalidOffset)
+ return false;
+
+ return true;
+}
+
} // namespace JSC
Modified: trunk/Source/_javascript_Core/runtime/JSArray.h (215776 => 215777)
--- trunk/Source/_javascript_Core/runtime/JSArray.h 2017-04-26 00:42:32 UTC (rev 215776)
+++ trunk/Source/_javascript_Core/runtime/JSArray.h 2017-04-26 00:52:35 UTC (rev 215777)
@@ -150,7 +150,7 @@
JS_EXPORT_PRIVATE void fillArgList(ExecState*, MarkedArgumentBuffer&);
JS_EXPORT_PRIVATE void copyToArguments(ExecState*, VirtualRegister firstElementDest, unsigned offset, unsigned length);
- bool isIteratorProtocolFastAndNonObservable();
+ JS_EXPORT_PRIVATE bool isIteratorProtocolFastAndNonObservable();
static Structure* createStructure(VM& vm, JSGlobalObject* globalObject, JSValue prototype, IndexingType indexingType)
{
Modified: trunk/Source/_javascript_Core/runtime/JSArrayInlines.h (215776 => 215777)
--- trunk/Source/_javascript_Core/runtime/JSArrayInlines.h 2017-04-26 00:42:32 UTC (rev 215776)
+++ trunk/Source/_javascript_Core/runtime/JSArrayInlines.h 2017-04-26 00:52:35 UTC (rev 215777)
@@ -93,9 +93,4 @@
return lengthValue.toLength(exec);
}
-ALWAYS_INLINE bool JSArray::isIteratorProtocolFastAndNonObservable()
-{
- return globalObject()->isArrayIteratorProtocolFastAndNonObservable();
-}
-
} // namespace JSC
Modified: trunk/Source/_javascript_Core/runtime/JSGlobalObject.h (215776 => 215777)
--- trunk/Source/_javascript_Core/runtime/JSGlobalObject.h 2017-04-26 00:42:32 UTC (rev 215776)
+++ trunk/Source/_javascript_Core/runtime/JSGlobalObject.h 2017-04-26 00:52:35 UTC (rev 215777)
@@ -412,7 +412,7 @@
std::unique_ptr<ArrayIteratorAdaptiveWatchpoint> m_arrayPrototypeSymbolIteratorWatchpoint;
std::unique_ptr<ArrayIteratorAdaptiveWatchpoint> m_arrayIteratorPrototypeNext;
- bool isArrayIteratorProtocolFastAndNonObservable();
+ bool isArrayPrototypeIteratorProtocolFastAndNonObservable();
TemplateRegistry m_templateRegistry;
Modified: trunk/Source/_javascript_Core/runtime/JSGlobalObjectInlines.h (215776 => 215777)
--- trunk/Source/_javascript_Core/runtime/JSGlobalObjectInlines.h 2017-04-26 00:42:32 UTC (rev 215776)
+++ trunk/Source/_javascript_Core/runtime/JSGlobalObjectInlines.h 2017-04-26 00:52:35 UTC (rev 215777)
@@ -53,7 +53,7 @@
}
-ALWAYS_INLINE bool JSGlobalObject::isArrayIteratorProtocolFastAndNonObservable()
+ALWAYS_INLINE bool JSGlobalObject::isArrayPrototypeIteratorProtocolFastAndNonObservable()
{
// We're fast if we don't have any indexed properties on the prototype.
// We're non-observable if the iteration protocol hasn't changed.
Modified: trunk/Source/WebCore/ChangeLog (215776 => 215777)
--- trunk/Source/WebCore/ChangeLog 2017-04-26 00:42:32 UTC (rev 215776)
+++ trunk/Source/WebCore/ChangeLog 2017-04-26 00:52:35 UTC (rev 215777)
@@ -1,3 +1,40 @@
+2017-04-25 Saam Barati <sbar...@apple.com>
+
+ JSArray::isArrayPrototypeIteratorProtocolFastAndNonObservable is wrong because it does not do the necessary checks on the base object
+ https://bugs.webkit.org/show_bug.cgi?id=171150
+ <rdar://problem/31771880>
+
+ Reviewed by Sam Weinig.
+
+ This patch moves the sequence converters to use the now fixed
+ JSArray::isArrayPrototypeIteratorProtocolFastAndNonObservable test
+ inside JSC.
+
+ This patch also fixes a few bugs:
+ 1. Converting to a sequence of numbers must prove that the JSArray
+ is filled only with Int32/Double. If there is a chance the array
+ contains objects, the conversion to a numeric IDLType can be observable
+ (via valueOf()), and can change the iterator protocol.
+ 2. There are other conversions that can have side effects a-la valueOf().
+ This patch introduces a new static constant in the various Converter
+ classes that tell the sequence converter if the conversion operation
+ can have JS side effects. If it does have side effects, we fall back to
+ the generic conversion that uses the iterator protocol. If not, we can
+ do a faster version that iterates over each element of the array,
+ reading it directly, and converting it.
+
+ Tests: js/sequence-iterator-protocol-2.html
+ js/sequence-iterator-protocol.html
+
+ * bindings/js/JSDOMConvertAny.h: Does not have side effects.
+ * bindings/js/JSDOMConvertBase.h: We pessimistically assume inside DefaultConverter that converions have side effects.
+ * bindings/js/JSDOMConvertBoolean.h: Does not have side effects.
+ * bindings/js/JSDOMConvertCallbacks.h: Does not have side effects.
+ * bindings/js/JSDOMConvertObject.h: Does not have side effects.
+ * bindings/js/JSDOMConvertSequences.h:
+ (WebCore::Detail::NumericSequenceConverter::convert):
+ (WebCore::Detail::SequenceConverter::convert):
+
2017-04-25 Michael Saboff <msab...@apple.com>
Call bmalloc scavenger first when handling a memory pressure event
Modified: trunk/Source/WebCore/bindings/js/JSDOMConvertAny.h (215776 => 215777)
--- trunk/Source/WebCore/bindings/js/JSDOMConvertAny.h 2017-04-26 00:42:32 UTC (rev 215776)
+++ trunk/Source/WebCore/bindings/js/JSDOMConvertAny.h 2017-04-26 00:52:35 UTC (rev 215777)
@@ -32,7 +32,9 @@
template<> struct Converter<IDLAny> : DefaultConverter<IDLAny> {
using ReturnType = JSC::JSValue;
-
+
+ static constexpr bool conversionHasSideEffects = false;
+
static JSC::JSValue convert(JSC::ExecState&, JSC::JSValue value)
{
return value;
Modified: trunk/Source/WebCore/bindings/js/JSDOMConvertBase.h (215776 => 215777)
--- trunk/Source/WebCore/bindings/js/JSDOMConvertBase.h 2017-04-26 00:42:32 UTC (rev 215776)
+++ trunk/Source/WebCore/bindings/js/JSDOMConvertBase.h 2017-04-26 00:52:35 UTC (rev 215777)
@@ -178,6 +178,16 @@
template<typename T> struct DefaultConverter {
using ReturnType = typename T::ImplementationType;
+
+ // We assume the worst, subtypes can override to be less pessimistic.
+ // An example of something that can have side effects
+ // is having a converter that does JSC::JSValue::toNumber.
+ // toNumber() in _javascript_ can call arbitrary JS functions.
+ //
+ // An example of something that does not have side effects
+ // is something having a converter that does JSC::JSValue::toBoolean.
+ // toBoolean() in JS can't call arbitrary functions.
+ static constexpr bool conversionHasSideEffects = true;
};
} // namespace WebCore
Modified: trunk/Source/WebCore/bindings/js/JSDOMConvertBoolean.h (215776 => 215777)
--- trunk/Source/WebCore/bindings/js/JSDOMConvertBoolean.h 2017-04-26 00:42:32 UTC (rev 215776)
+++ trunk/Source/WebCore/bindings/js/JSDOMConvertBoolean.h 2017-04-26 00:52:35 UTC (rev 215777)
@@ -31,6 +31,9 @@
namespace WebCore {
template<> struct Converter<IDLBoolean> : DefaultConverter<IDLBoolean> {
+
+ static constexpr bool conversionHasSideEffects = false;
+
static bool convert(JSC::ExecState& state, JSC::JSValue value)
{
return value.toBoolean(&state);
Modified: trunk/Source/WebCore/bindings/js/JSDOMConvertCallbacks.h (215776 => 215777)
--- trunk/Source/WebCore/bindings/js/JSDOMConvertCallbacks.h 2017-04-26 00:42:32 UTC (rev 215776)
+++ trunk/Source/WebCore/bindings/js/JSDOMConvertCallbacks.h 2017-04-26 00:52:35 UTC (rev 215777)
@@ -31,6 +31,9 @@
namespace WebCore {
template<typename T> struct Converter<IDLCallbackFunction<T>> : DefaultConverter<IDLCallbackFunction<T>> {
+
+ static constexpr bool conversionHasSideEffects = false;
+
template<typename ExceptionThrower = DefaultExceptionThrower>
static RefPtr<T> convert(JSC::ExecState& state, JSC::JSValue value, JSDOMGlobalObject& globalObject, ExceptionThrower&& exceptionThrower = ExceptionThrower())
{
Modified: trunk/Source/WebCore/bindings/js/JSDOMConvertObject.h (215776 => 215777)
--- trunk/Source/WebCore/bindings/js/JSDOMConvertObject.h 2017-04-26 00:42:32 UTC (rev 215776)
+++ trunk/Source/WebCore/bindings/js/JSDOMConvertObject.h 2017-04-26 00:52:35 UTC (rev 215777)
@@ -31,6 +31,9 @@
namespace WebCore {
template<> struct Converter<IDLObject> : DefaultConverter<IDLObject> {
+
+ static constexpr bool conversionHasSideEffects = false;
+
template<typename ExceptionThrower = DefaultExceptionThrower>
static JSC::Strong<JSC::JSObject> convert(JSC::ExecState& state, JSC::JSValue value, ExceptionThrower&& exceptionThrower = ExceptionThrower())
{
Modified: trunk/Source/WebCore/bindings/js/JSDOMConvertSequences.h (215776 => 215777)
--- trunk/Source/WebCore/bindings/js/JSDOMConvertSequences.h 2017-04-26 00:42:32 UTC (rev 215776)
+++ trunk/Source/WebCore/bindings/js/JSDOMConvertSequences.h 2017-04-26 00:52:35 UTC (rev 215777)
@@ -41,7 +41,11 @@
static ReturnType convert(JSC::ExecState& state, JSC::JSObject* jsObject)
{
- ReturnType result;
+ return convert(state, jsObject, ReturnType());
+ }
+
+ static ReturnType convert(JSC::ExecState& state, JSC::JSObject* jsObject, ReturnType&& result)
+ {
forEachInIterable(&state, jsObject, [&result](JSC::VM& vm, JSC::ExecState* state, JSC::JSValue jsValue) {
auto scope = DECLARE_THROW_SCOPE(vm);
@@ -78,35 +82,27 @@
return GenericConverter::convert(state, object);
JSC::JSArray* array = JSC::asArray(object);
- if (!array->globalObject()->isArrayIteratorProtocolFastAndNonObservable())
+ if (!array->isIteratorProtocolFastAndNonObservable())
return GenericConverter::convert(state, object);
unsigned length = array->length();
-
ReturnType result;
+ // If we're not an int32/double array, it's possible that converting a
+ // JSValue to a number could cause the iterator protocol to change, hence,
+ // we may need more capacity, or less. In such cases, we use the length
+ // as a proxy for the capacity we will most likely need (it's unlikely that
+ // a program is written with a valueOf that will augment the iterator protocol).
+ // If we are an int32/double array, then length is precisely the capacity we need.
if (!result.tryReserveCapacity(length)) {
// FIXME: Is the right exception to throw?
throwTypeError(&state, scope);
return { };
}
-
+
JSC::IndexingType indexingType = array->indexingType() & JSC::IndexingShapeMask;
+ if (indexingType != JSC::Int32Shape && indexingType != JSC::DoubleShape)
+ return GenericConverter::convert(state, object, WTFMove(result));
- if (indexingType == JSC::ContiguousShape) {
- for (unsigned i = 0; i < length; i++) {
- auto indexValue = array->butterfly()->contiguous()[i].get();
- if (!indexValue)
- result.uncheckedAppend(0);
- else {
- auto convertedValue = Converter<IDLType>::convert(state, indexValue);
- RETURN_IF_EXCEPTION(scope, { });
-
- result.uncheckedAppend(convertedValue);
- }
- }
- return result;
- }
-
if (indexingType == JSC::Int32Shape) {
for (unsigned i = 0; i < length; i++) {
auto indexValue = array->butterfly()->contiguousInt32()[i].get();
@@ -119,31 +115,15 @@
return result;
}
- if (indexingType == JSC::DoubleShape) {
- for (unsigned i = 0; i < length; i++) {
- auto doubleValue = array->butterfly()->contiguousDouble()[i];
- if (std::isnan(doubleValue))
- result.uncheckedAppend(0);
- else {
- auto convertedValue = Converter<IDLType>::convert(state, scope, doubleValue);
- RETURN_IF_EXCEPTION(scope, { });
-
- result.uncheckedAppend(convertedValue);
- }
- }
- return result;
- }
-
+ ASSERT(indexingType == JSC::DoubleShape);
for (unsigned i = 0; i < length; i++) {
- auto indexValue = array->getDirectIndex(&state, i);
- RETURN_IF_EXCEPTION(scope, { });
-
- if (!indexValue)
+ auto doubleValue = array->butterfly()->contiguousDouble()[i];
+ if (std::isnan(doubleValue))
result.uncheckedAppend(0);
else {
- auto convertedValue = Converter<IDLType>::convert(state, indexValue);
+ auto convertedValue = Converter<IDLType>::convert(state, scope, doubleValue);
RETURN_IF_EXCEPTION(scope, { });
-
+
result.uncheckedAppend(convertedValue);
}
}
@@ -167,11 +147,14 @@
}
JSC::JSObject* object = JSC::asObject(value);
+ if (Converter<IDLType>::conversionHasSideEffects)
+ return GenericConverter::convert(state, object);
+
if (!JSC::isJSArray(object))
return GenericConverter::convert(state, object);
JSC::JSArray* array = JSC::asArray(object);
- if (!array->globalObject()->isArrayIteratorProtocolFastAndNonObservable())
+ if (!array->isIteratorProtocolFastAndNonObservable())
return GenericConverter::convert(state, object);
unsigned length = array->length();
