Bug 1585819 - Restrict loading XBL bindings from WrapObject to the little amount of XUL elements that remain with bindings. r=bzbarsky

People keep shooting themselves in the feet because of this codepath and writing
slow code.

There are just a few elements with bindings left, so just check those.

Also simplify a bit the code. The XUL element + tagname check should be pretty
fast now, and ComputedStyle objects no longer keep weak pointers to pres
contexts and such, so can be safely kept on a RefPtr across an style flush.

Differential Revision: https://phabricator.services.mozilla.com/D47995
This commit is contained in:
Emilio Cobos Álvarez
2019-10-03 02:31:16 +00:00
parent fddd20afaf
commit 4056a1d27d
4 changed files with 37 additions and 89 deletions

View File

@@ -371,16 +371,6 @@ class CustomElementRegistry final : public nsISupports, public nsWrapperCache {
}; };
public: public:
/**
* Returns whether there's a definition that is likely to match this type
* atom. This is not exact, so should only be used for optimization, but it's
* good enough to prove that the chrome code doesn't need an XBL binding.
*/
bool IsLikelyToBeCustomElement(nsAtom* aTypeAtom) const {
return mCustomDefinitions.GetWeak(aTypeAtom) ||
mElementCreationCallbacks.GetWeak(aTypeAtom);
}
/** /**
* Looking up a custom element definition. * Looking up a custom element definition.
* https://html.spec.whatwg.org/#look-up-a-custom-element-definition * https://html.spec.whatwg.org/#look-up-a-custom-element-definition

View File

@@ -517,48 +517,26 @@ void Element::ClearStyleStateLocks() {
NotifyStyleStateChange(locks.mLocks); NotifyStyleStateChange(locks.mLocks);
} }
static bool IsLikelyCustomElement(const nsXULElement& aElement) { static bool MayNeedToLoadXBLBinding(const Element& aElement) {
const CustomElementData* data = aElement.GetCustomElementData(); if (!aElement.IsAnyOfXULElements(nsGkAtoms::panel, nsGkAtoms::textbox,
if (!data) { nsGkAtoms::arrowscrollbox)) {
// Other elements no longer have XBL bindings. Please don't add to the list
// above unless completely necessary.
return false; return false;
} }
if (!aElement.IsInComposedDoc()) {
const CustomElementRegistry* registry =
nsContentUtils::GetCustomElementRegistry(aElement.OwnerDoc());
if (!registry) {
return false;
}
return registry->IsLikelyToBeCustomElement(data->GetCustomElementType());
}
static bool MayNeedToLoadXBLBinding(const Document& aDocument,
const Element& aElement) {
auto* xulElem = nsXULElement::FromNode(aElement);
if (!xulElem) {
return false; return false;
} }
// If we have a frame, the frame has already loaded the binding. // If we have a frame, the frame has already loaded the binding.
// Otherwise, don't do anything else here unless we're dealing with XUL. if (aElement.GetPrimaryFrame() || !aElement.OwnerDoc()->GetPresShell()) {
if (!aDocument.GetPresShell() || aElement.GetPrimaryFrame()) {
return false; return false;
} }
return !IsLikelyCustomElement(*xulElem); // If we have a binding, well..
} if (aElement.GetXBLBinding()) {
return false;
StyleUrlOrNone Element::GetBindingURL(Document* aDocument) {
if (!MayNeedToLoadXBLBinding(*aDocument, *this)) {
return StyleUrlOrNone::None();
} }
// We need to try.
// Get the computed -moz-binding directly from the ComputedStyle return true;
RefPtr<ComputedStyle> style =
nsComputedDOMStyle::GetComputedStyleNoFlush(this, nullptr);
if (!style) {
return StyleUrlOrNone::None();
}
return style->StyleDisplay()->mBinding;
} }
JSObject* Element::WrapObject(JSContext* aCx, JSObject* Element::WrapObject(JSContext* aCx,
@@ -568,61 +546,42 @@ JSObject* Element::WrapObject(JSContext* aCx,
return nullptr; return nullptr;
} }
if (XRE_IsContentProcess() && !NodePrincipal()->IsSystemPrincipal()) { if (!MayNeedToLoadXBLBinding(*this)) {
// We don't use XBL in content privileged content processes.
return obj; return obj;
} }
Document* doc = GetComposedDoc(); RefPtr<ComputedStyle> style =
if (!doc) { nsComputedDOMStyle::GetComputedStyleNoFlush(this, nullptr);
// There's no baseclass that cares about this call so we just if (!style) {
// return here.
return obj; return obj;
} }
// We must ensure that the XBL Binding is installed before we hand // We have a binding that must be installed.
// back this object. const StyleUrlOrNone& computedBinding = style->StyleDisplay()->mBinding;
if (!computedBinding.IsUrl()) {
if (HasFlag(NODE_MAY_BE_IN_BINDING_MNGR) && GetXBLBinding()) {
// There's already a binding for this element so nothing left to
// be done here.
// In theory we could call ExecuteAttachedHandler here when it's safe to
// run script if we also removed the binding from the PAQ queue, but that
// seems like a scary change that would mostly just add more
// inconsistencies.
return obj; return obj;
} }
// Make sure the ComputedStyle goes away _before_ we load the binding auto& url = computedBinding.AsUrl();
// since that can destroy the relevant presshell. nsCOMPtr<nsIURI> uri = url.GetURI();
nsCOMPtr<nsIPrincipal> principal = url.ExtraData().Principal();
{ nsXBLService* xblService = nsXBLService::GetInstance();
StyleUrlOrNone result = GetBindingURL(doc); if (!xblService) {
if (result.IsUrl()) { dom::Throw(aCx, NS_ERROR_NOT_AVAILABLE);
auto& url = result.AsUrl(); return nullptr;
nsCOMPtr<nsIURI> uri = url.GetURI(); }
nsCOMPtr<nsIPrincipal> principal = url.ExtraData().Principal();
// We have a binding that must be installed. RefPtr<nsXBLBinding> binding;
nsXBLService* xblService = nsXBLService::GetInstance(); xblService->LoadBindings(this, uri, principal, getter_AddRefs(binding));
if (!xblService) {
dom::Throw(aCx, NS_ERROR_NOT_AVAILABLE);
return nullptr;
}
RefPtr<nsXBLBinding> binding; if (binding) {
xblService->LoadBindings(this, uri, principal, getter_AddRefs(binding)); if (nsContentUtils::IsSafeToRunScript()) {
binding->ExecuteAttachedHandler();
if (binding) { } else {
if (nsContentUtils::IsSafeToRunScript()) { nsContentUtils::AddScriptRunner(
binding->ExecuteAttachedHandler(); NewRunnableMethod("nsXBLBinding::ExecuteAttachedHandler", binding,
} else { &nsXBLBinding::ExecuteAttachedHandler));
nsContentUtils::AddScriptRunner(
NewRunnableMethod("nsXBLBinding::ExecuteAttachedHandler", binding,
&nsXBLBinding::ExecuteAttachedHandler));
}
}
} }
} }

View File

@@ -459,8 +459,6 @@ class Element : public FragmentOrElement {
} }
} }
mozilla::StyleUrlOrNone GetBindingURL(Document* aDocument);
Directionality GetComputedDirectionality() const; Directionality GetComputedDirectionality() const;
static const uint32_t kAllServoDescendantBits = static const uint32_t kAllServoDescendantBits =

View File

@@ -143,6 +143,7 @@ STATIC_ATOMS = [
Atom("aria_valuemin", "aria-valuemin"), Atom("aria_valuemin", "aria-valuemin"),
Atom("aria_valuenow", "aria-valuenow"), Atom("aria_valuenow", "aria-valuenow"),
Atom("arrow", "arrow"), Atom("arrow", "arrow"),
Atom("arrowscrollbox", "arrowscrollbox"),
Atom("article", "article"), Atom("article", "article"),
Atom("as", "as"), Atom("as", "as"),
Atom("ascending", "ascending"), Atom("ascending", "ascending"),