From e787289888b945e5c11019cd3f6cc4a205ecd3c6 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Thu, 11 Jul 2013 11:58:28 -0400 Subject: [PATCH] Bug 890193. Make the XML prettyprinter actually drop its binding when it unhooks, and remove the now-dead concept of "style binding" in the process. r=mrbkap --- content/xbl/src/nsBindingManager.cpp | 5 - content/xbl/src/nsXBLBinding.cpp | 159 ++++++++++++--------------- content/xbl/src/nsXBLBinding.h | 5 - content/xbl/src/nsXBLService.cpp | 34 +++--- 4 files changed, 86 insertions(+), 117 deletions(-) diff --git a/content/xbl/src/nsBindingManager.cpp b/content/xbl/src/nsBindingManager.cpp index fbcce395eeb..e8ff10e1b36 100644 --- a/content/xbl/src/nsBindingManager.cpp +++ b/content/xbl/src/nsBindingManager.cpp @@ -442,11 +442,6 @@ nsBindingManager::ClearBinding(nsIContent* aContent) // For now we can only handle removing a binding if it's the only one NS_ENSURE_FALSE(binding->GetBaseBinding(), NS_ERROR_FAILURE); - // Make sure it isn't a style binding - if (binding->IsStyleBinding()) { - return NS_OK; - } - // Hold strong ref in case removing the binding tries to close the // window or something. // XXXbz should that be ownerdoc? Wouldn't we need a ref to the diff --git a/content/xbl/src/nsXBLBinding.cpp b/content/xbl/src/nsXBLBinding.cpp index 03f2be7dbbb..a361a396a61 100644 --- a/content/xbl/src/nsXBLBinding.cpp +++ b/content/xbl/src/nsXBLBinding.cpp @@ -142,8 +142,7 @@ nsXBLJSClass::Destroy() // Constructors/Destructors nsXBLBinding::nsXBLBinding(nsXBLPrototypeBinding* aBinding) - : mIsStyleBinding(true), - mMarkedForDeath(false), + : mMarkedForDeath(false), mPrototypeBinding(aBinding) { NS_ASSERTION(mPrototypeBinding, "Must have a prototype binding!"); @@ -714,95 +713,92 @@ nsXBLBinding::ChangeDocument(nsIDocument* aOldDocument, nsIDocument* aNewDocumen if (aOldDocument == aNewDocument) return; - // Only style bindings get their prototypes unhooked. First do ourselves. - if (mIsStyleBinding) { - // Now the binding dies. Unhook our prototypes. - if (mPrototypeBinding->HasImplementation()) { - nsCOMPtr global = do_QueryInterface( - aOldDocument->GetScopeObject()); - if (global) { - nsCOMPtr context = global->GetContext(); - if (context) { - JSContext *cx = context->GetNativeContext(); + // Now the binding dies. Unhook our prototypes. + if (mPrototypeBinding->HasImplementation()) { + nsCOMPtr global = do_QueryInterface( + aOldDocument->GetScopeObject()); + if (global) { + nsCOMPtr context = global->GetContext(); + if (context) { + JSContext *cx = context->GetNativeContext(); - nsCxPusher pusher; - pusher.Push(cx); + nsCxPusher pusher; + pusher.Push(cx); - // scope might be null if we've cycle-collected the global - // object, since the Unlink phase of cycle collection happens - // after JS GC finalization. But in that case, we don't care - // about fixing the prototype chain, since everything's going - // away immediately. - JS::Rooted scope(cx, global->GetGlobalJSObject()); - JS::Rooted scriptObject(cx, mBoundElement->GetWrapper()); - if (scope && scriptObject) { - // XXX Stay in sync! What if a layered binding has an - // ?! - // XXXbz what does that comment mean, really? It seems to date - // back to when there was such a thing as an , whever - // that was... + // scope might be null if we've cycle-collected the global + // object, since the Unlink phase of cycle collection happens + // after JS GC finalization. But in that case, we don't care + // about fixing the prototype chain, since everything's going + // away immediately. + JS::Rooted scope(cx, global->GetGlobalJSObject()); + JS::Rooted scriptObject(cx, mBoundElement->GetWrapper()); + if (scope && scriptObject) { + // XXX Stay in sync! What if a layered binding has an + // ?! + // XXXbz what does that comment mean, really? It seems to date + // back to when there was such a thing as an , whever + // that was... - // Find the right prototype. - JSAutoCompartment ac(cx, scriptObject); + // Find the right prototype. + JSAutoCompartment ac(cx, scriptObject); - JS::Rooted base(cx, scriptObject); - JS::Rooted proto(cx); - for ( ; true; base = proto) { // Will break out on null proto - if (!JS_GetPrototype(cx, base, proto.address())) { - return; - } - if (!proto) { - break; - } - - JSClass* clazz = ::JS_GetClass(proto); - if (!clazz || - (~clazz->flags & - (JSCLASS_HAS_PRIVATE | JSCLASS_PRIVATE_IS_NSISUPPORTS)) || - JSCLASS_RESERVED_SLOTS(clazz) != 1 || - clazz->finalize != XBLFinalize) { - // Clearly not the right class - continue; - } - - nsRefPtr docInfo = - static_cast(::JS_GetPrivate(proto)); - if (!docInfo) { - // Not the proto we seek - continue; - } - - JS::Value protoBinding = ::JS_GetReservedSlot(proto, 0); - - if (JSVAL_TO_PRIVATE(protoBinding) != mPrototypeBinding) { - // Not the right binding - continue; - } - - // Alright! This is the right prototype. Pull it out of the - // proto chain. - JS::Rooted grandProto(cx); - if (!JS_GetPrototype(cx, proto, grandProto.address())) { - return; - } - ::JS_SetPrototype(cx, base, grandProto); + JS::Rooted base(cx, scriptObject); + JS::Rooted proto(cx); + for ( ; true; base = proto) { // Will break out on null proto + if (!JS_GetPrototype(cx, base, proto.address())) { + return; + } + if (!proto) { break; } - mPrototypeBinding->UndefineFields(cx, scriptObject); + JSClass* clazz = ::JS_GetClass(proto); + if (!clazz || + (~clazz->flags & + (JSCLASS_HAS_PRIVATE | JSCLASS_PRIVATE_IS_NSISUPPORTS)) || + JSCLASS_RESERVED_SLOTS(clazz) != 1 || + clazz->finalize != XBLFinalize) { + // Clearly not the right class + continue; + } - // Don't remove the reference from the document to the - // wrapper here since it'll be removed by the element - // itself when that's taken out of the document. + nsRefPtr docInfo = + static_cast(::JS_GetPrivate(proto)); + if (!docInfo) { + // Not the proto we seek + continue; + } + + JS::Value protoBinding = ::JS_GetReservedSlot(proto, 0); + + if (JSVAL_TO_PRIVATE(protoBinding) != mPrototypeBinding) { + // Not the right binding + continue; + } + + // Alright! This is the right prototype. Pull it out of the + // proto chain. + JS::Rooted grandProto(cx); + if (!JS_GetPrototype(cx, proto, grandProto.address())) { + return; + } + ::JS_SetPrototype(cx, base, grandProto); + break; } + + mPrototypeBinding->UndefineFields(cx, scriptObject); + + // Don't remove the reference from the document to the + // wrapper here since it'll be removed by the element + // itself when that's taken out of the document. } } } - - // Remove our event handlers - UnhookEventHandlers(); } + // Remove our event handlers + UnhookEventHandlers(); + { nsAutoScriptBlocker scriptBlocker; @@ -1076,15 +1072,6 @@ nsXBLBinding::RootBinding() return this; } -nsXBLBinding* -nsXBLBinding::GetFirstStyleBinding() -{ - if (mIsStyleBinding) - return this; - - return mNextBinding ? mNextBinding->GetFirstStyleBinding() : nullptr; -} - bool nsXBLBinding::ResolveAllFields(JSContext *cx, JS::Handle obj) const { diff --git a/content/xbl/src/nsXBLBinding.h b/content/xbl/src/nsXBLBinding.h index 49bb885fff6..072c4e76a20 100644 --- a/content/xbl/src/nsXBLBinding.h +++ b/content/xbl/src/nsXBLBinding.h @@ -73,9 +73,6 @@ public: mJSClass = aClass; } - bool IsStyleBinding() const { return mIsStyleBinding; } - void SetIsStyleBinding(bool aIsStyle) { mIsStyleBinding = aIsStyle; } - /* * Does a lookup for a method or attribute provided by one of the bindings' * prototype implementation. If found, |desc| will be set up appropriately, @@ -122,7 +119,6 @@ public: nsIAtom* GetBaseTag(int32_t* aNameSpaceID); nsXBLBinding* RootBinding(); - nsXBLBinding* GetFirstStyleBinding(); // Resolve all the fields for this binding and all ancestor bindings on the // object |obj|. False return means a JS exception was set. @@ -166,7 +162,6 @@ public: // MEMBER VARIABLES protected: - bool mIsStyleBinding; bool mMarkedForDeath; nsXBLPrototypeBinding* mPrototypeBinding; // Weak, but we're holding a ref to the docinfo diff --git a/content/xbl/src/nsXBLService.cpp b/content/xbl/src/nsXBLService.cpp index 8a14ee31d89..84ff104ac25 100644 --- a/content/xbl/src/nsXBLService.cpp +++ b/content/xbl/src/nsXBLService.cpp @@ -455,19 +455,16 @@ nsXBLService::LoadBindings(nsIContent* aContent, nsIURI* aURL, nsXBLBinding *binding = bindingManager->GetBinding(aContent); if (binding) { - nsXBLBinding *styleBinding = binding->GetFirstStyleBinding(); - if (styleBinding) { - if (binding->MarkedForDeath()) { - FlushStyleBindings(aContent); - binding = nullptr; - } - else { - // See if the URIs match. - if (styleBinding->PrototypeBinding()->CompareBindingURI(aURL)) - return NS_OK; - FlushStyleBindings(aContent); - binding = nullptr; - } + if (binding->MarkedForDeath()) { + FlushStyleBindings(aContent); + binding = nullptr; + } + else { + // See if the URIs match. + if (binding->PrototypeBinding()->CompareBindingURI(aURL)) + return NS_OK; + FlushStyleBindings(aContent); + binding = nullptr; } } @@ -537,15 +534,10 @@ nsXBLService::FlushStyleBindings(nsIContent* aContent) nsXBLBinding *binding = bindingManager->GetBinding(aContent); if (binding) { - nsXBLBinding *styleBinding = binding->GetFirstStyleBinding(); + // Clear out the script references. + binding->ChangeDocument(document, nullptr); - if (styleBinding) { - // Clear out the script references. - styleBinding->ChangeDocument(document, nullptr); - } - - if (styleBinding == binding) - bindingManager->SetBinding(aContent, nullptr); // Flush old style bindings + bindingManager->SetBinding(aContent, nullptr); // Flush old style bindings } return NS_OK;