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

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Emilio Cobos Álvarez 2019-10-03 02:31:16 +00:00
parent c09aa065ce
commit d4d8e98bd6
4 changed files with 37 additions and 89 deletions

View File

@ -371,16 +371,6 @@ class CustomElementRegistry final : public nsISupports, public nsWrapperCache {
};
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.
* https://html.spec.whatwg.org/#look-up-a-custom-element-definition

View File

@ -517,48 +517,26 @@ void Element::ClearStyleStateLocks() {
NotifyStyleStateChange(locks.mLocks);
}
static bool IsLikelyCustomElement(const nsXULElement& aElement) {
const CustomElementData* data = aElement.GetCustomElementData();
if (!data) {
static bool MayNeedToLoadXBLBinding(const Element& aElement) {
if (!aElement.IsAnyOfXULElements(nsGkAtoms::panel, nsGkAtoms::textbox,
nsGkAtoms::arrowscrollbox)) {
// Other elements no longer have XBL bindings. Please don't add to the list
// above unless completely necessary.
return false;
}
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) {
if (!aElement.IsInComposedDoc()) {
return false;
}
// 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 (!aDocument.GetPresShell() || aElement.GetPrimaryFrame()) {
if (aElement.GetPrimaryFrame() || !aElement.OwnerDoc()->GetPresShell()) {
return false;
}
return !IsLikelyCustomElement(*xulElem);
}
StyleUrlOrNone Element::GetBindingURL(Document* aDocument) {
if (!MayNeedToLoadXBLBinding(*aDocument, *this)) {
return StyleUrlOrNone::None();
// If we have a binding, well..
if (aElement.GetXBLBinding()) {
return false;
}
// Get the computed -moz-binding directly from the ComputedStyle
RefPtr<ComputedStyle> style =
nsComputedDOMStyle::GetComputedStyleNoFlush(this, nullptr);
if (!style) {
return StyleUrlOrNone::None();
}
return style->StyleDisplay()->mBinding;
// We need to try.
return true;
}
JSObject* Element::WrapObject(JSContext* aCx,
@ -568,61 +546,42 @@ JSObject* Element::WrapObject(JSContext* aCx,
return nullptr;
}
if (XRE_IsContentProcess() && !NodePrincipal()->IsSystemPrincipal()) {
// We don't use XBL in content privileged content processes.
if (!MayNeedToLoadXBLBinding(*this)) {
return obj;
}
Document* doc = GetComposedDoc();
if (!doc) {
// There's no baseclass that cares about this call so we just
// return here.
RefPtr<ComputedStyle> style =
nsComputedDOMStyle::GetComputedStyleNoFlush(this, nullptr);
if (!style) {
return obj;
}
// We must ensure that the XBL Binding is installed before we hand
// back this object.
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.
// We have a binding that must be installed.
const StyleUrlOrNone& computedBinding = style->StyleDisplay()->mBinding;
if (!computedBinding.IsUrl()) {
return obj;
}
// Make sure the ComputedStyle goes away _before_ we load the binding
// since that can destroy the relevant presshell.
auto& url = computedBinding.AsUrl();
nsCOMPtr<nsIURI> uri = url.GetURI();
nsCOMPtr<nsIPrincipal> principal = url.ExtraData().Principal();
{
StyleUrlOrNone result = GetBindingURL(doc);
if (result.IsUrl()) {
auto& url = result.AsUrl();
nsCOMPtr<nsIURI> uri = url.GetURI();
nsCOMPtr<nsIPrincipal> principal = url.ExtraData().Principal();
nsXBLService* xblService = nsXBLService::GetInstance();
if (!xblService) {
dom::Throw(aCx, NS_ERROR_NOT_AVAILABLE);
return nullptr;
}
// We have a binding that must be installed.
nsXBLService* xblService = nsXBLService::GetInstance();
if (!xblService) {
dom::Throw(aCx, NS_ERROR_NOT_AVAILABLE);
return nullptr;
}
RefPtr<nsXBLBinding> binding;
xblService->LoadBindings(this, uri, principal, getter_AddRefs(binding));
RefPtr<nsXBLBinding> binding;
xblService->LoadBindings(this, uri, principal, getter_AddRefs(binding));
if (binding) {
if (nsContentUtils::IsSafeToRunScript()) {
binding->ExecuteAttachedHandler();
} else {
nsContentUtils::AddScriptRunner(
NewRunnableMethod("nsXBLBinding::ExecuteAttachedHandler", binding,
&nsXBLBinding::ExecuteAttachedHandler));
}
}
if (binding) {
if (nsContentUtils::IsSafeToRunScript()) {
binding->ExecuteAttachedHandler();
} else {
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;
static const uint32_t kAllServoDescendantBits =

View File

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