Title: [200678] trunk
Revision
200678
Author
youenn.fab...@crf.canon.fr
Date
2016-05-11 00:50:49 -0700 (Wed, 11 May 2016)

Log Message

Ensure DOM iterators remain done
https://bugs.webkit.org/show_bug.cgi?id=157453

Reviewed by Darin Adler.

Source/WebCore:

Covered by updated test.

Making DOMWrapped::Iterator an Optional.
Setting it to Nullopt on the first time Iterator is returning null.

For set iterators, incrementing a counter which value is used in forEach callbacks and entries iterators.

* bindings/js/JSDOMIterator.h:
(WebCore::JSDOMIterator<JSWrapper>::asJS):
(WebCore::appendForEachArguments):
(WebCore::iteratorForEach):
(WebCore::JSDOMIterator<JSWrapper>::next):

LayoutTests:

* fast/dom/nodeListIterator-expected.txt:
* fast/text/font-face-set-_javascript_-expected.txt:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (200677 => 200678)


--- trunk/LayoutTests/ChangeLog	2016-05-11 07:14:31 UTC (rev 200677)
+++ trunk/LayoutTests/ChangeLog	2016-05-11 07:50:49 UTC (rev 200678)
@@ -1,3 +1,13 @@
+2016-05-11  Youenn Fablet  <youenn.fab...@crf.canon.fr>
+
+        Ensure DOM iterators remain done
+        https://bugs.webkit.org/show_bug.cgi?id=157453
+
+        Reviewed by Darin Adler.
+
+        * fast/dom/nodeListIterator-expected.txt:
+        * fast/text/font-face-set-_javascript_-expected.txt:
+
 2016-05-11  Joanmarie Diggs  <jdi...@igalia.com>
 
         [GTK] accessibility/aria-readonly.html is failing

Modified: trunk/LayoutTests/fast/dom/nodeListIterator-expected.txt (200677 => 200678)


--- trunk/LayoutTests/fast/dom/nodeListIterator-expected.txt	2016-05-11 07:14:31 UTC (rev 200677)
+++ trunk/LayoutTests/fast/dom/nodeListIterator-expected.txt	2016-05-11 07:50:49 UTC (rev 200678)
@@ -9,10 +9,10 @@
 PASS pair[0] is children[0]
 PASS pair[1] is children[1]
 PASS forEachContainer is nodeList
-FAIL forEachIndex should be 0 (of type number). Was [object HTMLDivElement] (of type object).
+PASS forEachIndex is index
 PASS node is children[index++]
 PASS forEachContainer is nodeList
-FAIL forEachIndex should be 1 (of type number). Was [object HTMLOListElement] (of type object).
+PASS forEachIndex is index
 PASS node is children[index++]
 PASS iterator.next().value is children[0]
 PASS iterator.next().value is children[1]
@@ -23,15 +23,15 @@
 PASS end.done is true
 PASS end.value is undefined
 PASS pair.length is 2
-FAIL pair[0] should be 0 (of type number). Was [object HTMLDivElement] (of type object).
+PASS pair[0] is 0
 PASS pair[1] is children[0]
 PASS pair.length is 2
-FAIL pair[0] should be 1 (of type number). Was [object HTMLOListElement] (of type object).
+PASS pair[0] is 1
 PASS pair[1] is children[1]
 PASS end.done is true
 PASS end.value is undefined
-FAIL end.done should be true. Was false.
-FAIL end.value should be undefined (of type undefined). Was [object HTMLParagraphElement],[object HTMLParagraphElement] (of type object).
+PASS end.done is true
+PASS end.value is undefined
 PASS successfullyParsed is true
 
 TEST COMPLETE

Modified: trunk/LayoutTests/fast/text/font-face-set-_javascript_-expected.txt (200677 => 200678)


--- trunk/LayoutTests/fast/text/font-face-set-_javascript_-expected.txt	2016-05-11 07:14:31 UTC (rev 200677)
+++ trunk/LayoutTests/fast/text/font-face-set-_javascript_-expected.txt	2016-05-11 07:50:49 UTC (rev 200678)
@@ -4,7 +4,7 @@
 PASS fontFaceSet.status is "loaded"
 PASS item.done is false
 PASS item.value.length is 2
-FAIL item.value[0] should be 0 (of type number). Was [object FontFace] (of type object).
+PASS item.value[0] is 0
 PASS item.value[1] is fontFace1
 PASS item.done is true
 PASS item.value is undefined

Modified: trunk/Source/WebCore/ChangeLog (200677 => 200678)


--- trunk/Source/WebCore/ChangeLog	2016-05-11 07:14:31 UTC (rev 200677)
+++ trunk/Source/WebCore/ChangeLog	2016-05-11 07:50:49 UTC (rev 200678)
@@ -1,3 +1,23 @@
+2016-05-11  Youenn Fablet  <youenn.fab...@crf.canon.fr>
+
+        Ensure DOM iterators remain done
+        https://bugs.webkit.org/show_bug.cgi?id=157453
+
+        Reviewed by Darin Adler.
+
+        Covered by updated test.
+
+        Making DOMWrapped::Iterator an Optional.
+        Setting it to Nullopt on the first time Iterator is returning null.
+
+        For set iterators, incrementing a counter which value is used in forEach callbacks and entries iterators.
+
+        * bindings/js/JSDOMIterator.h:
+        (WebCore::JSDOMIterator<JSWrapper>::asJS):
+        (WebCore::appendForEachArguments):
+        (WebCore::iteratorForEach):
+        (WebCore::JSDOMIterator<JSWrapper>::next):
+
 2016-05-11  Joanmarie Diggs  <jdi...@igalia.com>
 
         [GTK] accessibility/aria-readonly.html is failing

Modified: trunk/Source/WebCore/bindings/js/JSDOMIterator.h (200677 => 200678)


--- trunk/Source/WebCore/bindings/js/JSDOMIterator.h	2016-05-11 07:14:31 UTC (rev 200677)
+++ trunk/Source/WebCore/bindings/js/JSDOMIterator.h	2016-05-11 07:50:49 UTC (rev 200678)
@@ -108,10 +108,16 @@
     {
     }
 
+    template<typename IteratorValue> typename std::enable_if<IteratorInspector<IteratorValue>::isMap, JSC::JSValue>::type
+    asJS(JSC::ExecState&, IteratorValue&);
+    template<typename IteratorValue> typename std::enable_if<IteratorInspector<IteratorValue>::isSet, JSC::JSValue>::type
+    asJS(JSC::ExecState&, IteratorValue&);
+
     static void destroy(JSC::JSCell*);
 
-    typename DOMWrapped::Iterator m_iterator;
+    Optional<typename DOMWrapped::Iterator> m_iterator;
     IterationKind m_kind;
+    size_t m_index { 0 };
 };
 
 template<typename JSWrapper>
@@ -129,30 +135,31 @@
     return JSC::JSValue::encode(JSDOMIterator<JSWrapper>::create(globalObject.vm(), getDOMStructure<JSDOMIterator<JSWrapper>>(globalObject.vm(), globalObject), *wrapper, kind));
 }
 
-template<typename IteratorValue> typename std::enable_if<IteratorInspector<IteratorValue>::isMap, JSC::JSValue>::type
-toJS(JSC::ExecState& state, JSDOMGlobalObject* globalObject, IteratorValue& value, IterationKind kind)
+template<typename JSWrapper>
+template<typename IteratorValue> inline typename std::enable_if<IteratorInspector<IteratorValue>::isMap, JSC::JSValue>::type
+JSDOMIterator<JSWrapper>::asJS(JSC::ExecState& state, IteratorValue& value)
 {
     ASSERT(value);
-    if (kind != IterationKind::KeyValue)
-        return toJS(&state, globalObject, (kind == IterationKind::Key) ? value->key : value->value);
+    if (m_kind != IterationKind::KeyValue)
+        return toJS(&state, globalObject(), (m_kind == IterationKind::Key) ? value->key : value->value);
 
-    return jsPair(state, globalObject, value->key, value->value);
+    return jsPair(state, globalObject(), value->key, value->value);
 }
 
-template<typename IteratorValue> typename std::enable_if<IteratorInspector<IteratorValue>::isSet, JSC::JSValue>::type
-toJS(JSC::ExecState& state, JSDOMGlobalObject* globalObject, IteratorValue& value, IterationKind kind)
+template<typename JSWrapper>
+template<typename IteratorValue> inline typename std::enable_if<IteratorInspector<IteratorValue>::isSet, JSC::JSValue>::type
+JSDOMIterator<JSWrapper>::asJS(JSC::ExecState& state, IteratorValue& value)
 {
     ASSERT(value);
-    JSC::JSValue result = toJS(&state, globalObject, *value);
-    if (kind != IterationKind::KeyValue)
+    JSC::JSValue result = toJS(&state, globalObject(), *value);
+    if (m_kind != IterationKind::KeyValue)
         return result;
 
-    // FIXME: first pair value should be the index of result.
-    return jsPair(state, globalObject, result, result);
+    return jsPair(state, globalObject(), JSC::jsNumber(m_index++), result);
 }
 
 template<typename IteratorValue> typename std::enable_if<IteratorInspector<IteratorValue>::isMap, void>::type
-appendForEachArguments(JSC::ExecState& state, JSDOMGlobalObject* globalObject, JSC::MarkedArgumentBuffer& arguments, IteratorValue& value)
+appendForEachArguments(JSC::ExecState& state, JSDOMGlobalObject* globalObject, JSC::MarkedArgumentBuffer& arguments, IteratorValue& value, size_t&)
 {
     ASSERT(value);
     arguments.append(toJS(&state, globalObject, value->value));
@@ -160,12 +167,12 @@
 }
 
 template<typename IteratorValue> typename std::enable_if<IteratorInspector<IteratorValue>::isSet, void>::type
-appendForEachArguments(JSC::ExecState& state, JSDOMGlobalObject* globalObject, JSC::MarkedArgumentBuffer& arguments, IteratorValue& value)
+appendForEachArguments(JSC::ExecState& state, JSDOMGlobalObject* globalObject, JSC::MarkedArgumentBuffer& arguments, IteratorValue& value, size_t& index)
 {
     ASSERT(value);
     JSC::JSValue argument = toJS(&state, globalObject, *value);
     arguments.append(argument);
-    arguments.append(argument);
+    arguments.append(JSC::jsNumber(index++));
 }
 
 template<typename JSWrapper>
@@ -180,10 +187,11 @@
     if (callType == JSC::CallType::None)
         return throwVMTypeError(&state);
 
+    size_t index = 0;
     auto iterator = wrapper->wrapped().createIterator();
     while (auto value = iterator.next()) {
         JSC::MarkedArgumentBuffer arguments;
-        appendForEachArguments(state, wrapper->globalObject(), arguments, value);
+        appendForEachArguments(state, wrapper->globalObject(), arguments, value, index);
         arguments.append(wrapper);
         JSC::call(&state, state.argument(0), callType, callData, wrapper, arguments);
         if (state.hadException())
@@ -202,10 +210,13 @@
 template<typename JSWrapper>
 JSC::JSValue JSDOMIterator<JSWrapper>::next(JSC::ExecState& state)
 {
-    auto iteratorValue = m_iterator.next();
-    if (!iteratorValue)
-        return createIteratorResultObject(&state, JSC::jsUndefined(), true);
-    return createIteratorResultObject(&state, toJS(state, globalObject(), iteratorValue, m_kind), false);
+    if (m_iterator) {
+        auto iteratorValue = m_iterator->next();
+        if (iteratorValue)
+            return createIteratorResultObject(&state, asJS(state, iteratorValue), false);
+        m_iterator = Nullopt;
+    }
+    return createIteratorResultObject(&state, JSC::jsUndefined(), true);
 }
 
 template<typename JSWrapper>
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to