New FormData methods: get, getAll, has, set, delete and iterable

Source/core/html/DOMFormData.cpp

Index: Source/core/html/DOMFormData.cpp
diff --git a/Source/core/html/DOMFormData.cpp b/Source/core/html/DOMFormData.cpp
index 92fb7695d5cb5641b7f7ca0bb310c061bcaf71b9..6496e75011a70e449e52130361d4776f8372935e 100644
--- a/Source/core/html/DOMFormData.cpp
+++ b/Source/core/html/DOMFormData.cpp
@@ -31,13 +31,92 @@
 #include "config.h"
 #include "core/html/DOMFormData.h"
 
+#include "bindings/core/v8/V8Binding.h"
+#include "bindings/core/v8/V8File.h"
+#include "core/dom/Iterator.h"
 #include "core/fileapi/Blob.h"
+#include "core/fileapi/File.h"
 #include "core/html/HTMLFormElement.h"
 #include "wtf/text/TextEncoding.h"
 #include "wtf/text/WTFString.h"
 
 namespace blink {
 
+namespace {
+
+ScriptValue entryValueToScriptValue(ScriptState* scriptState, const FormDataList::Entry& entry)
+{
+    if (entry.isString())
+        return ScriptValue(scriptState, v8String(scriptState->isolate(), entry.string()));
+    if (entry.isFile())
+        return ScriptValue(scriptState, toV8(entry.file(), scriptState->context()->Global(), scriptState->isolate()));
+    ASSERT(entry.isNone());
+    return ScriptValue(scriptState, v8::Null(scriptState->isolate()));
+}
+
+class DOMFormDataIterator FINAL : public Iterator {
+public:
+    enum IterationType { KeyValue, Key, Value };
+
+    DOMFormDataIterator(DOMFormData* formData, IterationType type) : m_formData(formData), m_type(type), m_current(0) { }
+
+    virtual ScriptValue next(ScriptState* scriptState, ExceptionState& exceptionState) OVERRIDE
+    {
+        // FIXME: This neither snapshots nor handles deletion during iteration
+        // the same way as the ES6 spec (i.e. with 'empty' slots). Removing
+        // entries at/behind the iterator position will result in later
+        // entries being skipped.

[arv (Not doing code reviews), 2014/09/17 19:49:38]

That is pretty unfortunate. Can we make sure this fixme is fixed before we
enable this?

[jsbell, 2014/09/18 16:18:59]

Agreed. 

I think we need to spec to stabilize, too. DOM APIs should be consistent
(snapshot vs. smarter live iterators)... IIRC there was some W3C bug about that.

+
+        v8::Isolate* isolate = scriptState->isolate();
+        if (m_current >= m_formData->size())
+            return ScriptValue(scriptState, v8DoneIteratorResult(isolate));
+
+        const FormDataList::Entry entry = m_formData->getEntry(m_current++);
+        ASSERT(!entry.isNone());
+        ScriptValue name, value;
+        if (m_type != Value)
+            name = ScriptValue(scriptState, v8String(scriptState->isolate(), entry.name()));
+        if (m_type != Key)
+            value = entryValueToScriptValue(scriptState, entry);
+
+        switch (m_type) {
+        case KeyValue: {
+            Vector<ScriptValue> pair;
+            pair.append(name);
+            pair.append(value);
+            return ScriptValue(scriptState, v8IteratorResult(scriptState, pair));
+        }
+        case Key:
+            return ScriptValue(scriptState, v8IteratorResult(scriptState, name));
+
+        case Value:
+            return ScriptValue(scriptState, v8IteratorResult(scriptState, value));
+        }
+        ASSERT_NOT_REACHED();
+        return ScriptValue();
+    }
+
+    virtual ScriptValue next(ScriptState* scriptState, ScriptValue value, ExceptionState& exceptionState) OVERRIDE
+    {
+        // FIXME: Implement the "bring your own payload object" variant of next().

[arv (Not doing code reviews), 2014/09/17 19:49:38]

Not sure what this means?

[jsbell, 2014/09/18 16:18:59]

Huh... I thought at one point you could call iter.next(obj) passing your own
object, onto which value/done would be applied, to avoid GC overhead. But I'm
not seeing that in the latest ES6 draft. Removed? Never made it?

So... I can remove the FIXME. I copy/pasted this next() overload from elsewhere.


@arv: Is it future-proofing by rejecting a passed argument? Or should it be
removed (here and elsewhere)?

[arv (Not doing code reviews), 2014/09/18 17:33:45]

You should not reject an argument, just ignore it. The argument passed to next
is used by generators.

function* f() {
  return yield 1;
}
var g = f();
g.next();  // {value: 1, done: false}
g.next(2);  // {value: 2, done: true}

[jsbell, 2014/09/18 23:56:29]

Got it. I was looking at the InternalsIterator for guidance rather than the
MIDIPort one. :P

+        exceptionState.throwTypeError("Not implemented");
+        return ScriptValue();
+    }
+
+    virtual void trace(Visitor* visitor)
+    {
+        Iterator::trace(visitor);
+        visitor->trace(m_formData);
+    }
+
+private:
+    const RefPtrWillBeMember<DOMFormData> m_formData;
+    const IterationType m_type;
+    size_t m_current;
+};
+
+} // namespace
+
 DOMFormData::DOMFormData(const WTF::TextEncoding& encoding)
     : FormDataList(encoding)
 {
@@ -68,4 +147,60 @@ void DOMFormData::append(const String& name, Blob* blob, const String& filename)
         appendBlob(name, blob, filename);
 }
 
+void DOMFormData::remove(const String& name)

[arv (Not doing code reviews), 2014/09/17 19:49:38]

Why this extra indirection? Can you just use implementedAs in the IDL?

[jsbell, 2014/09/18 16:18:58]

Yeah, this doesn't gain anything. I'll flatten this.

+{
+    return deleteEntry(name);
+}
+
+ScriptValue DOMFormData::get(ScriptState* scriptState, const String& name)
+{
+    return entryValueToScriptValue(scriptState, getEntry(name));
+}
+
+Vector<ScriptValue> DOMFormData::getAll(ScriptState* scriptState, const String& name)
+{
+    Vector<ScriptValue> results;
+    WillBeHeapVector<FormDataList::Entry> entries = FormDataList::getAll(name);
+    for (size_t i = 0; i < entries.size(); ++i) {
+        const FormDataList::Entry& entry = entries[i];
+        ASSERT(!entry.isNone());
+        ASSERT(entry.name() == name);
+        results.append(entryValueToScriptValue(scriptState, entry));
+    }
+    ASSERT(results.size() == entries.size());
+    return results;
+}
+
+bool DOMFormData::has(const String& name)
+{
+    return hasEntry(name);

[arv (Not doing code reviews), 2014/09/17 19:49:38]

Maybe use implementedAs in the IDL here too?

[jsbell, 2014/09/18 16:18:58]

Ditto.

+}
+
+void DOMFormData::set(const String& name, const String& value)
+{
+    if (!name.isEmpty())
+        setData(name, value);
+}
+
+void DOMFormData::set(const String& name, Blob* blob, const String& filename)
+{
+    if (!name.isEmpty())
+        setBlob(name, blob, filename);
+}
+
+Iterator* DOMFormData::entries()
+{
+    return new DOMFormDataIterator(this, DOMFormDataIterator::KeyValue);
+}
+
+Iterator* DOMFormData::keys()
+{
+    return new DOMFormDataIterator(this, DOMFormDataIterator::Key);
+}
+
+Iterator* DOMFormData::values()
+{
+    return new DOMFormDataIterator(this, DOMFormDataIterator::Value);
+}
+
 } // namespace blink