Bug 1297304. Fix our named property DOM proxy code to handle deletion correctly when expandos are involved. r=peterv
authorBoris Zbarsky <bzbarsky@mit.edu>
Tue, 20 Sep 2016 13:24:34 +0100
changeset 314622 3c60fe237a35defea33b60a4a0ab15b700cdc110
parent 314621 2daffd62a19b2a060e573bddc3fb0536607faf5e
child 314623 e6abf7c38f0f32367217802c73a7ca8a888b3008
push id30732
push usercbook@mozilla.com
push dateWed, 21 Sep 2016 10:04:03 +0000
treeherdermozilla-central@560b2c805bf7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspeterv
bugs1297304
milestone52.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1297304. Fix our named property DOM proxy code to handle deletion correctly when expandos are involved. r=peterv
dom/bindings/Codegen.py
testing/web-platform/tests/dom/collections/HTMLCollection-supported-property-indices.html
testing/web-platform/tests/dom/collections/HTMLCollection-supported-property-names.html
testing/web-platform/tests/html/semantics/forms/the-form-element/form-indexed-element.html
testing/web-platform/tests/html/semantics/forms/the-form-element/form-nameditem.html
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -11238,41 +11238,60 @@ class CGDOMJSProxyHandler_delete(ClassMe
                   $*{indexedBody}
                   return deleteSucceeded ? opresult.succeed() : opresult.failCantDelete();
                 }
                 """,
                 indexedBody=indexedBody)
 
         namedBody = getDeleterBody("Named", foundVar="found")
         if namedBody is not None:
+            delete += dedent(
+                """
+                // Try named delete only if the named property visibility
+                // algorithm says the property is visible.
+                bool tryNamedDelete = true;
+                { // Scope for expando
+                  JS::Rooted<JSObject*> expando(cx, DOMProxyHandler::GetExpandoObject(proxy));
+                  if (expando) {
+                    bool hasProp;
+                    if (!JS_HasPropertyById(cx, expando, id, &hasProp)) {
+                      return false;
+                    }
+                    tryNamedDelete = !hasProp;
+                  }
+                }
+                """)
+
+            if not self.descriptor.interface.getExtendedAttribute('OverrideBuiltins'):
+                delete += dedent(
+                    """
+                    if (tryNamedDelete) {
+                      bool hasOnProto;
+                      if (!HasPropertyOnPrototype(cx, proxy, id, &hasOnProto)) {
+                        return false;
+                      }
+                      tryNamedDelete = !hasOnProto;
+                    }
+                    """)
+
             # We always return above for an index id in the case when we support
             # indexed properties, so we can just treat the id as a name
             # unconditionally here.
             delete += fill(
                 """
-                bool found = false;
-                bool deleteSucceeded;
-                $*{namedBody}
-                if (found) {
-                  return deleteSucceeded ? opresult.succeed() : opresult.failCantDelete();
+                if (tryNamedDelete) {
+                  bool found = false;
+                  bool deleteSucceeded;
+                  $*{namedBody}
+                  if (found) {
+                    return deleteSucceeded ? opresult.succeed() : opresult.failCantDelete();
+                  }
                 }
                 """,
                 namedBody=namedBody)
-            if not self.descriptor.interface.getExtendedAttribute('OverrideBuiltins'):
-                delete = fill(
-                    """
-                    bool hasOnProto;
-                    if (!HasPropertyOnPrototype(cx, proxy, id, &hasOnProto)) {
-                      return false;
-                    }
-                    if (!hasOnProto) {
-                      $*{delete}
-                    }
-                    """,
-                    delete=delete)
 
         delete += dedent("""
 
             return dom::DOMProxyHandler::delete_(cx, proxy, id, opresult);
             """)
 
         return delete
 
--- a/testing/web-platform/tests/dom/collections/HTMLCollection-supported-property-indices.html
+++ b/testing/web-platform/tests/dom/collections/HTMLCollection-supported-property-indices.html
@@ -92,9 +92,88 @@ test(function() {
   assert_equals(collection.namedItem(4294967297),
                 document.getElementById("4294967297"));
   assert_equals(collection[4294967293], undefined);
   assert_equals(collection[4294967294], undefined);
   assert_equals(collection[4294967295], document.getElementById("4294967295"));
   assert_equals(collection[4294967296], document.getElementById("4294967296"));
   assert_equals(collection[4294967297], document.getElementById("4294967297"));
 }, "Handling of property names that look like integers around 2^32");
+
+test(function() {
+  var elements = document.getElementsByTagName("foo");
+  var old_item = elements[0];
+  var old_desc = Object.getOwnPropertyDescriptor(elements, 0);
+  assert_equals(old_desc.value, old_item);
+  assert_true(old_desc.enumerable);
+  assert_true(old_desc.configurable);
+  assert_false(old_desc.writable);
+
+  elements[0] = 5;
+  assert_equals(elements[0], old_item);
+  assert_throws(new TypeError(), function() {
+    "use strict";
+    elements[0] = 5;
+  });
+  assert_throws(new TypeError(), function() {
+    Object.defineProperty(elements, 0, { value: 5 });
+  });
+
+  delete elements[0];
+  assert_equals(elements[0], old_item);
+
+  assert_throws(new TypeError(), function() {
+    "use strict";
+    delete elements[0];
+  });
+  assert_equals(elements[0], old_item);
+}, 'Trying to set an expando that would shadow an already-existing indexed property');
+
+test(function() {
+  var elements = document.getElementsByTagName("foo");
+  var idx = elements.length;
+  var old_item = elements[idx];
+  var old_desc = Object.getOwnPropertyDescriptor(elements, idx);
+  assert_equals(old_item, undefined);
+  assert_equals(old_desc, undefined);
+
+  // [[DefineOwnProperty]] will disallow defining an indexed expando.
+  elements[idx] = 5;
+  assert_equals(elements[idx], undefined);
+  assert_throws(new TypeError(), function() {
+    "use strict";
+    elements[idx] = 5;
+  });
+  assert_throws(new TypeError(), function() {
+    Object.defineProperty(elements, idx, { value: 5 });
+  });
+
+  // Check that deletions out of range do not throw
+  delete elements[idx];
+  (function() {
+    "use strict";
+    delete elements[idx];
+  })();
+}, 'Trying to set an expando with an indexed property name past the end of the list');
+
+test(function(){
+  var elements = document.getElementsByTagName("foo");
+  var old_item = elements[0];
+  var old_desc = Object.getOwnPropertyDescriptor(elements, 0);
+  assert_equals(old_desc.value, old_item);
+  assert_true(old_desc.enumerable);
+  assert_true(old_desc.configurable);
+  assert_false(old_desc.writable);
+
+  Object.prototype[0] = 5;
+  this.add_cleanup(function () { delete Object.prototype[0]; });
+  assert_equals(elements[0], old_item);
+
+  delete elements[0];
+  assert_equals(elements[0], old_item);
+
+  assert_throws(new TypeError(), function() {
+    "use strict";
+    delete elements[0];
+  });
+  assert_equals(elements[0], old_item);
+}, 'Trying to delete an indexed property name should never work');
 </script>
--- a/testing/web-platform/tests/dom/collections/HTMLCollection-supported-property-names.html
+++ b/testing/web-platform/tests/dom/collections/HTMLCollection-supported-property-names.html
@@ -46,9 +46,90 @@ test(function () {
   this.add_cleanup(function () {elem.remove();});
   document.body.appendChild(elem);
 
   var elements = document.getElementsByTagName("foo");
   elements.someProperty = "some value";
 
   assert_array_equals(Object.getOwnPropertyNames(elements), ['0', 'someProperty']);
 }, 'Object.getOwnPropertyNames on HTMLCollection with expando object');
+
+test(function() {
+  var elements = document.getElementsByTagName("span");
+  var old_item = elements["some-id"];
+  var old_desc = Object.getOwnPropertyDescriptor(elements, "some-id");
+  assert_equals(old_desc.value, old_item);
+  assert_false(old_desc.enumerable);
+  assert_true(old_desc.configurable);
+  assert_false(old_desc.writable);
+
+  elements["some-id"] = 5;
+  assert_equals(elements["some-id"], old_item);
+  assert_throws(new TypeError(), function() {
+    "use strict";
+    elements["some-id"] = 5;
+  });
+  assert_throws(new TypeError(), function() {
+    Object.defineProperty(elements, "some-id", { value: 5 });
+  });
+
+  delete elements["some-id"];
+  assert_equals(elements["some-id"], old_item);
+
+  assert_throws(new TypeError(), function() {
+    "use strict";
+    delete elements["some-id"];
+  });
+  assert_equals(elements["some-id"], old_item);
+
+}, 'Trying to set an expando that would shadow an already-existing named property');
+
+test(function() {
+  var elements = document.getElementsByTagName("span");
+  var old_item = elements["new-id"];
+  var old_desc = Object.getOwnPropertyDescriptor(elements, "new-id");
+  assert_equals(old_item, undefined);
+  assert_equals(old_desc, undefined);
+
+  elements["new-id"] = 5;
+  assert_equals(elements["new-id"], 5);
+
+  var span = document.createElement("span");
+  this.add_cleanup(function () {span.remove();});
+  span.id = "new-id";
+  document.body.appendChild(span);
+
+  assert_equals(elements.namedItem("new-id"), span);
+  assert_equals(elements["new-id"], 5);
+
+  delete elements["new-id"];
+  assert_equals(elements["new-id"], span);
+}, 'Trying to set an expando that shadows a named property that gets added later');
+
+test(function() {
+  var elements = document.getElementsByTagName("span");
+  var old_item = elements["new-id2"];
+  var old_desc = Object.getOwnPropertyDescriptor(elements, "new-id2");
+  assert_equals(old_item, undefined);
+  assert_equals(old_desc, undefined);
+
+  Object.defineProperty(elements, "new-id2", { configurable: false, writable:
+                                              false, value: 5 });
+  assert_equals(elements["new-id2"], 5);
+
+  var span = document.createElement("span");
+  this.add_cleanup(function () {span.remove();});
+  span.id = "new-id2";
+  document.body.appendChild(span);
+
+  assert_equals(elements.namedItem("new-id2"), span);
+  assert_equals(elements["new-id2"], 5);
+
+  delete elements["new-id2"];
+  assert_equals(elements["new-id2"], 5);
+
+  assert_throws(new TypeError(), function() {
+    "use strict";
+    delete elements["new-id2"];
+  });
+  assert_equals(elements["new-id2"], 5);
+}, 'Trying to set a non-configurable expando that shadows a named property that gets added later');
 </script>
--- a/testing/web-platform/tests/html/semantics/forms/the-form-element/form-indexed-element.html
+++ b/testing/web-platform/tests/html/semantics/forms/the-form-element/form-indexed-element.html
@@ -13,10 +13,33 @@
 </div>
 <script>
 test(function() {
   var form = document.getElementById("form");
   assert_equals(form[0], document.getElementById("r1"));
   assert_equals(form[1], document.getElementById("r2"));
   assert_equals(form[2], undefined);
   assert_equals(form[-1], undefined);
-  }, "form.elements should be accessed correctly by index")
+}, "form.elements should be accessed correctly by index")
+
+test(function(){
+  var form = document.getElementById("form");
+  var old_item = form[0];
+  var old_desc = Object.getOwnPropertyDescriptor(form, 0);
+  assert_equals(old_desc.value, old_item);
+  assert_true(old_desc.enumerable);
+  assert_true(old_desc.configurable);
+  assert_false(old_desc.writable);
+
+  Object.prototype[0] = 5;
+  this.add_cleanup(function () { delete Object.prototype[0]; });
+  assert_equals(form[0], old_item);
+
+  delete form[0];
+  assert_equals(form[0], old_item);
+
+  assert_throws(new TypeError(), function() {
+    "use strict";
+    delete form[0];
+  });
+  assert_equals(form[0], old_item);
+}, 'Trying to delete an indexed property name should never work');
 </script>
--- a/testing/web-platform/tests/html/semantics/forms/the-form-element/form-nameditem.html
+++ b/testing/web-platform/tests/html/semantics/forms/the-form-element/form-nameditem.html
@@ -242,9 +242,89 @@ test(function() {
   f.id = "f"
   var g = f.appendChild(document.createElement("form"))
   g.id = "g"
   var input = g.appendChild(document.createElement("input"))
   input.name = "x"
   assert_equals(f.x, undefined)
   assert_equals(g.x, input)
 }, "Input should only be a named property on the innermost form that contains it")
+
+test(function() {
+  var form = document.getElementsByTagName("form")[1];
+  var old_item = form["l1"];
+  var old_desc = Object.getOwnPropertyDescriptor(form, "l1");
+  assert_equals(old_desc.value, old_item);
+  assert_false(old_desc.enumerable);
+  assert_true(old_desc.configurable);
+  assert_false(old_desc.writable);
+
+  form["l1"] = 5;
+  assert_equals(form["l1"], old_item);
+  assert_throws(new TypeError(), function() {
+    "use strict";
+    form["l1"] = 5;
+  });
+  assert_throws(new TypeError(), function() {
+    Object.defineProperty(form, "l1", { value: 5 });
+  });
+
+  delete form["l1"];
+  assert_equals(form["l1"], old_item);
+
+  assert_throws(new TypeError(), function() {
+    "use strict";
+    delete form["l1"];
+  });
+  assert_equals(form["l1"], old_item);
+
+}, 'Trying to set an expando that would shadow an already-existing named property');
+
+test(function() {
+  var form = document.getElementsByTagName("form")[1];
+  var old_item = form["new-name"];
+  var old_desc = Object.getOwnPropertyDescriptor(form, "new-name");
+  assert_equals(old_item, undefined);
+  assert_equals(old_desc, undefined);
+
+  form["new-name"] = 5;
+  assert_equals(form["new-name"], 5);
+
+  var input = document.createElement("input");
+  this.add_cleanup(function () {input.remove();});
+  input.name = "new-name";
+  form.appendChild(input);
+
+  assert_equals(form["new-name"], 5);
+
+  delete form["new-name"];
+  assert_equals(form["new-name"], input);
+}, 'Trying to set an expando that shadows a named property that gets added later');
+
+test(function() {
+  var form = document.getElementsByTagName("form")[1];
+  var old_item = form["new-name2"];
+  var old_desc = Object.getOwnPropertyDescriptor(form, "new-name2");
+  assert_equals(old_item, undefined);
+  assert_equals(old_desc, undefined);
+
+  Object.defineProperty(form, "new-name2", { configurable: false, writable:
+                                              false, value: 5 });
+  assert_equals(form["new-name2"], 5);
+
+  var input = document.createElement("input");
+  this.add_cleanup(function () {input.remove();});
+  input.name = "new-name2";
+  form.appendChild(input);
+
+  assert_equals(form["new-name2"], 5);
+
+  delete form["new-name2"];
+  assert_equals(form["new-name2"], 5);
+
+  assert_throws(new TypeError(), function() {
+    "use strict";
+    delete form["new-name2"];
+  });
+  assert_equals(form["new-name2"], 5);
+}, 'Trying to set a non-configurable expando that shadows a named property that gets added later');
+
 </script>