Auto merge of #9049 - KiChjang:form-data-refactor, r=eefriedman

Refactor FormData code to match updated spec

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9049)
<!-- Reviewable:end -->

components/script/dom/formdata.rs

@@ -5,32 +5,33 @@
 use dom::bindings::cell::DOMRefCell;
 use dom::bindings::codegen::Bindings::FormDataBinding;
 use dom::bindings::codegen::Bindings::FormDataBinding::FormDataMethods;
-use dom::bindings::codegen::UnionTypes::FileOrString;
-use dom::bindings::codegen::UnionTypes::FileOrString::{eFile, eString};
+use dom::bindings::codegen::UnionTypes::BlobOrUSVString::{self, eBlob, eUSVString};
 use dom::bindings::error::{Fallible};
 use dom::bindings::global::{GlobalField, GlobalRef};
 use dom::bindings::inheritance::Castable;
 use dom::bindings::js::{JS, Root};
 use dom::bindings::reflector::{Reflector, reflect_dom_object};
+use dom::bindings::str::USVString;
 use dom::blob::Blob;
 use dom::file::File;
 use dom::htmlformelement::HTMLFormElement;
 use std::collections::HashMap;
 use std::collections::hash_map::Entry::{Occupied, Vacant};
+use string_cache::Atom;
 use util::str::DOMString;
 
 #[derive(JSTraceable, Clone)]
 #[must_root]
 #[derive(HeapSizeOf)]
 pub enum FormDatum {
-    StringData(DOMString),
-    FileData(JS<File>)
+    StringData(String),
+    BlobData(JS<Blob>)
 }
 
 #[dom_struct]
 pub struct FormData {
     reflector_: Reflector,
-    data: DOMRefCell<HashMap<DOMString, Vec<FormDatum>>>,
+    data: DOMRefCell<HashMap<Atom, Vec<FormDatum>>>,
     global: GlobalField,
     form: Option<JS<HTMLFormElement>>
 }
@@ -51,72 +52,87 @@ impl FormData {
     }
 
     pub fn Constructor(global: GlobalRef, form: Option<&HTMLFormElement>) -> Fallible<Root<FormData>> {
+        // TODO: Construct form data set for form if it is supplied
         Ok(FormData::new(form, global))
     }
 }
 
 impl FormDataMethods for FormData {
-    #[allow(unrooted_must_root)]
     // https://xhr.spec.whatwg.org/#dom-formdata-append
-    fn Append(&self, name: DOMString, value: &Blob, filename: Option<DOMString>) {
-        let file = FormDatum::FileData(JS::from_rooted(&self.get_file_from_blob(value, filename)));
+    fn Append(&self, name: USVString, value: USVString) {
         let mut data = self.data.borrow_mut();
-        match data.entry(name) {
-            Occupied(entry) => entry.into_mut().push(file),
-            Vacant(entry) => {
-                entry.insert(vec!(file));
-            }
+        match data.entry(Atom::from(&*name.0)) {
+            Occupied(entry) => entry.into_mut().push(FormDatum::StringData(value.0)),
+            Vacant  (entry) => { entry.insert(vec!(FormDatum::StringData(value.0))); }
         }
     }
 
+    #[allow(unrooted_must_root)]
     // https://xhr.spec.whatwg.org/#dom-formdata-append
-    fn Append_(&self, name: DOMString, value: DOMString) {
+    fn Append_(&self, name: USVString, value: &Blob, filename: Option<USVString>) {
+        let blob = FormDatum::BlobData(JS::from_rooted(&self.get_file_or_blob(value, filename)));
         let mut data = self.data.borrow_mut();
-        match data.entry(name) {
-            Occupied(entry) => entry.into_mut().push(FormDatum::StringData(value)),
-            Vacant  (entry) => { entry.insert(vec!(FormDatum::StringData(value))); },
+        match data.entry(Atom::from(&*name.0)) {
+            Occupied(entry) => entry.into_mut().push(blob),
+            Vacant(entry) => {
+                entry.insert(vec!(blob));
+            }
         }
     }
 
     // https://xhr.spec.whatwg.org/#dom-formdata-delete
-    fn Delete(&self, name: DOMString) {
-        self.data.borrow_mut().remove(&name);
+    fn Delete(&self, name: USVString) {
+        self.data.borrow_mut().remove(&Atom::from(&*name.0));
     }
 
     // https://xhr.spec.whatwg.org/#dom-formdata-get
-    fn Get(&self, name: DOMString) -> Option<FileOrString> {
+    fn Get(&self, name: USVString) -> Option<BlobOrUSVString> {
         self.data.borrow()
-                 .get(&name)
+                 .get(&Atom::from(&*name.0))
                  .map(|entry| match entry[0] {
-                     FormDatum::StringData(ref s) => eString(s.clone()),
-                     FormDatum::FileData(ref f) => eFile(Root::from_ref(&*f)),
+                     FormDatum::StringData(ref s) => eUSVString(USVString(s.clone())),
+                     FormDatum::BlobData(ref b) => eBlob(Root::from_ref(&*b)),
                  })
     }
 
-    // https://xhr.spec.whatwg.org/#dom-formdata-has
-    fn Has(&self, name: DOMString) -> bool {
-        self.data.borrow().contains_key(&name)
+    // https://xhr.spec.whatwg.org/#dom-formdata-getall
+    fn GetAll(&self, name: USVString) -> Vec<BlobOrUSVString> {
+        self.data.borrow()
+                 .get(&Atom::from(&*name.0))
+                 .map_or(vec![], |data|
+                    data.iter().map(|item| match *item {
+                        FormDatum::StringData(ref s) => eUSVString(USVString(s.clone())),
+                        FormDatum::BlobData(ref b) => eBlob(Root::from_ref(&*b)),
+                    }).collect()
+                 )
     }
 
-    // https://xhr.spec.whatwg.org/#dom-formdata-set
-    fn Set_(&self, name: DOMString, value: DOMString) {
-        self.data.borrow_mut().insert(name, vec!(FormDatum::StringData(value)));
+    // https://xhr.spec.whatwg.org/#dom-formdata-has
+    fn Has(&self, name: USVString) -> bool {
+        self.data.borrow().contains_key(&Atom::from(&*name.0))
     }
 
     #[allow(unrooted_must_root)]
     // https://xhr.spec.whatwg.org/#dom-formdata-set
-    fn Set(&self, name: DOMString, value: &Blob, filename: Option<DOMString>) {
-        let file = FormDatum::FileData(JS::from_rooted(&self.get_file_from_blob(value, filename)));
-        self.data.borrow_mut().insert(name, vec!(file));
+    fn Set(&self, name: USVString, value: BlobOrUSVString) {
+        let val = match value {
+            eUSVString(s) => FormDatum::StringData(s.0),
+            eBlob(b) => FormDatum::BlobData(JS::from_rooted(&b))
+        };
+        self.data.borrow_mut().insert(Atom::from(&*name.0), vec!(val));
     }
 }
 
 
 impl FormData {
-    fn get_file_from_blob(&self, value: &Blob, filename: Option<DOMString>) -> Root<File> {
-        let global = self.global.root();
-        let f = value.downcast::<File>();
-        let name = filename.unwrap_or(f.map(|inner| inner.name().clone()).unwrap_or(DOMString::from("blob")));
-        File::new(global.r(), value, name)
+    fn get_file_or_blob(&self, value: &Blob, filename: Option<USVString>) -> Root<Blob> {
+        match filename {
+            Some(fname) => {
+                let global = self.global.root();
+                let name = DOMString::from(fname.0);
+                Root::upcast(File::new(global.r(), value, name))
+            }
+            None => Root::from_ref(value)
+        }
     }
 }

components/script/dom/webidls/FormData.webidl

@@ -4,19 +4,20 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 /*
  * The origin of this IDL file is
- * https://xhr.spec.whatwg.org
+ * https://xhr.spec.whatwg.org/#interface-formdata
  */
 
-typedef (File or DOMString) FormDataEntryValue;
+typedef (Blob or USVString) FormDataEntryValue;
 
-[Constructor(optional HTMLFormElement form)]
+[Constructor(optional HTMLFormElement form),
+ /*Exposed=(Window,Worker)*/]
 interface FormData {
-  void append(DOMString name, Blob value, optional DOMString filename);
-  void append(DOMString name, DOMString value);
-  void delete(DOMString name);
-  FormDataEntryValue? get(DOMString name);
-  // sequence<FormDataEntryValue> getAll(DOMString name);
-  boolean has(DOMString name);
-  void set(DOMString name, Blob value, optional DOMString filename);
-  void set(DOMString name, DOMString value);
+  void append(USVString name, USVString value);
+  void append(USVString name, Blob value, optional USVString filename);
+  void delete(USVString name);
+  FormDataEntryValue? get(USVString name);
+  sequence<FormDataEntryValue> getAll(USVString name);
+  boolean has(USVString name);
+  void set(USVString name, FormDataEntryValue value);
+  // iterable<USVString, FormDataEntryValue>;
 };

tests/wpt/metadata/MANIFEST.json

@@ -30174,6 +30174,30 @@
     "deleted": [],
     "items": {
       "testharness": {
+        "XMLHttpRequest/formdata-delete.htm": [
+          {
+            "path": "XMLHttpRequest/formdata-delete.htm",
+            "url": "/XMLHttpRequest/formdata-delete.htm"
+          }
+        ],
+        "XMLHttpRequest/formdata-get.htm": [
+          {
+            "path": "XMLHttpRequest/formdata-get.htm",
+            "url": "/XMLHttpRequest/formdata-get.htm"
+          }
+        ],
+        "XMLHttpRequest/formdata-has.htm": [
+          {
+            "path": "XMLHttpRequest/formdata-has.htm",
+            "url": "/XMLHttpRequest/formdata-has.htm"
+          }
+        ],
+        "XMLHttpRequest/formdata-set.htm": [
+          {
+            "path": "XMLHttpRequest/formdata-set.htm",
+            "url": "/XMLHttpRequest/formdata-set.htm"
+          }
+        ],
         "html/semantics/forms/the-button-element/button-activate.html": [
           {
             "path": "html/semantics/forms/the-button-element/button-activate.html",

tests/wpt/metadata/XMLHttpRequest/FormData-append.html.ini

@@ -2,4 +2,6 @@
   type: testharness
   [Passing a String object to FormData.append should work.]
     expected: FAIL
+  [testFormDataAppendEmptyBlob]
+    expected: FAIL
 

tests/wpt/metadata/XMLHttpRequest/formdata-delete.htm.ini

@@ -0,0 +1,7 @@
+[formdata-delete.htm]
+  type:testharness
+  [testFormDataDeleteFromFormNonExistentKey]
+    expected: FAIL
+  [testFormDataDeleteFromFormOtherKey]
+    expected: FAIL
+

tests/wpt/metadata/XMLHttpRequest/formdata-get.htm.ini

@@ -0,0 +1,6 @@
+[formdata-get.htm]
+  type: testharness
+  [testFormDataGetFromForm]
+    expected: FAIL
+  [testFormDataGetAllFromForm]
+    expected: FAIL

tests/wpt/metadata/XMLHttpRequest/formdata-has.htm.ini

@@ -0,0 +1,5 @@
+[formdata-has.htm]
+  type: testharness
+  [testFormDataHasFromForm]
+    expected: FAIL
+

tests/wpt/metadata/XMLHttpRequest/formdata-set.htm.ini

@@ -0,0 +1,7 @@
+[formdata-set.htm]
+  type: testharness
+  [Passing a String object to FormData.set should work]
+    expected: FAIL
+  [testFormDataSetEmptyBlob]
+    expected: FAIL
+

tests/wpt/metadata/XMLHttpRequest/interfaces.html.ini

@@ -1,20 +1,5 @@
 [interfaces.html]
   type: testharness
-  [FormData interface: new FormData() must inherit property "getAll" with the proper type (4)]
-    expected: FAIL
-
-  [FormData interface: new FormData(form) must inherit property "getAll" with the proper type (4)]
-    expected: FAIL
-
-  [FormData interface: operation getAll(USVString)]
-    expected: FAIL
-
-  [FormData interface: calling getAll(USVString) on new FormData() with too few arguments must throw TypeError]
-    expected: FAIL
-
-  [FormData interface: calling getAll(USVString) on new FormData(form) with too few arguments must throw TypeError]
-    expected: FAIL
-
   [ProgressEvent interface: existence and properties of interface object]
     expected: FAIL
 

tests/wpt/web-platform-tests/XMLHttpRequest/FormData-append.html

@@ -1,28 +1,94 @@
 <!doctype html>
-<meta charset=utf-8>
+<meta charset="utf-8">
 <title>FormData.append</title>
-<link rel=help href=https://xhr.spec.whatwg.org/#dom-formdata-append>
-<script src=/resources/testharness.js></script>
-<script src=/resources/testharnessreport.js></script>
-<div id=log></div>
+<link rel="help" href="https://xhr.spec.whatwg.org/#dom-formdata-append">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<div id="log"></div>
+<form id="form" />
 <script>
-function test_formdata(creator, verifier, description) {
-  async_test(description).step(function() {
-    var fd = creator();
-    var xhr = new XMLHttpRequest();
-    xhr.onload = this.step_func(function() {
-      verifier(xhr.responseText);
-      this.done();
-    });
-    xhr.open("POST", "resources/upload.py");
-    xhr.send(fd);
-  })
-}
-test_formdata(function() {
-  var fd = new FormData();
-  fd.append("name", new String("value"));
-  return fd;
-}, function(data) {
-  assert_equals(data, "name=value,\n");
-}, "Passing a String object to FormData.append should work.");
+    function test_formdata(creator, verifier, description) {
+        async_test(description).step(function() {
+            var fd = creator();
+            var xhr = new XMLHttpRequest();
+            xhr.onload = this.step_func(function() {
+                verifier(xhr.responseText);
+                this.done();
+            });
+            xhr.open("POST", "resources/upload.py");
+            xhr.send(fd);
+        });
+    }
+
+    test_formdata(function() {
+        var fd = new FormData();
+        fd.append("name", new String("value"));
+        return fd;
+    }, function(data) {
+        assert_equals(data, "name=value,\n");
+    }, "Passing a String object to FormData.append should work.");
+
+    test(function() {
+        assert_equals(create_formdata(['key', 'value1']).get('key'), "value1");
+    }, 'testFormDataAppend1');
+    test(function() {
+        assert_equals(create_formdata(['key', 'value2'], ['key', 'value1']).get('key'), "value2");
+    }, 'testFormDataAppend2');
+    test(function() {
+        assert_equals(create_formdata(['key', undefined]).get('key'), "undefined");
+    }, 'testFormDataAppendUndefined1');
+    test(function() {
+        assert_equals(create_formdata(['key', undefined], ['key', 'value1']).get('key'), "undefined");
+    }, 'testFormDataAppendUndefined2');
+    test(function() {
+        assert_equals(create_formdata(['key', null]).get('key'), "null");
+    }, 'testFormDataAppendNull1');
+    test(function() {
+        assert_equals(create_formdata(['key', null], ['key', 'value1']).get('key'), "null");
+    }, 'testFormDataAppendNull2');
+    test(function() {
+        var fd = new FormData(document.getElementById("form"));
+        fd.append('key', 'value1');
+        assert_equals(fd.get('key'), "value1");
+    }, 'testFormDataAppendToForm1');
+    test(function() {
+        var fd = new FormData(document.getElementById("form"));
+        fd.append('key', 'value2');
+        fd.append('key', 'value1');
+        assert_equals(fd.get('key'), "value2");
+    }, 'testFormDataAppendToForm2');
+    test(function() {
+        var fd = new FormData(document.getElementById("form"));
+        fd.append('key', undefined);
+        assert_equals(fd.get('key'), "undefined");
+    }, 'testFormDataAppendToFormUndefined1');
+    test(function() {
+        var fd = new FormData(document.getElementById("form"));
+        fd.append('key', undefined);
+        fd.append('key', 'value1');
+        assert_equals(fd.get('key'), "undefined");
+    }, 'testFormDataAppendToFormUndefined2');
+    test(function() {
+        var fd = new FormData(document.getElementById("form"));
+        fd.append('key', null);
+        assert_equals(fd.get('key'), "null");
+    }, 'testFormDataAppendToFormNull1');
+    test(function() {
+        var fd = new FormData(document.getElementById("form"));
+        fd.append('key', null);
+        fd.append('key', 'value1');
+        assert_equals(fd.get('key'), "null");
+    }, 'testFormDataAppendToFormNull2');
+    test(function() {
+        assert_object_equals(create_formdata(['key', new Blob(), 'blank.txt']).get('key'),
+                             new File(new Blob(), 'blank.txt'));
+    }, 'testFormDataAppendEmptyBlob');
+
+    function create_formdata() {
+        var fd = new FormData();
+        for (var i = 0; i < arguments.length; i++) {
+            fd.append.apply(fd, arguments[i]);
+        };
+        return fd;
+    }
 </script>