Bug 1294747. Make sure we expose the expando of a [OverrideBuiltins] proxy to active JS when it gets cleared from the proxy. r=peterv

This should also fix bug 1296775 and bug 1290359.

There's a very good chance it will also fix bug 1293386, bug 1292855, bug
1289452, and bug 1303340: those would get hit if we happened to start _another_
gc after the expando died but while it was still in the Rooted.  All of them
seem to be dying under the domClass->mGetProto call, which could finish up a GC
that kills the expando and then do _another_ one, causing the Rooted to try to
mark a dead object.

dom/base/nsWrapperCache.cpp

@@ -55,7 +55,7 @@ nsWrapperCache::ReleaseWrapper(void* aScriptObjectHolder)
     // from both here.
     JSObject* obj = GetWrapperPreserveColor();
     if (IsDOMBinding() && obj && js::IsProxy(obj)) {
-      DOMProxyHandler::GetAndClearExpandoObject(obj);
+      DOMProxyHandler::ClearExternalRefsForWrapperRelease(obj);
     }
     SetPreservingWrapper(false);
     cyclecollector::DropJSObjectsImpl(aScriptObjectHolder);

dom/bindings/DOMJSProxyHandler.cpp

@@ -70,6 +70,33 @@ struct SetDOMProxyInformation
 
 SetDOMProxyInformation gSetDOMProxyInformation;
 
+// static
+void
+DOMProxyHandler::ClearExternalRefsForWrapperRelease(JSObject* obj)
+{
+  MOZ_ASSERT(IsDOMProxy(obj), "expected a DOM proxy object");
+  JS::Value v = js::GetProxyExtra(obj, JSPROXYSLOT_EXPANDO);
+  if (v.isUndefined()) {
+    // No expando.
+    return;
+  }
+
+  // See EnsureExpandoObject for the work we're trying to undo here.
+
+  if (v.isObject()) {
+    // Drop us from the DOM expando hashtable.  Don't worry about clearing our
+    // slot reference to the expando; we're about to die anyway.
+    xpc::ObjectScope(obj)->RemoveDOMExpandoObject(obj);
+    return;
+  }
+
+  // Prevent having a dangling pointer to our expando from the
+  // ExpandoAndGeneration.
+  js::ExpandoAndGeneration* expandoAndGeneration =
+    static_cast<js::ExpandoAndGeneration*>(v.toPrivate());
+  expandoAndGeneration->expando = UndefinedValue();
+}
+
 // static
 JSObject*
 DOMProxyHandler::GetAndClearExpandoObject(JSObject* obj)
@@ -90,6 +117,19 @@ DOMProxyHandler::GetAndClearExpandoObject(JSObject* obj)
     if (v.isUndefined()) {
       return nullptr;
     }
+    // We have to expose v to active JS here.  The reason for that is that we
+    // might be in the middle of a GC right now.  If our proxy hasn't been
+    // traced yet, when it _does_ get traced it won't trace the expando, since
+    // we're breaking that link.  But the Rooted we're presumably being placed
+    // into is also not going to trace us, because Rooted marking is done at
+    // the very beginning of the GC.  In that situation, we need to manually
+    // mark the expando as live here.  JS::ExposeValueToActiveJS will do just
+    // that for us.
+    //
+    // We don't need to do this in the non-expandoAndGeneration case, because
+    // in that case our value is stored in a slot and slots will already mark
+    // the old thing live when the value in the slot changes.
+    JS::ExposeValueToActiveJS(v);
     expandoAndGeneration->expando = UndefinedValue();
   }
 

dom/bindings/DOMJSProxyHandler.h

@@ -132,10 +132,35 @@ public:
   virtual bool setCustom(JSContext* cx, JS::Handle<JSObject*> proxy, JS::Handle<jsid> id,
                          JS::Handle<JS::Value> v, bool *done) const;
 
+  /*
+   * Get the expando object for the given DOM proxy.
+   */
   static JSObject* GetExpandoObject(JSObject* obj);
 
-  /* GetAndClearExpandoObject does not DROP or clear the preserving wrapper flag. */
+  /*
+   * Clear the "external references" to this object.  If you are not
+   * nsWrapperCAche::ReleaseWrapper, you do NOT want to be calling this method.
+   *
+   * XXXbz if we nixed the DOM expando hash and just had a finalizer that
+   * cleared out the value in the ExpandoAndGeneration in the shadowing case,
+   * could we just get rid of this function altogether?
+   */
+  static void ClearExternalRefsForWrapperRelease(JSObject* obj);
+
+  /*
+   * Clear the expando object for the given DOM proxy and return it.  This
+   * function will ensure that the returned object is exposed to active JS if
+   * the given object is exposed.
+   *
+   * GetAndClearExpandoObject does not DROP or clear the preserving wrapper
+   * flag.
+   */
   static JSObject* GetAndClearExpandoObject(JSObject* obj);
+
+  /*
+   * Ensure that the given proxy (obj) has an expando object, and return it.
+   * Returns null on failure.
+   */
   static JSObject* EnsureExpandoObject(JSContext* cx,
                                        JS::Handle<JSObject*> obj);