IDL: Add forEach() method to iterable<>/maplike<>/setlike<> interfaces

Source/core/dom/Iterable.h

Index: Source/core/dom/Iterable.h
diff --git a/Source/core/dom/Iterable.h b/Source/core/dom/Iterable.h
index c1ba4d8e5832ed2fa0631708c206a47e530a7f25..71b090dcd3e4d06ae1febd6c76256e923d819246 100644
--- a/Source/core/dom/Iterable.h
+++ b/Source/core/dom/Iterable.h
@@ -6,6 +6,7 @@
 #define Iterable_h
 
 #include "bindings/core/v8/V8IteratorResultValue.h"
+#include "bindings/core/v8/V8ScriptRunner.h"
 #include "core/dom/Iterator.h"
 
 namespace blink {
@@ -38,6 +39,41 @@ public:
         return new IterableIterator<EntrySelector>(source);
     }
 
+    void forEach(ScriptState* scriptState, const ScriptValue& thisValue, const ScriptValue& callback, const ScriptValue& thisArg, ExceptionState& exceptionState)
+    {
+        IterationSource* source = this->startIteration(scriptState, exceptionState);
+
+        v8::Isolate* isolate = scriptState->isolate();
+        v8::TryCatch tryCatch(isolate);

[haraken, 2015/01/27 01:56:30]

I guess you don't need this TryCatch (see the below comment).

+
+        v8::Local<v8::Object> creationContext(scriptState->context()->Global());
+        v8::Local<v8::Function> v8Callback(callback.v8Value().As<v8::Function>());
+        v8::Local<v8::Value> v8ThisArg(thisArg.v8Value());
+        v8::Local<v8::Value> args[3];
+
+        args[2] = thisValue.v8Value();
+
+        while (true) {
+            KeyType key;
+            ValueType value;
+
+            if (!source->next(scriptState, key, value, exceptionState))
+                return;
+
+            ASSERT(!exceptionState.hadException());
+
+            args[0] = toV8(value, creationContext, isolate);
+            args[1] = toV8(key, creationContext, isolate);

[haraken, 2015/01/27 01:56:30]

Nit: It is interesting that the order is value => key, not key => value.

[Jens Widell, 2015/01/27 07:19:23]

Yeah, in this context it seems a bit backwards. This is compatible with the
native [].forEach() though.

+
+            V8ScriptRunner::callFunction(v8Callback, scriptState->executionContext(), v8ThisArg, 3, args, isolate);
+
+            if (tryCatch.HasCaught()) {

[haraken, 2015/01/27 01:56:30]

Can we use exceptionState.hadException()? We don't want to mix TryCatch and
ExceptionState.

[Jens Widell, 2015/01/27 07:19:23]

Since I'm not passing 'exceptionState' to V8ScriptRunner::callFunction(), it
can't throw that way. I don't know if there's another "call function" utility
function that does take an ExceptionState that I can use instead.

Technically, we have to "mix" ExceptionState and TryCatch in some places, namely
at the boundary points between "Blink" and "V8". This is one such boundary point
(we're calling into V8 from Blink, after all.)

Also, since this is a loop, it's somewhat advantageous to declare the TryCatch
at this level, outside the loop, to limit the overhead.

+                exceptionState.rethrowV8Exception(tryCatch.Exception());
+                return;
+            }
+        }
+    }
+
     class IterationSource : public GarbageCollectedFinalized<IterationSource> {
     public:
         virtual ~IterationSource() { }