Bug 1189822 part 4. Fix finalization for DOM overridebuildins proxies to not clear the expando if it's for a different reflector. r=peterv, sfink
authorBoris Zbarsky <bzbarsky@mit.edu>
Wed, 17 May 2017 00:52:53 -0400
changeset 358627 793ab87511f5f4a77388cb5f7293251e9955afd9
parent 358626 5ccd27cf7166db5cdee297abcc8b5fd0ed4d81c6
child 358628 30f6ea5ad9e6d3891b12ab8a72e1e10dc0bd82c4
push id90361
push userbzbarsky@mozilla.com
push dateWed, 17 May 2017 04:54:18 +0000
treeherdermozilla-inbound@159f82e6813c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspeterv, sfink
bugs1189822, 1352430
milestone55.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 1189822 part 4. Fix finalization for DOM overridebuildins proxies to not clear the expando if it's for a different reflector. r=peterv, sfink This is something that should have happened in bug 1352430 but was missed there.
dom/bindings/Codegen.py
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -1651,20 +1651,40 @@ class CGAddPropertyHook(CGAbstractClassH
               PreserveWrapper(self);
             }
             return true;
             """)
 
 
 def finalizeHook(descriptor, hookName, freeOp, obj):
     finalize = ""
+    if descriptor.interface.getExtendedAttribute('OverrideBuiltins'):
+        finalize += fill(
+            """
+            // Either our proxy created an expando object or not.  If it did,
+            // then we would have preserved ourselves, and hence if we're going
+            // away so is our C++ object and we should reset its expando value.
+            // It's possible that in this situation the C++ object's reflector
+            // pointer has been nulled out, but if not it's pointing to us.  If
+            // our proxy did _not_ create an expando object then it's possible
+            // that we're no longer the reflector for our C++ object (and
+            // incremental finalization is finally getting to us), and that in
+            // the meantime the new reflector has created an expando object.
+            // In that case we do NOT want to clear the expando pointer in the
+            // C++ object.
+            //
+            // It's important to do this before we ClearWrapper, of course.
+            JSObject* reflector = self->GetWrapperMaybeDead();
+            if (!reflector || reflector == ${obj}) {
+              self->mExpandoAndGeneration.expando = JS::UndefinedValue();
+            }
+            """,
+            obj=obj)
     if descriptor.wrapperCache:
         finalize += "ClearWrapper(self, self, %s);\n" % obj
-    if descriptor.interface.getExtendedAttribute('OverrideBuiltins'):
-        finalize += "self->mExpandoAndGeneration.expando = JS::UndefinedValue();\n"
     if descriptor.isGlobal():
         finalize += "mozilla::dom::FinalizeGlobal(CastToJSFreeOp(%s), %s);\n" % (freeOp, obj)
     finalize += ("AddForDeferredFinalization<%s>(self);\n" %
                  descriptor.nativeType)
     return CGIfWrapper(CGGeneric(finalize), "self")
 
 
 class CGClassFinalizeHook(CGAbstractClassHook):