Bug 1546467 - Preserve reflectors of custom elements when they already have one. r=bzbarsky
authorEmilio Cobos Álvarez <emilio@crisal.io>
Tue, 23 Apr 2019 21:01:39 +0000
changeset 470558 db8e013ac78c8b0b4436fd86f867ce75e0064b5a
parent 470557 fd2bf318a8b29e7c1ab67b985c19c2c218a76d6e
child 470559 f5d44a3e3a7a45700df23849b5741836fb94ba8f
push id35908
push useraciure@mozilla.com
push dateWed, 24 Apr 2019 04:28:40 +0000
treeherdermozilla-central@c9f0730a57a6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbzbarsky
bugs1546467
milestone68.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 1546467 - Preserve reflectors of custom elements when they already have one. r=bzbarsky Differential Revision: https://phabricator.services.mozilla.com/D28542
dom/base/test/mochitest.ini
dom/base/test/test_custom_element_reflector.html
dom/bindings/BindingUtils.cpp
--- a/dom/base/test/mochitest.ini
+++ b/dom/base/test/mochitest.ini
@@ -644,16 +644,17 @@ skip-if = toolkit == 'android' && !e10s 
 [test_createHTMLDocument.html]
 [test_data_uri.html]
 skip-if = verify
 [test_document.all_iteration.html]
 [test_document.all_unqualified.html]
 [test_document_constructor.html]
 [test_document_importNode_document.html]
 [test_custom_element.html]
+[test_custom_element_reflector.html]
 [test_domparser_null_char.html]
 [test_domparsing.html]
 [test_domrequest.html]
 [test_domwindowutils.html]
 skip-if = toolkit == 'android' && !is_fennec # Bug 1525959
 [test_element.matches.html]
 [test_element_closest.html]
 [test_elementTraversal.html]
new file mode 100644
--- /dev/null
+++ b/dom/base/test/test_custom_element_reflector.html
@@ -0,0 +1,27 @@
+<!doctype html>
+<title>Custom Elements don't lose their reflectors</title>
+<script src="/tests/SimpleTest/SimpleTest.js"></script>
+<!-- First tests upgrading with an existing reflector, second without -->
+<custom-element></custom-element>
+<custom-element></custom-element>
+<script>
+  (function() {
+    // Ensure we create a reflector for the first element before-hand.
+    let firstElement = document.querySelector("custom-element");
+  }());
+
+  customElements.define("custom-element", class MyCustomElement extends HTMLElement {
+    myFunction() {
+      // Do nothing
+    }
+  });
+
+  ok(!!document.querySelector("custom-element").myFunction, "Has the right prototype before GC");;
+  ok(!!document.querySelectorAll("custom-element")[1].myFunction, "Has the right prototype before GC");;
+
+  SpecialPowers.forceCC();
+  SpecialPowers.forceGC();
+
+  ok(!!document.querySelector("custom-element").myFunction, "Has the right prototype after GC");;
+  ok(!!document.querySelectorAll("custom-element")[1].myFunction, "Has the right prototype before GC");;
+</script>
--- a/dom/bindings/BindingUtils.cpp
+++ b/dom/bindings/BindingUtils.cpp
@@ -3789,27 +3789,29 @@ bool HTMLConstructor(JSContext* aCx, uns
     // Step 10.
     if (element == ALREADY_CONSTRUCTED_MARKER) {
       return Throw(aCx, NS_ERROR_DOM_INVALID_STATE_ERR);
     }
 
     // Step 11.
     // Do prototype swizzling for upgrading a custom element here, for cases
     // when we have a reflector already.  If we don't have one yet, we will
-    // create it with the right proto (by calling DoGetOrCreateDOMReflector with
-    // that proto).
+    // create it with the right proto (by calling GetOrCreateDOMReflector with
+    // that proto), and will preserve it by means of the proto != canonicalProto
+    // check).
     JS::Rooted<JSObject*> reflector(aCx, element->GetWrapper());
     if (reflector) {
       // reflector might be in different realm.
       JSAutoRealm ar(aCx, reflector);
       JS::Rooted<JSObject*> givenProto(aCx, desiredProto);
       if (!JS_WrapObject(aCx, &givenProto) ||
           !JS_SetPrototype(aCx, reflector, givenProto)) {
         return false;
       }
+      PreserveWrapper(element.get());
     }
 
     // Step 12.
     constructionStack.LastElement() = ALREADY_CONSTRUCTED_MARKER;
   }
 
   // Tail end of step 8 and step 13: returning the element.  We want to do this
   // part in the global's realm, though in practice it won't matter much