From 48d31ea5ebe2a4901fcb542e1e48b6fd6b67d64c Mon Sep 17 00:00:00 2001 From: Moonchild Date: Fri, 7 Aug 2020 21:15:52 +0000 Subject: Pref and disable getRootNode() This is apparently used for fallback selection and if available it is "assumed" Shadow DOM is also available, while this is a utility function. Webcompat is a nightmare sometimes. --- dom/webidl/Node.webidl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'dom') diff --git a/dom/webidl/Node.webidl b/dom/webidl/Node.webidl index 0a14e3624..128efcb50 100644 --- a/dom/webidl/Node.webidl +++ b/dom/webidl/Node.webidl @@ -38,7 +38,7 @@ interface Node : EventTarget { readonly attribute boolean isConnected; [Pure] readonly attribute Document? ownerDocument; - [Pure] + [Pure, Pref="dom.getRootNode.enabled"] Node getRootNode(optional GetRootNodeOptions options); [Pure] readonly attribute Node? parentNode; -- cgit v1.2.3 From 0bbe6ec10589b8540aa86200bf2b209ad4e563ba Mon Sep 17 00:00:00 2001 From: Andy Date: Tue, 4 Aug 2020 13:54:01 -0700 Subject: Issue #1620 - Use Intrinsic Aspect Ratio for Images (uplift) --- dom/html/HTMLImageElement.cpp | 2 +- dom/html/HTMLImageElement.h | 5 ++++ dom/html/nsGenericHTMLElement.cpp | 60 +++++++++++++++++++++++++++++---------- dom/html/nsGenericHTMLElement.h | 4 ++- 4 files changed, 54 insertions(+), 17 deletions(-) (limited to 'dom') diff --git a/dom/html/HTMLImageElement.cpp b/dom/html/HTMLImageElement.cpp index fab1cdef4..08f2404ce 100644 --- a/dom/html/HTMLImageElement.cpp +++ b/dom/html/HTMLImageElement.cpp @@ -324,7 +324,7 @@ HTMLImageElement::MapAttributesIntoRule(const nsMappedAttributes* aAttributes, nsGenericHTMLElement::MapImageAlignAttributeInto(aAttributes, aData); nsGenericHTMLElement::MapImageBorderAttributeInto(aAttributes, aData); nsGenericHTMLElement::MapImageMarginAttributeInto(aAttributes, aData); - nsGenericHTMLElement::MapImageSizeAttributesInto(aAttributes, aData); + nsGenericHTMLElement::MapImageSizeAttributesInto(aAttributes, aData, true); nsGenericHTMLElement::MapCommonAttributesInto(aAttributes, aData); } diff --git a/dom/html/HTMLImageElement.h b/dom/html/HTMLImageElement.h index 62323e801..1e63cd79c 100644 --- a/dom/html/HTMLImageElement.h +++ b/dom/html/HTMLImageElement.h @@ -206,6 +206,11 @@ public: return GetReferrerPolicyAsEnum(); } + bool IsAwaitingLoad() const + { + return !!mPendingImageLoadTask; + } + int32_t X(); int32_t Y(); // Uses XPCOM GetLowsrc. diff --git a/dom/html/nsGenericHTMLElement.cpp b/dom/html/nsGenericHTMLElement.cpp index ef077cfb2..922ba1d29 100644 --- a/dom/html/nsGenericHTMLElement.cpp +++ b/dom/html/nsGenericHTMLElement.cpp @@ -1449,29 +1449,59 @@ nsGenericHTMLElement::MapImageMarginAttributeInto(const nsMappedAttributes* aAtt void nsGenericHTMLElement::MapImageSizeAttributesInto(const nsMappedAttributes* aAttributes, - nsRuleData* aData) + nsRuleData* aData, + bool aMapAspectRatio) { if (!(aData->mSIDs & NS_STYLE_INHERIT_BIT(Position))) return; + auto* aWidth = aAttributes->GetAttr(nsGkAtoms::width); + auto* aHeight = aAttributes->GetAttr(nsGkAtoms::height); + // width: value - nsCSSValue* width = aData->ValueForWidth(); - if (width->GetUnit() == eCSSUnit_Null) { - const nsAttrValue* value = aAttributes->GetAttr(nsGkAtoms::width); - if (value && value->Type() == nsAttrValue::eInteger) - width->SetFloatValue((float)value->GetIntegerValue(), eCSSUnit_Pixel); - else if (value && value->Type() == nsAttrValue::ePercent) - width->SetPercentValue(value->GetPercentValue()); + if (aWidth) { + nsCSSValue* cWidth = aData->ValueForWidth(); + if (cWidth->GetUnit() == eCSSUnit_Null) { + if (aWidth->Type() == nsAttrValue::eInteger) + cWidth->SetFloatValue((float)aWidth->GetIntegerValue(), eCSSUnit_Pixel); + else if (aWidth->Type() == nsAttrValue::ePercent) + cWidth->SetPercentValue(aWidth->GetPercentValue()); + } } // height: value - nsCSSValue* height = aData->ValueForHeight(); - if (height->GetUnit() == eCSSUnit_Null) { - const nsAttrValue* value = aAttributes->GetAttr(nsGkAtoms::height); - if (value && value->Type() == nsAttrValue::eInteger) - height->SetFloatValue((float)value->GetIntegerValue(), eCSSUnit_Pixel); - else if (value && value->Type() == nsAttrValue::ePercent) - height->SetPercentValue(value->GetPercentValue()); + if (aHeight) { + nsCSSValue* cHeight = aData->ValueForHeight(); + if (cHeight->GetUnit() == eCSSUnit_Null) { + if (aHeight->Type() == nsAttrValue::eInteger) + cHeight->SetFloatValue((float)aHeight->GetIntegerValue(), eCSSUnit_Pixel); + else if (aHeight->Type() == nsAttrValue::ePercent) + cHeight->SetPercentValue(aHeight->GetPercentValue()); + } + } + + // 2020-07-15 (RealityRipple) Much of this is a guess based on a few sources. + // Please go over this with a fine-tooth comb before production. + if (Preferences::GetBool("layout.css.width-and-height-map-to-aspect-ratio.enabled") && + aMapAspectRatio && aWidth && aHeight) { + Maybe w; + if (aWidth->Type() == nsAttrValue::eInteger) { + w.emplace(aWidth->GetIntegerValue()); + } else if (aWidth->Type() == nsAttrValue::eDoubleValue) { + w.emplace(aWidth->GetDoubleValue()); + } + + Maybe h; + if (aHeight->Type() == nsAttrValue::eInteger) { + h.emplace(aHeight->GetIntegerValue()); + } else if (aHeight->Type() == nsAttrValue::eDoubleValue) { + h.emplace(aHeight->GetDoubleValue()); + } + + if (w && h && *w != 0 && *h != 0) { + nsCSSValue* aspect_ratio = aData->ValueForAspectRatio(); + aspect_ratio->SetFloatValue((float(*w) / float(*h)), eCSSUnit_Number); + } } } diff --git a/dom/html/nsGenericHTMLElement.h b/dom/html/nsGenericHTMLElement.h index 23fabc4e8..6d7dc0cef 100644 --- a/dom/html/nsGenericHTMLElement.h +++ b/dom/html/nsGenericHTMLElement.h @@ -712,10 +712,12 @@ public: * * @param aAttributes the list of attributes to map * @param aData the returned rule data [INOUT] + * @param aMapAspectRatio map width and height attributes on aspect-ratio * @see GetAttributeMappingFunction */ static void MapImageSizeAttributesInto(const nsMappedAttributes* aAttributes, - nsRuleData* aData); + nsRuleData* aData, + bool = false); /** * Helper to map the background attribute * into a style struct. -- cgit v1.2.3 From c75e412f17a0cfcf4036a4c7a87ad1476601cc5e Mon Sep 17 00:00:00 2001 From: Andy Date: Fri, 7 Aug 2020 14:29:41 -0700 Subject: Issue #1620 - Remove Development Comments --- dom/html/nsGenericHTMLElement.cpp | 2 -- 1 file changed, 2 deletions(-) (limited to 'dom') diff --git a/dom/html/nsGenericHTMLElement.cpp b/dom/html/nsGenericHTMLElement.cpp index 922ba1d29..afbc49b6b 100644 --- a/dom/html/nsGenericHTMLElement.cpp +++ b/dom/html/nsGenericHTMLElement.cpp @@ -1480,8 +1480,6 @@ nsGenericHTMLElement::MapImageSizeAttributesInto(const nsMappedAttributes* aAttr } } - // 2020-07-15 (RealityRipple) Much of this is a guess based on a few sources. - // Please go over this with a fine-tooth comb before production. if (Preferences::GetBool("layout.css.width-and-height-map-to-aspect-ratio.enabled") && aMapAspectRatio && aWidth && aHeight) { Maybe w; -- cgit v1.2.3 From bf95a03189d74d118d149b49ac5f45a134683fd6 Mon Sep 17 00:00:00 2001 From: Andy Date: Fri, 7 Aug 2020 14:32:59 -0700 Subject: Issue #1620 - Enable Intrinsic Ratio by Default A simpler name feels so much cleaner. --- dom/html/nsGenericHTMLElement.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'dom') diff --git a/dom/html/nsGenericHTMLElement.cpp b/dom/html/nsGenericHTMLElement.cpp index afbc49b6b..ddb476cf4 100644 --- a/dom/html/nsGenericHTMLElement.cpp +++ b/dom/html/nsGenericHTMLElement.cpp @@ -1480,7 +1480,7 @@ nsGenericHTMLElement::MapImageSizeAttributesInto(const nsMappedAttributes* aAttr } } - if (Preferences::GetBool("layout.css.width-and-height-map-to-aspect-ratio.enabled") && + if (Preferences::GetBool("layout.css.intrinsic-aspect-ratio.enabled") && aMapAspectRatio && aWidth && aHeight) { Maybe w; if (aWidth->Type() == nsAttrValue::eInteger) { -- cgit v1.2.3 From a4780ebaeb123ce9c793b85bb38a1701fad8f7ac Mon Sep 17 00:00:00 2001 From: Moonchild Date: Sun, 30 Aug 2020 09:29:45 +0000 Subject: Issue #1629 - Uplift implementation of behavior for stylesheets. --- dom/base/nsContentSink.cpp | 5 ++- dom/base/nsStyleLinkElement.cpp | 8 ++-- dom/base/nsStyleLinkElement.h | 3 +- dom/html/HTMLLinkElement.cpp | 59 ++++++++++++++++++++------ dom/html/HTMLLinkElement.h | 16 +++++-- dom/html/HTMLStyleElement.cpp | 6 ++- dom/html/HTMLStyleElement.h | 5 ++- dom/svg/SVGStyleElement.cpp | 4 +- dom/svg/SVGStyleElement.h | 3 +- dom/webidl/HTMLLinkElement.webidl | 2 +- dom/xml/XMLStylesheetProcessingInstruction.cpp | 4 +- dom/xml/XMLStylesheetProcessingInstruction.h | 3 +- 12 files changed, 87 insertions(+), 31 deletions(-) (limited to 'dom') diff --git a/dom/base/nsContentSink.cpp b/dom/base/nsContentSink.cpp index 1e6465a1b..59f4a9f9a 100644 --- a/dom/base/nsContentSink.cpp +++ b/dom/base/nsContentSink.cpp @@ -789,13 +789,14 @@ nsContentSink::ProcessStyleLink(nsIContent* aElement, // If this is a fragment parser, we don't want to observe. // We don't support CORS for processing instructions bool isAlternate; + bool isExplicitlyEnabled; rv = mCSSLoader->LoadStyleLink(aElement, url, aTitle, aMedia, aAlternate, CORS_NONE, mDocument->GetReferrerPolicy(), integrity, mRunsToCompletion ? nullptr : this, - &isAlternate); + &isAlternate, &isExplicitlyEnabled); NS_ENSURE_SUCCESS(rv, rv); - if (!isAlternate && !mRunsToCompletion) { + if ((!isAlternate || isExplicitlyEnabled) && !mRunsToCompletion) { ++mPendingSheetCount; mScriptLoader->AddParserBlockingScriptExecutionBlocker(); } diff --git a/dom/base/nsStyleLinkElement.cpp b/dom/base/nsStyleLinkElement.cpp index 8ab2dab0b..2e5cdac6f 100644 --- a/dom/base/nsStyleLinkElement.cpp +++ b/dom/base/nsStyleLinkElement.cpp @@ -393,8 +393,9 @@ nsStyleLinkElement::DoUpdateStyleSheet(nsIDocument* aOldDocument, nsAutoString title, type, media; bool isScoped; bool isAlternate; + bool isExplicitlyEnabled; - GetStyleSheetInfo(title, type, media, &isScoped, &isAlternate); + GetStyleSheetInfo(title, type, media, &isScoped, &isAlternate, &isExplicitlyEnabled); if (!type.LowerCaseEqualsLiteral("text/css")) { return NS_OK; @@ -425,7 +426,7 @@ nsStyleLinkElement::DoUpdateStyleSheet(nsIDocument* aOldDocument, // Parse the style sheet. rv = doc->CSSLoader()-> LoadInlineStyle(thisContent, text, mLineNumber, title, media, - scopeElement, aObserver, &doneLoading, &isAlternate); + scopeElement, aObserver, &doneLoading, &isAlternate, &isExplicitlyEnabled); } else { nsAutoString integrity; @@ -452,13 +453,14 @@ nsStyleLinkElement::DoUpdateStyleSheet(nsIDocument* aOldDocument, rv = doc->CSSLoader()-> LoadStyleLink(thisContent, clonedURI, title, media, isAlternate, GetCORSMode(), referrerPolicy, integrity, - aObserver, &isAlternate); + aObserver, &isAlternate, &isExplicitlyEnabled); if (NS_FAILED(rv)) { // Don't propagate LoadStyleLink() errors further than this, since some // consumers (e.g. nsXMLContentSink) will completely abort on innocuous // things like a stylesheet load being blocked by the security system. doneLoading = true; isAlternate = false; + isExplicitlyEnabled = false; rv = NS_OK; } } diff --git a/dom/base/nsStyleLinkElement.h b/dom/base/nsStyleLinkElement.h index a41ae5e1d..d9042c756 100644 --- a/dom/base/nsStyleLinkElement.h +++ b/dom/base/nsStyleLinkElement.h @@ -98,7 +98,8 @@ protected: nsAString& aType, nsAString& aMedia, bool* aIsScoped, - bool* aIsAlternate) = 0; + bool* aIsAlternate, + bool* aIsExplicitlyEnabled) = 0; virtual mozilla::CORSMode GetCORSMode() const { diff --git a/dom/html/HTMLLinkElement.cpp b/dom/html/HTMLLinkElement.cpp index 8afe767bd..1b78cbd69 100644 --- a/dom/html/HTMLLinkElement.cpp +++ b/dom/html/HTMLLinkElement.cpp @@ -33,6 +33,8 @@ #define LINK_ELEMENT_FLAG_BIT(n_) \ NODE_FLAG_BIT(ELEMENT_TYPE_SPECIFIC_BITS_OFFSET + (n_)) +#define LINK_DISABLED Preferences::GetBool("dom.link.disabled_attribute.enabled", true) + // Link element specific bits enum { // Indicates that a DNS Prefetch has been requested from this Link element. @@ -92,9 +94,14 @@ NS_INTERFACE_TABLE_TAIL_INHERITING(nsGenericHTMLElement) NS_IMPL_ELEMENT_CLONE(HTMLLinkElement) + bool -HTMLLinkElement::Disabled() +HTMLLinkElement::Disabled() const { + if (LINK_DISABLED) { + return GetBoolAttr(nsGkAtoms::disabled); + } + StyleSheet* ss = GetSheet(); return ss && ss->Disabled(); } @@ -107,8 +114,12 @@ HTMLLinkElement::GetMozDisabled(bool* aDisabled) } void -HTMLLinkElement::SetDisabled(bool aDisabled) -{ +HTMLLinkElement::SetDisabled(bool aDisabled, ErrorResult& aRv) +{ + if (LINK_DISABLED) { + return SetHTMLBoolAttr(nsGkAtoms::disabled, aDisabled, aRv); + } + if (StyleSheet* ss = GetSheet()) { ss->SetDisabled(aDisabled); } @@ -117,11 +128,11 @@ HTMLLinkElement::SetDisabled(bool aDisabled) NS_IMETHODIMP HTMLLinkElement::SetMozDisabled(bool aDisabled) { - SetDisabled(aDisabled); - return NS_OK; + ErrorResult rv; + SetDisabled(aDisabled, rv); + return rv.StealNSResult(); } - NS_IMPL_STRING_ATTR(HTMLLinkElement, Charset, charset) NS_IMPL_URI_ATTR(HTMLLinkElement, Href, href) NS_IMPL_STRING_ATTR(HTMLLinkElement, Hreflang, hreflang) @@ -369,7 +380,8 @@ HTMLLinkElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, aName == nsGkAtoms::rel || aName == nsGkAtoms::title || aName == nsGkAtoms::media || - aName == nsGkAtoms::type)) { + aName == nsGkAtoms::type || + (LINK_DISABLED && aName == nsGkAtoms::disabled))) { bool dropSheet = false; if (aName == nsGkAtoms::rel) { nsAutoString value; @@ -396,17 +408,24 @@ HTMLLinkElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, dropSheet || (aName == nsGkAtoms::title || aName == nsGkAtoms::media || - aName == nsGkAtoms::type)); + aName == nsGkAtoms::type || + (LINK_DISABLED && aName == nsGkAtoms::disabled))); } } else { - // Since removing href or rel makes us no longer link to a - // stylesheet, force updates for those too. + // If the disabled attribute is removed from a link element, the + // stylesheet may be explicitly enabled. if (aNameSpaceID == kNameSpaceID_None) { + if (aName == nsGkAtoms::disabled && LINK_DISABLED) { + mExplicitlyEnabled = true; + } + // Since removing href or rel makes us no longer link to a + // stylesheet, force updates for those too. if (aName == nsGkAtoms::href || aName == nsGkAtoms::rel || aName == nsGkAtoms::title || aName == nsGkAtoms::media || - aName == nsGkAtoms::type) { + aName == nsGkAtoms::type || + (LINK_DISABLED && aName == nsGkAtoms::disabled)) { UpdateStyleSheetInternal(nullptr, nullptr, true); } if (aName == nsGkAtoms::href || @@ -499,13 +518,15 @@ HTMLLinkElement::GetStyleSheetInfo(nsAString& aTitle, nsAString& aType, nsAString& aMedia, bool* aIsScoped, - bool* aIsAlternate) + bool* aIsAlternate, + bool* aIsExplicitlyEnabled) { aTitle.Truncate(); aType.Truncate(); aMedia.Truncate(); *aIsScoped = false; *aIsAlternate = false; + *aIsExplicitlyEnabled = false; nsAutoString rel; GetAttr(kNameSpaceID_None, nsGkAtoms::rel, rel); @@ -515,6 +536,20 @@ HTMLLinkElement::GetStyleSheetInfo(nsAString& aTitle, return; } + if (LINK_DISABLED) { + + // Is the link disabled? + if (Disabled()) { + return; + } + + // Is it explicitly enabled? + if (mExplicitlyEnabled) { + *aIsExplicitlyEnabled = true; + } + + } + nsAutoString title; GetAttr(kNameSpaceID_None, nsGkAtoms::title, title); title.CompressWhitespace(); diff --git a/dom/html/HTMLLinkElement.h b/dom/html/HTMLLinkElement.h index 421b149e9..f9c832c8d 100644 --- a/dom/html/HTMLLinkElement.h +++ b/dom/html/HTMLLinkElement.h @@ -84,8 +84,8 @@ public: virtual bool HasDeferredDNSPrefetchRequest() override; // WebIDL - bool Disabled(); - void SetDisabled(bool aDisabled); + bool Disabled() const; + void SetDisabled(bool aDisabled, ErrorResult& aRv); // XPCOM GetHref is fine. void SetHref(const nsAString& aHref, ErrorResult& aRv) { @@ -179,10 +179,18 @@ protected: nsAString& aType, nsAString& aMedia, bool* aIsScoped, - bool* aIsAlternate) override; -protected: + bool* aIsAlternate, + bool* aIsExplicitlyEnabled) override; + RefPtr mRelList; + // The "explicitly enabled" flag. This flag is set whenever the 'disabled' + // attribute is explicitly unset, and makes alternate stylesheets not be + // disabled by default anymore. + // + // See https://github.com/whatwg/html/issues/3840#issuecomment-481034206. + bool mExplicitlyEnabled = false; + private: RefPtr mImportLoader; }; diff --git a/dom/html/HTMLStyleElement.cpp b/dom/html/HTMLStyleElement.cpp index 329dda648..87dc68f83 100644 --- a/dom/html/HTMLStyleElement.cpp +++ b/dom/html/HTMLStyleElement.cpp @@ -66,7 +66,7 @@ HTMLStyleElement::GetMozDisabled(bool* aDisabled) } bool -HTMLStyleElement::Disabled() +HTMLStyleElement::Disabled() const { StyleSheet* ss = GetSheet(); return ss && ss->Disabled(); @@ -222,12 +222,14 @@ HTMLStyleElement::GetStyleSheetInfo(nsAString& aTitle, nsAString& aType, nsAString& aMedia, bool* aIsScoped, - bool* aIsAlternate) + bool* aIsAlternate, + bool* aIsExplicitlyEnabled) { aTitle.Truncate(); aType.Truncate(); aMedia.Truncate(); *aIsAlternate = false; + *aIsExplicitlyEnabled = false; nsAutoString title; GetAttr(kNameSpaceID_None, nsGkAtoms::title, title); diff --git a/dom/html/HTMLStyleElement.h b/dom/html/HTMLStyleElement.h index 6b2a12b1f..9f82b8e51 100644 --- a/dom/html/HTMLStyleElement.h +++ b/dom/html/HTMLStyleElement.h @@ -58,7 +58,7 @@ public: NS_DECL_NSIMUTATIONOBSERVER_CONTENTINSERTED NS_DECL_NSIMUTATIONOBSERVER_CONTENTREMOVED - bool Disabled(); + bool Disabled() const; void SetDisabled(bool aDisabled); void SetMedia(const nsAString& aMedia, ErrorResult& aError) { @@ -87,7 +87,8 @@ protected: nsAString& aType, nsAString& aMedia, bool* aIsScoped, - bool* aIsAlternate) override; + bool* aIsAlternate, + bool* aIsExplicitlyEnabled) override; /** * Common method to call from the various mutation observer methods. * aContent is a content node that's either the one that changed or its diff --git a/dom/svg/SVGStyleElement.cpp b/dom/svg/SVGStyleElement.cpp index 22fb204df..7655c1198 100644 --- a/dom/svg/SVGStyleElement.cpp +++ b/dom/svg/SVGStyleElement.cpp @@ -271,9 +271,11 @@ SVGStyleElement::GetStyleSheetInfo(nsAString& aTitle, nsAString& aType, nsAString& aMedia, bool* aIsScoped, - bool* aIsAlternate) + bool* aIsAlternate, + bool* aIsExplicitlyEnabled) { *aIsAlternate = false; + *aIsExplicitlyEnabled = false; nsAutoString title; GetAttr(kNameSpaceID_None, nsGkAtoms::title, title); diff --git a/dom/svg/SVGStyleElement.h b/dom/svg/SVGStyleElement.h index 9b126e7e8..e637dfb18 100644 --- a/dom/svg/SVGStyleElement.h +++ b/dom/svg/SVGStyleElement.h @@ -95,7 +95,8 @@ protected: nsAString& aType, nsAString& aMedia, bool* aIsScoped, - bool* aIsAlternate) override; + bool* aIsAlternate, + bool* aIsExplicitlyEnabled) override; virtual CORSMode GetCORSMode() const override; /** diff --git a/dom/webidl/HTMLLinkElement.webidl b/dom/webidl/HTMLLinkElement.webidl index ec094e55e..0b77e9ea7 100644 --- a/dom/webidl/HTMLLinkElement.webidl +++ b/dom/webidl/HTMLLinkElement.webidl @@ -13,7 +13,7 @@ // http://www.whatwg.org/specs/web-apps/current-work/#the-link-element interface HTMLLinkElement : HTMLElement { - [Pure] + [CEReactions, SetterThrows, Pure] attribute boolean disabled; [SetterThrows, Pure] attribute DOMString href; diff --git a/dom/xml/XMLStylesheetProcessingInstruction.cpp b/dom/xml/XMLStylesheetProcessingInstruction.cpp index 3d94e179b..43e45131a 100644 --- a/dom/xml/XMLStylesheetProcessingInstruction.cpp +++ b/dom/xml/XMLStylesheetProcessingInstruction.cpp @@ -131,13 +131,15 @@ XMLStylesheetProcessingInstruction::GetStyleSheetInfo(nsAString& aTitle, nsAString& aType, nsAString& aMedia, bool* aIsScoped, - bool* aIsAlternate) + bool* aIsAlternate, + bool* aIsExplicitlyEnabled) { aTitle.Truncate(); aType.Truncate(); aMedia.Truncate(); *aIsScoped = false; *aIsAlternate = false; + *aIsExplicitlyEnabled = false; // xml-stylesheet PI is special only in prolog if (!nsContentUtils::InProlog(this)) { diff --git a/dom/xml/XMLStylesheetProcessingInstruction.h b/dom/xml/XMLStylesheetProcessingInstruction.h index 818b3392f..28061834a 100644 --- a/dom/xml/XMLStylesheetProcessingInstruction.h +++ b/dom/xml/XMLStylesheetProcessingInstruction.h @@ -82,7 +82,8 @@ protected: nsAString& aType, nsAString& aMedia, bool* aIsScoped, - bool* aIsAlternate) override; + bool* aIsAlternate, + bool* aIsExplicitlyEnabled) override; virtual nsGenericDOMDataNode* CloneDataNode(mozilla::dom::NodeInfo *aNodeInfo, bool aCloneText) const override; }; -- cgit v1.2.3 From 06a092d08ef5c20306d5c0b9a5f0bfd14ae28cad Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Wed, 19 Aug 2020 17:44:09 -0400 Subject: Follow up to eb28b1f32 - Correct inputmethod build file by putting Keyboard.jsm back in EXTRA_JS_MODULES JSMs are NOT Components. --- dom/inputmethod/moz.build | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'dom') diff --git a/dom/inputmethod/moz.build b/dom/inputmethod/moz.build index e6d994565..d0e817ee2 100644 --- a/dom/inputmethod/moz.build +++ b/dom/inputmethod/moz.build @@ -6,10 +6,11 @@ EXTRA_COMPONENTS += [ 'InputMethod.manifest', - 'Keyboard.jsm', 'MozKeyboard.js', ] +EXTRA_JS_MODULES += ['Keyboard.jsm'] + JAR_MANIFESTS += ['jar.mn'] MOCHITEST_CHROME_MANIFESTS += ['mochitest/chrome.ini'] -- cgit v1.2.3 From 2f145b6eda95c08a76711f0393e4d9ebe13a5f92 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Wed, 8 Jul 2020 10:50:53 +0000 Subject: Issue #618 - Fix error events fired when loading JS module dependencies fail When module dependencies fail, don't spam with errors for each import; only fire the error event once. Ref: BZ 1421259 --- dom/script/ScriptElement.cpp | 4 ++-- dom/script/ScriptLoader.cpp | 3 ++- dom/script/ScriptLoader.h | 3 ++- dom/script/nsIScriptLoaderObserver.idl | 9 +++++---- dom/xslt/xslt/txMozillaXMLOutput.cpp | 2 +- 5 files changed, 12 insertions(+), 9 deletions(-) (limited to 'dom') diff --git a/dom/script/ScriptElement.cpp b/dom/script/ScriptElement.cpp index 0cb17dcb0..9cb239c66 100644 --- a/dom/script/ScriptElement.cpp +++ b/dom/script/ScriptElement.cpp @@ -21,11 +21,11 @@ using namespace mozilla::dom; NS_IMETHODIMP ScriptElement::ScriptAvailable(nsresult aResult, nsIScriptElement *aElement, - bool aIsInline, + bool aIsInlineClassicScript, nsIURI *aURI, int32_t aLineNo) { - if (!aIsInline && NS_FAILED(aResult)) { + if (!aIsInlineClassicScript && NS_FAILED(aResult)) { nsCOMPtr parser = do_QueryReferent(mCreatorParser); if (parser) { parser->PushDefinedInsertionPoint(); diff --git a/dom/script/ScriptLoader.cpp b/dom/script/ScriptLoader.cpp index a53098974..1426c30c9 100644 --- a/dom/script/ScriptLoader.cpp +++ b/dom/script/ScriptLoader.cpp @@ -2327,7 +2327,8 @@ ScriptLoader::OnStreamComplete(nsIIncrementalStreamLoader* aLoader, MOZ_ASSERT(!modReq->IsTopLevel()); MOZ_ASSERT(!modReq->isInList()); modReq->Cancel(); - FireScriptAvailable(rv, request); + // A single error is fired for the top level module, so don't use + // FireScriptAvailable here. } else if (mParserBlockingRequest == request) { MOZ_ASSERT(!request->isInList()); mParserBlockingRequest = nullptr; diff --git a/dom/script/ScriptLoader.h b/dom/script/ScriptLoader.h index e6b75bf3b..b07dd4ec6 100644 --- a/dom/script/ScriptLoader.h +++ b/dom/script/ScriptLoader.h @@ -100,7 +100,8 @@ public: void FireScriptAvailable(nsresult aResult) { - mElement->ScriptAvailable(aResult, mElement, mIsInline, mURI, mLineNo); + bool isInlineClassicScript = mIsInline && !IsModuleRequest(); + mElement->ScriptAvailable(aResult, mElement, isInlineClassicScript, mURI, mLineNo); } void FireScriptEvaluated(nsresult aResult) { diff --git a/dom/script/nsIScriptLoaderObserver.idl b/dom/script/nsIScriptLoaderObserver.idl index ed7196525..880738360 100644 --- a/dom/script/nsIScriptLoaderObserver.idl +++ b/dom/script/nsIScriptLoaderObserver.idl @@ -20,15 +20,16 @@ interface nsIScriptLoaderObserver : nsISupports { * a script. If this is a failure code, script evaluation * will not occur. * @param aElement The element being processed. - * @param aIsInline Is this an inline script or externally loaded? + * @param aIsInline Is this an inline classic script (as opposed to an + * externally loaded classic script or module script)? * @param aURI What is the URI of the script (the document URI if * it is inline). * @param aLineNo At what line does the script appear (generally 1 * if it is a loaded script). */ - void scriptAvailable(in nsresult aResult, + void scriptAvailable(in nsresult aResult, in nsIScriptElement aElement, - in boolean aIsInline, + in boolean aIsInlineClassicScript, in nsIURI aURI, in int32_t aLineNo); @@ -40,7 +41,7 @@ interface nsIScriptLoaderObserver : nsISupports { * @param aElement The element being processed. * @param aIsInline Is this an inline script or externally loaded? */ - void scriptEvaluated(in nsresult aResult, + void scriptEvaluated(in nsresult aResult, in nsIScriptElement aElement, in boolean aIsInline); diff --git a/dom/xslt/xslt/txMozillaXMLOutput.cpp b/dom/xslt/xslt/txMozillaXMLOutput.cpp index 704d8ac11..21b3c228f 100644 --- a/dom/xslt/xslt/txMozillaXMLOutput.cpp +++ b/dom/xslt/xslt/txMozillaXMLOutput.cpp @@ -955,7 +955,7 @@ NS_IMPL_ISUPPORTS(txTransformNotifier, NS_IMETHODIMP txTransformNotifier::ScriptAvailable(nsresult aResult, nsIScriptElement *aElement, - bool aIsInline, + bool aIsInlineClassicScript, nsIURI *aURI, int32_t aLineNo) { -- cgit v1.2.3 From 498b1ab0c8db07784badbd2148f372027ef8cc27 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Wed, 8 Jul 2020 12:58:30 +0000 Subject: Issue #618 - Align error handling for module scripts with the spec (again) This updates module implementation to match spec regarding handling of instantiation errors, after it was changed yet again, this time to not remember instantiation errors, but instead immediately rethrow applicable ones. Ref: BZ 1420420 --- dom/script/ModuleLoadRequest.cpp | 4 +-- dom/script/ModuleScript.cpp | 43 +++++++++------------- dom/script/ModuleScript.h | 12 ++++--- dom/script/ScriptLoader.cpp | 77 ++++++++++++++++++++++++++-------------- dom/script/ScriptLoader.h | 1 + 5 files changed, 79 insertions(+), 58 deletions(-) (limited to 'dom') diff --git a/dom/script/ModuleLoadRequest.cpp b/dom/script/ModuleLoadRequest.cpp index d62214304..a12fcb162 100644 --- a/dom/script/ModuleLoadRequest.cpp +++ b/dom/script/ModuleLoadRequest.cpp @@ -83,7 +83,7 @@ ModuleLoadRequest::ModuleLoaded() // been loaded. mModuleScript = mLoader->GetFetchedModule(mURI); - if (!mModuleScript || mModuleScript->IsErrored()) { + if (!mModuleScript || mModuleScript->HasParseError()) { ModuleErrored(); return; } @@ -95,7 +95,7 @@ void ModuleLoadRequest::ModuleErrored() { mLoader->CheckModuleDependenciesLoaded(this); - MOZ_ASSERT(!mModuleScript || mModuleScript->IsErrored()); + MOZ_ASSERT(!mModuleScript || mModuleScript->HasParseError()); CancelImports(); SetReady(); diff --git a/dom/script/ModuleScript.cpp b/dom/script/ModuleScript.cpp index 28b97a3cb..1bf9d0b0f 100644 --- a/dom/script/ModuleScript.cpp +++ b/dom/script/ModuleScript.cpp @@ -26,7 +26,8 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(ModuleScript) NS_IMPL_CYCLE_COLLECTION_UNLINK(mLoader) NS_IMPL_CYCLE_COLLECTION_UNLINK(mBaseURL) tmp->UnlinkModuleRecord(); - tmp->mError.setUndefined(); + tmp->mParseError.setUndefined(); + tmp->mErrorToRethrow.setUndefined(); NS_IMPL_CYCLE_COLLECTION_UNLINK_END NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(ModuleScript) @@ -35,7 +36,8 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(ModuleScript) NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mModuleRecord) - NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mError) + NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mParseError) + NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mErrorToRethrow) NS_IMPL_CYCLE_COLLECTION_TRACE_END NS_IMPL_CYCLE_COLLECTING_ADDREF(ModuleScript) @@ -48,7 +50,8 @@ ModuleScript::ModuleScript(ScriptLoader *aLoader, nsIURI* aBaseURL) MOZ_ASSERT(mLoader); MOZ_ASSERT(mBaseURL); MOZ_ASSERT(!mModuleRecord); - MOZ_ASSERT(mError.isUndefined()); + MOZ_ASSERT(!HasParseError()); + MOZ_ASSERT(!HasErrorToRethrow()); } void @@ -74,7 +77,8 @@ void ModuleScript::SetModuleRecord(JS::Handle aModuleRecord) { MOZ_ASSERT(!mModuleRecord); - MOZ_ASSERT(mError.isUndefined()); + MOZ_ASSERT(!HasParseError()); + MOZ_ASSERT(!HasErrorToRethrow()); mModuleRecord = aModuleRecord; @@ -85,37 +89,24 @@ ModuleScript::SetModuleRecord(JS::Handle aModuleRecord) } void -ModuleScript::SetPreInstantiationError(const JS::Value& aError) +ModuleScript::SetParseError(const JS::Value& aError) { MOZ_ASSERT(!aError.isUndefined()); + MOZ_ASSERT(!HasParseError()); + MOZ_ASSERT(!HasErrorToRethrow()); UnlinkModuleRecord(); - mError = aError; - + mParseError = aError; HoldJSObjects(this); } -bool -ModuleScript::IsErrored() const -{ - if (!mModuleRecord) { - MOZ_ASSERT(!mError.isUndefined()); - return true; - } - - return JS::IsModuleErrored(mModuleRecord); -} - -JS::Value -ModuleScript::Error() const +void +ModuleScript::SetErrorToRethrow(const JS::Value& aError) { - MOZ_ASSERT(IsErrored()); - - if (!mModuleRecord) { - return mError; - } + MOZ_ASSERT(!aError.isUndefined()); + MOZ_ASSERT(!HasErrorToRethrow()); - return JS::GetModuleError(mModuleRecord); + mErrorToRethrow = aError; } } // dom namespace diff --git a/dom/script/ModuleScript.h b/dom/script/ModuleScript.h index 571359859..f765aa0fa 100644 --- a/dom/script/ModuleScript.h +++ b/dom/script/ModuleScript.h @@ -23,7 +23,8 @@ class ModuleScript final : public nsISupports RefPtr mLoader; nsCOMPtr mBaseURL; JS::Heap mModuleRecord; - JS::Heap mError; + JS::Heap mParseError; + JS::Heap mErrorToRethrow; ~ModuleScript(); @@ -35,14 +36,17 @@ public: nsIURI* aBaseURL); void SetModuleRecord(JS::Handle aModuleRecord); - void SetPreInstantiationError(const JS::Value& aError); + void SetParseError(const JS::Value& aError); + void SetErrorToRethrow(const JS::Value& aError); ScriptLoader* Loader() const { return mLoader; } JSObject* ModuleRecord() const { return mModuleRecord; } nsIURI* BaseURL() const { return mBaseURL; } - bool IsErrored() const; - JS::Value Error() const; + JS::Value ParseError() const { return mParseError; } + JS::Value ErrorToRethrow() const { return mErrorToRethrow; } + bool HasParseError() const { return !mParseError.isUndefined(); } + bool HasErrorToRethrow() const { return !mErrorToRethrow.isUndefined(); } void UnlinkModuleRecord(); }; diff --git a/dom/script/ScriptLoader.cpp b/dom/script/ScriptLoader.cpp index 1426c30c9..3dbbcacea 100644 --- a/dom/script/ScriptLoader.cpp +++ b/dom/script/ScriptLoader.cpp @@ -493,7 +493,7 @@ ScriptLoader::ProcessFetchedModuleSource(ModuleLoadRequest* aRequest) return rv; } - if (!aRequest->mModuleScript->IsErrored()) { + if (!aRequest->mModuleScript->HasParseError()) { StartFetchingModuleDependencies(aRequest); } @@ -568,7 +568,7 @@ ScriptLoader::CreateModuleScript(ModuleLoadRequest* aRequest) return NS_ERROR_FAILURE; } - moduleScript->SetPreInstantiationError(error); + moduleScript->SetParseError(error); aRequest->ModuleErrored(); return NS_OK; } @@ -580,8 +580,6 @@ ScriptLoader::CreateModuleScript(ModuleLoadRequest* aRequest) nsCOMArray urls; rv = ResolveRequestedModules(aRequest, urls); if (NS_FAILED(rv)) { - // ResolveRequestedModules sets pre-instanitation error on failure. - MOZ_ASSERT(moduleScript->IsErrored()); aRequest->ModuleErrored(); return NS_OK; } @@ -627,7 +625,7 @@ HandleResolveFailure(JSContext* aCx, ModuleScript* aScript, return NS_ERROR_OUT_OF_MEMORY; } - aScript->SetPreInstantiationError(error); + aScript->SetParseError(error); return NS_OK; } @@ -723,7 +721,6 @@ ResolveRequestedModules(ModuleLoadRequest* aRequest, nsCOMArray &aUrls) } // Let url be the result of resolving a module specifier given module script and requested. - ModuleScript* ms = aRequest->mModuleScript; nsCOMPtr uri = ResolveModuleSpecifier(ms, specifier); if (!uri) { nsresult rv = HandleResolveFailure(cx, ms, specifier); @@ -746,7 +743,7 @@ void ScriptLoader::StartFetchingModuleDependencies(ModuleLoadRequest* aRequest) { MOZ_ASSERT(aRequest->mModuleScript); - MOZ_ASSERT(!aRequest->mModuleScript->IsErrored()); + MOZ_ASSERT(!aRequest->mModuleScript->HasParseError()); MOZ_ASSERT(!aRequest->IsReadyToRun()); aRequest->mProgress = ModuleLoadRequest::Progress::FetchingImports; @@ -843,7 +840,7 @@ HostResolveImportedModule(JSContext* aCx, unsigned argc, JS::Value* vp) ModuleScript* ms = script->Loader()->GetFetchedModule(uri); MOZ_ASSERT(ms, "Resolved module not found in module map"); - MOZ_ASSERT(!ms->IsErrored()); + MOZ_ASSERT(!ms->HasParseError()); *vp = JS::ObjectValue(*ms->ModuleRecord()); return true; @@ -871,18 +868,16 @@ void ScriptLoader::CheckModuleDependenciesLoaded(ModuleLoadRequest* aRequest) { RefPtr moduleScript = aRequest->mModuleScript; - if (moduleScript && !moduleScript->IsErrored()) { - for (auto childRequest : aRequest->mImports) { - ModuleScript* childScript = childRequest->mModuleScript; - if (!childScript) { - // Load error - aRequest->mModuleScript = nullptr; - return; - } else if (childScript->IsErrored()) { - // Script error - moduleScript->SetPreInstantiationError(childScript->Error()); - return; - } + if (!moduleScript || moduleScript->HasParseError()) { + return; + } + + for (auto childRequest : aRequest->mImports) { + ModuleScript* childScript = childRequest->mModuleScript; + if (!childScript) { + aRequest->mModuleScript = nullptr; + // Load error on script load request; bail. + return; } } } @@ -892,7 +887,7 @@ ScriptLoader::ProcessLoadedModuleTree(ModuleLoadRequest* aRequest) { if (aRequest->IsTopLevel()) { ModuleScript* moduleScript = aRequest->mModuleScript; - if (moduleScript && !moduleScript->IsErrored()) { + if (moduleScript && !moduleScript->HasErrorToRethrow()) { if (!InstantiateModuleTree(aRequest)) { aRequest->mModuleScript = nullptr; } @@ -906,6 +901,28 @@ ScriptLoader::ProcessLoadedModuleTree(ModuleLoadRequest* aRequest) } } +JS::Value +ScriptLoader::FindFirstParseError(ModuleLoadRequest* aRequest) +{ + MOZ_ASSERT(aRequest); + + ModuleScript* moduleScript = aRequest->mModuleScript; + MOZ_ASSERT(moduleScript); + + if (moduleScript->HasParseError()) { + return moduleScript->ParseError(); + } + + for (ModuleLoadRequest* childRequest : aRequest->mImports) { + JS::Value error = FindFirstParseError(childRequest); + if (!error.isUndefined()) { + return error; + } + } + + return JS::UndefinedValue(); +} + bool ScriptLoader::InstantiateModuleTree(ModuleLoadRequest* aRequest) { @@ -916,6 +933,14 @@ ScriptLoader::InstantiateModuleTree(ModuleLoadRequest* aRequest) ModuleScript* moduleScript = aRequest->mModuleScript; MOZ_ASSERT(moduleScript); + + JS::Value parseError = FindFirstParseError(aRequest); + if (!parseError.isUndefined()) { + // Parse error found in the requested script + moduleScript->SetErrorToRethrow(parseError); + return true; + } + MOZ_ASSERT(moduleScript->ModuleRecord()); nsAutoMicroTask mt; @@ -937,7 +962,7 @@ ScriptLoader::InstantiateModuleTree(ModuleLoadRequest* aRequest) return false; } MOZ_ASSERT(!exception.isUndefined()); - // Ignore the exception. It will be recorded in the module record. + moduleScript->SetErrorToRethrow(exception); } return true; @@ -1462,7 +1487,7 @@ ScriptLoader::ProcessScriptElement(nsIScriptElement *aElement) } else { AddDeferRequest(request); } - if (!modReq->mModuleScript->IsErrored()) { + if (!modReq->mModuleScript->HasParseError()) { StartFetchingModuleDependencies(modReq); } return false; @@ -1966,9 +1991,9 @@ ScriptLoader::EvaluateScript(ScriptLoadRequest* aRequest) MOZ_ASSERT(!request->mOffThreadToken); ModuleScript* moduleScript = request->mModuleScript; - if (moduleScript->IsErrored()) { - // Module has an error status - JS::Rooted error(cx, moduleScript->Error()); + if (moduleScript->HasErrorToRethrow()) { + // Module has an error status to be rethrown + JS::Rooted error(cx, moduleScript->ErrorToRethrow()); JS_SetPendingException(cx, error); return NS_OK; // An error is reported by AutoEntryScript. } diff --git a/dom/script/ScriptLoader.h b/dom/script/ScriptLoader.h index b07dd4ec6..db2eeed31 100644 --- a/dom/script/ScriptLoader.h +++ b/dom/script/ScriptLoader.h @@ -583,6 +583,7 @@ private: nsresult ProcessFetchedModuleSource(ModuleLoadRequest* aRequest); void CheckModuleDependenciesLoaded(ModuleLoadRequest* aRequest); void ProcessLoadedModuleTree(ModuleLoadRequest* aRequest); + JS::Value FindFirstParseError(ModuleLoadRequest* aRequest); bool InstantiateModuleTree(ModuleLoadRequest* aRequest); void StartFetchingModuleDependencies(ModuleLoadRequest* aRequest); -- cgit v1.2.3 From d8686de401388c036ee02432cbcfa7b41272217d Mon Sep 17 00:00:00 2001 From: Jon Coppeard Date: Sun, 2 Aug 2020 07:20:25 +0000 Subject: Issue #618 - Record module dependency before starting fetch so that error handling works correctly Ref BZ 1395896 --- dom/script/ScriptLoader.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'dom') diff --git a/dom/script/ScriptLoader.cpp b/dom/script/ScriptLoader.cpp index 3dbbcacea..4050caada 100644 --- a/dom/script/ScriptLoader.cpp +++ b/dom/script/ScriptLoader.cpp @@ -793,6 +793,7 @@ ScriptLoader::StartFetchingModuleAndDependencies(ModuleLoadRequest* aRequest, childRequest->mIsInline = false; childRequest->mReferrerPolicy = aRequest->mReferrerPolicy; childRequest->mParent = aRequest; + aRequest->mImports.AppendElement(childRequest); RefPtr ready = childRequest->mReady.Ensure(__func__); @@ -803,7 +804,6 @@ ScriptLoader::StartFetchingModuleAndDependencies(ModuleLoadRequest* aRequest, return ready; } - aRequest->mImports.AppendElement(childRequest); return ready; } -- cgit v1.2.3 From fe912e94e44c4a6dfac1a75c265084f77adfd6ae Mon Sep 17 00:00:00 2001 From: Gaming4JC Date: Sun, 2 Aug 2020 12:46:52 -0400 Subject: Issue #618 - Simplify module map interface Ref: BZ 1365187 --- dom/script/ScriptLoader.cpp | 24 ++++++++++++------------ dom/script/ScriptLoader.h | 4 ++-- 2 files changed, 14 insertions(+), 14 deletions(-) (limited to 'dom') diff --git a/dom/script/ScriptLoader.cpp b/dom/script/ScriptLoader.cpp index 4050caada..7d5a19faf 100644 --- a/dom/script/ScriptLoader.cpp +++ b/dom/script/ScriptLoader.cpp @@ -388,12 +388,12 @@ ScriptLoader::ModuleScriptsEnabled() } bool -ScriptLoader::ModuleMapContainsModule(ModuleLoadRequest *aRequest) const +ScriptLoader::ModuleMapContainsURL(nsIURI* aURL) const { // Returns whether we have fetched, or are currently fetching, a module script - // for the request's URL. - return mFetchingModules.Contains(aRequest->mURI) || - mFetchedModules.Contains(aRequest->mURI); + // for a URL. + return mFetchingModules.Contains(aURL) || + mFetchedModules.Contains(aURL); } bool @@ -410,7 +410,7 @@ ScriptLoader::SetModuleFetchStarted(ModuleLoadRequest *aRequest) // Update the module map to indicate that a module is currently being fetched. MOZ_ASSERT(aRequest->IsLoading()); - MOZ_ASSERT(!ModuleMapContainsModule(aRequest)); + MOZ_ASSERT(!ModuleMapContainsURL(aRequest->mURI)); mFetchingModules.Put(aRequest->mURI, nullptr); } @@ -443,21 +443,21 @@ ScriptLoader::SetModuleFetchFinishedAndResumeWaitingRequests(ModuleLoadRequest * } RefPtr -ScriptLoader::WaitForModuleFetch(ModuleLoadRequest *aRequest) +ScriptLoader::WaitForModuleFetch(nsIURI* aURL) { - MOZ_ASSERT(ModuleMapContainsModule(aRequest)); + MOZ_ASSERT(ModuleMapContainsURL(aURL)); RefPtr promise; - if (mFetchingModules.Get(aRequest->mURI, getter_AddRefs(promise))) { + if (mFetchingModules.Get(aURL, getter_AddRefs(promise))) { if (!promise) { promise = new GenericPromise::Private(__func__); - mFetchingModules.Put(aRequest->mURI, promise); + mFetchingModules.Put(aURL, promise); } return promise; } RefPtr ms; - MOZ_ALWAYS_TRUE(mFetchedModules.Get(aRequest->mURI, getter_AddRefs(ms))); + MOZ_ALWAYS_TRUE(mFetchedModules.Get(aURL, getter_AddRefs(ms))); if (!ms) { return GenericPromise::CreateAndReject(NS_ERROR_FAILURE, __func__); } @@ -984,8 +984,8 @@ ScriptLoader::StartLoad(ScriptLoadRequest *aRequest, const nsAString &aType, // Check whether the module has been fetched or is currently being fetched, // and if so wait for it. ModuleLoadRequest* request = aRequest->AsModuleRequest(); - if (ModuleMapContainsModule(request)) { - WaitForModuleFetch(request) + if (ModuleMapContainsURL(request->mURI)) { + WaitForModuleFetch(request->mURI) ->Then(AbstractThread::GetCurrent(), __func__, request, &ModuleLoadRequest::ModuleLoaded, &ModuleLoadRequest::LoadFailed); diff --git a/dom/script/ScriptLoader.h b/dom/script/ScriptLoader.h index db2eeed31..955ac2cb7 100644 --- a/dom/script/ScriptLoader.h +++ b/dom/script/ScriptLoader.h @@ -572,8 +572,8 @@ private: bool IsFetchingModule(ModuleLoadRequest *aRequest) const; - bool ModuleMapContainsModule(ModuleLoadRequest *aRequest) const; - RefPtr WaitForModuleFetch(ModuleLoadRequest *aRequest); + bool ModuleMapContainsURL(nsIURI* aURL) const; + RefPtr WaitForModuleFetch(nsIURI* aURL); ModuleScript* GetFetchedModule(nsIURI* aURL) const; friend bool -- cgit v1.2.3 From 9982c1f5cb3dfe361b9e9d21e5582a5e35d4c7fb Mon Sep 17 00:00:00 2001 From: Gaming4JC Date: Mon, 3 Aug 2020 10:44:39 -0400 Subject: Issue #618 - Keep track of which modules in a graph have been fetched using a visited set Ref: BZ 1365187 --- dom/script/ModuleLoadRequest.cpp | 33 +++++++++++--- dom/script/ModuleLoadRequest.h | 31 +++++++++++--- dom/script/ScriptLoader.cpp | 93 ++++++++++++++++------------------------ dom/script/ScriptLoader.h | 7 ++- 4 files changed, 92 insertions(+), 72 deletions(-) (limited to 'dom') diff --git a/dom/script/ModuleLoadRequest.cpp b/dom/script/ModuleLoadRequest.cpp index a12fcb162..a75a922e2 100644 --- a/dom/script/ModuleLoadRequest.cpp +++ b/dom/script/ModuleLoadRequest.cpp @@ -17,26 +17,48 @@ NS_INTERFACE_MAP_END_INHERITING(ScriptLoadRequest) NS_IMPL_CYCLE_COLLECTION_INHERITED(ModuleLoadRequest, ScriptLoadRequest, mBaseURL, mLoader, - mParent, mModuleScript, mImports) NS_IMPL_ADDREF_INHERITED(ModuleLoadRequest, ScriptLoadRequest) NS_IMPL_RELEASE_INHERITED(ModuleLoadRequest, ScriptLoadRequest) -ModuleLoadRequest::ModuleLoadRequest(nsIScriptElement* aElement, +ModuleLoadRequest::ModuleLoadRequest(nsIURI* aURI, + nsIScriptElement* aElement, uint32_t aVersion, CORSMode aCORSMode, const SRIMetadata &aIntegrity, ScriptLoader* aLoader) : ScriptLoadRequest(ScriptKind::Module, + aURI, aElement, aVersion, aCORSMode, aIntegrity), mIsTopLevel(true), - mLoader(aLoader) -{} + mLoader(aLoader), + mVisitedSet(new VisitedURLSet()) +{ + mVisitedSet->PutEntry(aURI); +} + +ModuleLoadRequest::ModuleLoadRequest(nsIURI* aURI, + ModuleLoadRequest* aParent) + : ScriptLoadRequest(ScriptKind::Module, + aURI, + aParent->mElement, + aParent->mJSVersion, + aParent->mCORSMode, + aParent->mIntegrity), + mIsTopLevel(false), + mLoader(aParent->mLoader), + mVisitedSet(aParent->mVisitedSet) +{ + MOZ_ASSERT(mVisitedSet->Contains(aURI)); + + mIsInline = false; + mReferrerPolicy = aParent->mReferrerPolicy; +} void ModuleLoadRequest::Cancel() { @@ -132,8 +154,7 @@ ModuleLoadRequest::LoadFinished() { mLoader->ProcessLoadedModuleTree(this); mLoader = nullptr; - mParent = nullptr; } } // dom namespace -} // mozilla namespace \ No newline at end of file +} // mozilla namespace diff --git a/dom/script/ModuleLoadRequest.h b/dom/script/ModuleLoadRequest.h index 7b06dd2cf..2e9652881 100644 --- a/dom/script/ModuleLoadRequest.h +++ b/dom/script/ModuleLoadRequest.h @@ -8,6 +8,7 @@ #define mozilla_dom_ModuleLoadRequest_h #include "mozilla/dom/ScriptLoader.h" +#include "nsURIHashKey.h" #include "mozilla/MozPromise.h" namespace mozilla { @@ -16,6 +17,16 @@ namespace dom { class ModuleScript; class ScriptLoader; +// A reference counted set of URLs we have visited in the process of loading a +// module graph. +class VisitedURLSet : public nsTHashtable +{ + NS_INLINE_DECL_REFCOUNTING(VisitedURLSet) + +private: + ~VisitedURLSet() = default; +}; + // A load request for a module, created for every top level module script and // every module import. Load request can share a ModuleScript if there are // multiple imports of the same module. @@ -31,12 +42,18 @@ public: NS_DECL_ISUPPORTS_INHERITED NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(ModuleLoadRequest, ScriptLoadRequest) - ModuleLoadRequest(nsIScriptElement* aElement, + // Create a top-level module load request. + ModuleLoadRequest(nsIURI* aURI, + nsIScriptElement* aElement, uint32_t aVersion, CORSMode aCORSMode, const SRIMetadata& aIntegrity, ScriptLoader* aLoader); + // Create a module load request for an imported module. + ModuleLoadRequest(nsIURI* aURI, + ModuleLoadRequest* aParent); + bool IsTopLevel() const { return mIsTopLevel; } @@ -55,7 +72,7 @@ private: public: // Is this a request for a top level module script or an import? - bool mIsTopLevel; + const bool mIsTopLevel; // The base URL used for resolving relative module imports. nsCOMPtr mBaseURL; @@ -64,10 +81,6 @@ public: // finishes. RefPtr mLoader; - // The importing module, or nullptr for top level module scripts. Used to - // implement the ancestor list checked when fetching module dependencies. - RefPtr mParent; - // Set to a module script object after a successful load or nullptr on // failure. RefPtr mModuleScript; @@ -79,9 +92,13 @@ public: // Array of imported modules. nsTArray> mImports; + + // Set of module URLs visited while fetching the module graph this request is + // part of. + RefPtr mVisitedSet; }; } // dom namespace } // mozilla namespace -#endif // mozilla_dom_ModuleLoadRequest_h \ No newline at end of file +#endif // mozilla_dom_ModuleLoadRequest_h diff --git a/dom/script/ScriptLoader.cpp b/dom/script/ScriptLoader.cpp index 7d5a19faf..7a3cd8fe3 100644 --- a/dom/script/ScriptLoader.cpp +++ b/dom/script/ScriptLoader.cpp @@ -501,7 +501,7 @@ ScriptLoader::ProcessFetchedModuleSource(ModuleLoadRequest* aRequest) } static nsresult -ResolveRequestedModules(ModuleLoadRequest* aRequest, nsCOMArray& aUrls); +ResolveRequestedModules(ModuleLoadRequest* aRequest, nsCOMArray* aUrlsOut); nsresult ScriptLoader::CreateModuleScript(ModuleLoadRequest* aRequest) @@ -577,8 +577,7 @@ ScriptLoader::CreateModuleScript(ModuleLoadRequest* aRequest) // Validate requested modules and treat failure to resolve module specifiers // the same as a parse error. - nsCOMArray urls; - rv = ResolveRequestedModules(aRequest, urls); + rv = ResolveRequestedModules(aRequest, nullptr); if (NS_FAILED(rv)) { aRequest->ModuleErrored(); return NS_OK; @@ -665,33 +664,7 @@ ResolveModuleSpecifier(ModuleScript* aScript, } static nsresult -RequestedModuleIsInAncestorList(ModuleLoadRequest* aRequest, nsIURI* aURL, bool* aResult) -{ - const size_t ImportDepthLimit = 100; - - *aResult = false; - size_t depth = 0; - while (aRequest) { - if (depth++ == ImportDepthLimit) { - return NS_ERROR_FAILURE; - } - - bool equal; - nsresult rv = aURL->Equals(aRequest->mURI, &equal); - NS_ENSURE_SUCCESS(rv, rv); - if (equal) { - *aResult = true; - return NS_OK; - } - - aRequest = aRequest->mParent; - } - - return NS_OK; -} - -static nsresult -ResolveRequestedModules(ModuleLoadRequest* aRequest, nsCOMArray &aUrls) +ResolveRequestedModules(ModuleLoadRequest* aRequest, nsCOMArray* aUrlsOut) { ModuleScript* ms = aRequest->mModuleScript; @@ -728,11 +701,8 @@ ResolveRequestedModules(ModuleLoadRequest* aRequest, nsCOMArray &aUrls) return NS_ERROR_FAILURE; } - bool isAncestor; - nsresult rv = RequestedModuleIsInAncestorList(aRequest, uri, &isAncestor); - NS_ENSURE_SUCCESS(rv, rv); - if (!isAncestor) { - aUrls.AppendElement(uri.forget()); + if (aUrlsOut) { + aUrlsOut->AppendElement(uri.forget()); } } @@ -746,17 +716,33 @@ ScriptLoader::StartFetchingModuleDependencies(ModuleLoadRequest* aRequest) MOZ_ASSERT(!aRequest->mModuleScript->HasParseError()); MOZ_ASSERT(!aRequest->IsReadyToRun()); + auto visitedSet = aRequest->mVisitedSet; + MOZ_ASSERT(visitedSet->Contains(aRequest->mURI)); + aRequest->mProgress = ModuleLoadRequest::Progress::FetchingImports; nsCOMArray urls; - nsresult rv = ResolveRequestedModules(aRequest, urls); + nsresult rv = ResolveRequestedModules(aRequest, &urls); if (NS_FAILED(rv)) { aRequest->ModuleErrored(); return; } - if (urls.Length() == 0) { - // There are no descendents to load so this request is ready. + // Remove already visited URLs from the list. Put unvisited URLs into the + // visited set. + int32_t i = 0; + while (i < urls.Count()) { + nsIURI* url = urls[i]; + if (visitedSet->Contains(url)) { + urls.RemoveObjectAt(i); + } else { + visitedSet->PutEntry(url); + i++; + } + } + + if (urls.Count() == 0) { + // There are no descendants to load so this request is ready. aRequest->DependenciesLoaded(); return; } @@ -779,21 +765,14 @@ ScriptLoader::StartFetchingModuleDependencies(ModuleLoadRequest* aRequest) } RefPtr -ScriptLoader::StartFetchingModuleAndDependencies(ModuleLoadRequest* aRequest, +ScriptLoader::StartFetchingModuleAndDependencies(ModuleLoadRequest* aParent, nsIURI* aURI) { MOZ_ASSERT(aURI); - RefPtr childRequest = - new ModuleLoadRequest(aRequest->mElement, aRequest->mJSVersion, - aRequest->mCORSMode, aRequest->mIntegrity, this); + RefPtr childRequest = new ModuleLoadRequest(aURI, aParent); - childRequest->mIsTopLevel = false; - childRequest->mURI = aURI; - childRequest->mIsInline = false; - childRequest->mReferrerPolicy = aRequest->mReferrerPolicy; - childRequest->mParent = aRequest; - aRequest->mImports.AppendElement(childRequest); + aParent->mImports.AppendElement(childRequest); RefPtr ready = childRequest->mReady.Ensure(__func__); @@ -1201,17 +1180,19 @@ CSPAllowsInlineScript(nsIScriptElement *aElement, nsIDocument *aDocument) ScriptLoadRequest* ScriptLoader::CreateLoadRequest(ScriptKind aKind, + nsIURI* aURI, nsIScriptElement* aElement, uint32_t aVersion, CORSMode aCORSMode, const SRIMetadata &aIntegrity) { if (aKind == ScriptKind::Classic) { - return new ScriptLoadRequest(aKind, aElement, aVersion, aCORSMode, + return new ScriptLoadRequest(aKind, aURI, aElement, + aVersion,aCORSMode, aIntegrity); } MOZ_ASSERT(aKind == ScriptKind::Module); - return new ModuleLoadRequest(aElement, aVersion, aCORSMode, aIntegrity, this); + return new ModuleLoadRequest(aURI, aElement, aVersion, aCORSMode, aIntegrity, this); } bool @@ -1343,9 +1324,8 @@ ScriptLoader::ProcessScriptElement(nsIScriptElement *aElement) } } - request = CreateLoadRequest(scriptKind, aElement, version, ourCORSMode, - sriMetadata); - request->mURI = scriptURI; + request = CreateLoadRequest(scriptKind, scriptURI, aElement, + version, ourCORSMode, sriMetadata); request->mIsInline = false; request->mReferrerPolicy = ourRefPolicy; @@ -1466,11 +1446,11 @@ ScriptLoader::ProcessScriptElement(nsIScriptElement *aElement) } // Inline scripts ignore ther CORS mode and are always CORS_NONE - request = CreateLoadRequest(scriptKind, aElement, version, CORS_NONE, + request = CreateLoadRequest(scriptKind, mDocument->GetDocumentURI(), aElement, + version, CORS_NONE, SRIMetadata()); // SRI doesn't apply request->mJSVersion = version; request->mIsInline = true; - request->mURI = mDocument->GetDocumentURI(); request->mLineNo = aElement->GetScriptLineNumber(); if (request->IsModuleRequest()) { @@ -2603,9 +2583,8 @@ ScriptLoader::PreloadURI(nsIURI *aURI, const nsAString &aCharset, } RefPtr request = - CreateLoadRequest(ScriptKind::Classic, nullptr, 0, + CreateLoadRequest(ScriptKind::Classic, aURI, nullptr, 0, Element::StringToCORSMode(aCrossOrigin), sriMetadata); - request->mURI = aURI; request->mIsInline = false; request->mReferrerPolicy = aReferrerPolicy; diff --git a/dom/script/ScriptLoader.h b/dom/script/ScriptLoader.h index 955ac2cb7..3cbecbf03 100644 --- a/dom/script/ScriptLoader.h +++ b/dom/script/ScriptLoader.h @@ -62,6 +62,7 @@ protected: public: ScriptLoadRequest(ScriptKind aKind, + nsIURI* aURI, nsIScriptElement* aElement, uint32_t aVersion, mozilla::CORSMode aCORSMode, @@ -81,6 +82,7 @@ public: mScriptTextBuf(nullptr), mScriptTextLength(0), mJSVersion(aVersion), + mURI(aURI), mLineNo(1), mCORSMode(aCORSMode), mIntegrity(aIntegrity), @@ -165,7 +167,7 @@ public: char16_t* mScriptTextBuf; // Holds script text for non-inline scripts. Don't size_t mScriptTextLength; // use nsString so we can give ownership to jsapi. uint32_t mJSVersion; - nsCOMPtr mURI; + const nsCOMPtr mURI; nsCOMPtr mOriginPrincipal; nsAutoCString mURL; // Keep the URI's filename alive during off thread parsing. int32_t mLineNo; @@ -470,6 +472,7 @@ private: ScriptLoadRequest* CreateLoadRequest( ScriptKind aKind, + nsIURI* aURI, nsIScriptElement* aElement, uint32_t aVersion, mozilla::CORSMode aCORSMode, @@ -588,7 +591,7 @@ private: void StartFetchingModuleDependencies(ModuleLoadRequest* aRequest); RefPtr - StartFetchingModuleAndDependencies(ModuleLoadRequest* aRequest, nsIURI* aURI); + StartFetchingModuleAndDependencies(ModuleLoadRequest* aParent, nsIURI* aURI); nsIDocument* mDocument; // [WEAK] nsCOMArray mObservers; -- cgit v1.2.3 From 10a10fd3757374123eb5e3aab1e4720f86575f47 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Thu, 6 Aug 2020 18:31:36 +0000 Subject: Issue #618 - Simplify module resolve hook to be a function pointer This is an ahead-of time port to try and address #1624. This is based on BZ 1461751 and Jon Coppeard's work in it. --- dom/script/ScriptLoader.cpp | 45 ++++++++++++++++----------------------------- dom/script/ScriptLoader.h | 5 +++-- 2 files changed, 19 insertions(+), 31 deletions(-) (limited to 'dom') diff --git a/dom/script/ScriptLoader.cpp b/dom/script/ScriptLoader.cpp index 7a3cd8fe3..9de9a1d03 100644 --- a/dom/script/ScriptLoader.cpp +++ b/dom/script/ScriptLoader.cpp @@ -787,24 +787,20 @@ ScriptLoader::StartFetchingModuleAndDependencies(ModuleLoadRequest* aParent, } // 8.1.3.8.1 HostResolveImportedModule(referencingModule, specifier) -bool -HostResolveImportedModule(JSContext* aCx, unsigned argc, JS::Value* vp) +JSObject* +HostResolveImportedModule(JSContext* aCx, JS::Handle aModule, + JS::Handle aSpecifier) { - MOZ_ASSERT(argc == 2); - JS::CallArgs args = JS::CallArgsFromVp(argc, vp); - JS::Rooted module(aCx, &args[0].toObject()); - JS::Rooted specifier(aCx, args[1].toString()); - // Let referencing module script be referencingModule.[[HostDefined]]. - JS::Value value = JS::GetModuleHostDefinedField(module); + JS::Value value = JS::GetModuleHostDefinedField(aModule); auto script = static_cast(value.toPrivate()); - MOZ_ASSERT(script->ModuleRecord() == module); + MOZ_ASSERT(script->ModuleRecord() == aModule); // Let url be the result of resolving a module specifier given referencing // module script and specifier. nsAutoJSString string; - if (!string.init(aCx, specifier)) { - return false; + if (!string.init(aCx, aSpecifier)) { + return nullptr; } nsCOMPtr uri = ResolveModuleSpecifier(script, string); @@ -820,27 +816,20 @@ HostResolveImportedModule(JSContext* aCx, unsigned argc, JS::Value* vp) MOZ_ASSERT(ms, "Resolved module not found in module map"); MOZ_ASSERT(!ms->HasParseError()); + MOZ_ASSERT(ms->ModuleRecord()); - *vp = JS::ObjectValue(*ms->ModuleRecord()); - return true; + return ms->ModuleRecord(); } -static nsresult +static void EnsureModuleResolveHook(JSContext* aCx) { - if (JS::GetModuleResolveHook(aCx)) { - return NS_OK; - } - - JS::Rooted func(aCx); - func = JS_NewFunction(aCx, HostResolveImportedModule, 2, 0, - "HostResolveImportedModule"); - if (!func) { - return NS_ERROR_FAILURE; + JSRuntime* rt = JS_GetRuntime(aCx); + if (JS::GetModuleResolveHook(rt)) { + return; } - JS::SetModuleResolveHook(aCx, func); - return NS_OK; + JS::SetModuleResolveHook(rt, HostResolveImportedModule); } void @@ -928,8 +917,7 @@ ScriptLoader::InstantiateModuleTree(ModuleLoadRequest* aRequest) return false; } - nsresult rv = EnsureModuleResolveHook(jsapi.cx()); - NS_ENSURE_SUCCESS(rv, false); + EnsureModuleResolveHook(jsapi.cx()); JS::Rooted module(jsapi.cx(), moduleScript->ModuleRecord()); bool ok = NS_SUCCEEDED(nsJSUtils::ModuleInstantiate(jsapi.cx(), module)); @@ -1963,8 +1951,7 @@ ScriptLoader::EvaluateScript(ScriptLoadRequest* aRequest) } if (aRequest->IsModuleRequest()) { - rv = EnsureModuleResolveHook(cx); - NS_ENSURE_SUCCESS(rv, rv); + EnsureModuleResolveHook(cx); ModuleLoadRequest* request = aRequest->AsModuleRequest(); MOZ_ASSERT(request->mModuleScript); diff --git a/dom/script/ScriptLoader.h b/dom/script/ScriptLoader.h index 3cbecbf03..4155f08f8 100644 --- a/dom/script/ScriptLoader.h +++ b/dom/script/ScriptLoader.h @@ -579,8 +579,9 @@ private: RefPtr WaitForModuleFetch(nsIURI* aURL); ModuleScript* GetFetchedModule(nsIURI* aURL) const; - friend bool - HostResolveImportedModule(JSContext* aCx, unsigned argc, JS::Value* vp); + friend JSObject* + HostResolveImportedModule(JSContext* aCx, JS::Handle aModule, + JS::Handle aSpecifier); nsresult CreateModuleScript(ModuleLoadRequest* aRequest); nsresult ProcessFetchedModuleSource(ModuleLoadRequest* aRequest); -- cgit v1.2.3 From c735355ae167b247fa80e03772b657fc34983f49 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Thu, 13 Aug 2020 13:42:52 +0000 Subject: Issue #618: Ignore 'event' and 'for' attributes for module scripts. Because the spec says so. --- dom/script/ScriptLoader.cpp | 53 +++++++++++++++++++++++++-------------------- 1 file changed, 30 insertions(+), 23 deletions(-) (limited to 'dom') diff --git a/dom/script/ScriptLoader.cpp b/dom/script/ScriptLoader.cpp index 9de9a1d03..903822ef5 100644 --- a/dom/script/ScriptLoader.cpp +++ b/dom/script/ScriptLoader.cpp @@ -299,8 +299,12 @@ ScriptLoader::~ScriptLoader() //