Bug 1386840: Defer loading and don't block rendering for non-matching stylesheets. r=bz,heycam

MozReview-Commit-ID: 24UJZDooGmn
This commit is contained in:
Emilio Cobos Álvarez 2018-04-24 19:17:33 +02:00
parent 31aff796a0
commit 8239c1300e
14 changed files with 139 additions and 44 deletions

View file

@ -235,12 +235,12 @@ nsContentSink::Init(nsIDocument* aDoc,
NS_IMETHODIMP
nsContentSink::StyleSheetLoaded(StyleSheet* aSheet,
bool aWasAlternate,
bool aWasDeferred,
nsresult aStatus)
{
NS_ASSERTION(!mRunsToCompletion, "How come a fragment parser observed sheets?");
if (!aWasAlternate) {
NS_ASSERTION(mPendingSheetCount > 0, "How'd that happen?");
MOZ_ASSERT(!mRunsToCompletion, "How come a fragment parser observed sheets?");
if (!aWasDeferred) {
MOZ_ASSERT(mPendingSheetCount > 0, "How'd that happen?");
--mPendingSheetCount;
if (mPendingSheetCount == 0 &&

View file

@ -38,21 +38,30 @@ public:
No,
};
enum class MediaMatched
{
Yes,
No,
};
struct Update
{
private:
bool mWillNotify;
bool mIsAlternate;
bool mMediaMatched;
public:
Update()
: mWillNotify(false)
, mIsAlternate(false)
, mMediaMatched(false)
{ }
Update(Completed aCompleted, IsAlternate aIsAlternate)
Update(Completed aCompleted, IsAlternate aIsAlternate, MediaMatched aMediaMatched)
: mWillNotify(aCompleted == Completed::No)
, mIsAlternate(aIsAlternate == IsAlternate::Yes)
, mMediaMatched(aMediaMatched == MediaMatched::Yes)
{ }
bool WillNotify() const
@ -66,7 +75,7 @@ public:
return false;
}
return !mIsAlternate;
return !mIsAlternate && mMediaMatched;
}
};

View file

@ -164,7 +164,7 @@ nsXBLResourceLoader::LoadResources(nsIContent* aBoundElement)
// nsICSSLoaderObserver
NS_IMETHODIMP
nsXBLResourceLoader::StyleSheetLoaded(StyleSheet* aSheet,
bool aWasAlternate,
bool aWasDeferred,
nsresult aStatus)
{
if (!mResources) {

View file

@ -425,11 +425,11 @@ nsXMLContentSink::OnTransformDone(nsresult aResult,
NS_IMETHODIMP
nsXMLContentSink::StyleSheetLoaded(StyleSheet* aSheet,
bool aWasAlternate,
bool aWasDeferred,
nsresult aStatus)
{
if (!mPrettyPrinting) {
return nsContentSink::StyleSheetLoaded(aSheet, aWasAlternate, aStatus);
return nsContentSink::StyleSheetLoaded(aSheet, aWasDeferred, aStatus);
}
if (!mDocument->CSSLoader()->HasPendingLoads()) {

View file

@ -974,7 +974,7 @@ txTransformNotifier::ScriptEvaluated(nsresult aResult,
NS_IMETHODIMP
txTransformNotifier::StyleSheetLoaded(StyleSheet* aSheet,
bool aWasAlternate,
bool aWasDeferred,
nsresult aStatus)
{
if (mPendingStylesheetCount == 0) {
@ -985,7 +985,7 @@ txTransformNotifier::StyleSheetLoaded(StyleSheet* aSheet,
}
// We're never waiting for alternate stylesheets
if (!aWasAlternate) {
if (!aWasDeferred) {
--mPendingStylesheetCount;
SignalTransformEnd();
}

View file

@ -2879,14 +2879,13 @@ XULDocument::DoneWalking()
NS_IMETHODIMP
XULDocument::StyleSheetLoaded(StyleSheet* aSheet,
bool aWasAlternate,
bool aWasDeferred,
nsresult aStatus)
{
if (!aWasAlternate) {
if (!aWasDeferred) {
// Don't care about when alternate sheets finish loading
NS_ASSERTION(mPendingSheets > 0,
"Unexpected StyleSheetLoaded notification");
MOZ_ASSERT(mPendingSheets > 0,
"Unexpected StyleSheetLoaded notification");
--mPendingSheets;

View file

@ -3440,7 +3440,7 @@ HTMLEditor::DebugUnitTests(int32_t* outNumTests,
NS_IMETHODIMP
HTMLEditor::StyleSheetLoaded(StyleSheet* aSheet,
bool aWasAlternate,
bool aWasDeferred,
nsresult aStatus)
{
AutoPlaceholderBatch batchIt(this);

View file

@ -1868,7 +1868,7 @@ FontFaceSet::PrefEnabled()
NS_IMETHODIMP
FontFaceSet::StyleSheetLoaded(StyleSheet* aSheet,
bool aWasAlternate,
bool aWasDeferred,
nsresult aStatus)
{
CheckLoadingFinished();

View file

@ -145,6 +145,7 @@ SheetLoadData::SheetLoadData(Loader* aLoader,
StyleSheet* aSheet,
nsIStyleSheetLinkingElement* aOwningElement,
bool aIsAlternate,
bool aMediaMatches,
nsICSSLoaderObserver* aObserver,
nsIPrincipal* aLoaderPrincipal,
nsINode* aRequestingNode)
@ -163,6 +164,7 @@ SheetLoadData::SheetLoadData(Loader* aLoader,
, mIsCancelled(false)
, mMustNotify(false)
, mWasAlternate(aIsAlternate)
, mMediaMatched(aMediaMatches)
, mUseSystemPrincipal(false)
, mSheetAlreadyComplete(false)
, mIsCrossOriginNoCORS(false)
@ -199,6 +201,7 @@ SheetLoadData::SheetLoadData(Loader* aLoader,
, mIsCancelled(false)
, mMustNotify(false)
, mWasAlternate(false)
, mMediaMatched(true)
, mUseSystemPrincipal(false)
, mSheetAlreadyComplete(false)
, mIsCrossOriginNoCORS(false)
@ -245,6 +248,7 @@ SheetLoadData::SheetLoadData(Loader* aLoader,
, mIsCancelled(false)
, mMustNotify(false)
, mWasAlternate(false)
, mMediaMatched(true)
, mUseSystemPrincipal(aUseSystemPrincipal)
, mSheetAlreadyComplete(false)
, mIsCrossOriginNoCORS(false)
@ -417,7 +421,7 @@ Loader::DropDocumentReference(void)
// loads should short-circuit through the mDocument check in
// LoadSheet and just end up in SheetComplete immediately
if (mSheets) {
StartAlternateLoads();
StartDeferredLoads();
}
}
@ -1117,12 +1121,32 @@ Loader::CreateSheet(nsIURI* aURI,
return NS_OK;
}
static Loader::MediaMatched
MediaListMatches(const MediaList* aMediaList, const nsIDocument* aDocument)
{
if (!aMediaList || !aDocument) {
return Loader::MediaMatched::Yes;
}
nsPresContext* pc = aDocument->GetPresContext();
if (!pc) {
// Conservatively assume a match.
return Loader::MediaMatched::Yes;
}
if (aMediaList->Matches(pc)) {
return Loader::MediaMatched::Yes;
}
return Loader::MediaMatched::No;
}
/**
* PrepareSheet() handles setting the media and title on the sheet, as
* well as setting the enabled state based on the title and whether
* the sheet had "alternate" in its rel.
*/
void
Loader::MediaMatched
Loader::PrepareSheet(StyleSheet* aSheet,
const nsAString& aTitle,
const nsAString& aMediaString,
@ -1143,6 +1167,7 @@ Loader::PrepareSheet(StyleSheet* aSheet,
aSheet->SetTitle(aTitle);
aSheet->SetEnabled(aIsAlternate == IsAlternate::No);
return MediaListMatches(mediaList, mDocument);
}
/**
@ -1405,7 +1430,7 @@ Loader::LoadSheet(SheetLoadData* aLoadData,
data = data->mNext;
}
data->mNext = aLoadData; // transfer ownership
if (aSheetState == eSheetPending && !aLoadData->mWasAlternate) {
if (aSheetState == eSheetPending && !aLoadData->ShouldDefer()) {
// Kick the load off; someone cares about it right away
#ifdef DEBUG
@ -1500,7 +1525,7 @@ Loader::LoadSheet(SheetLoadData* aLoadData,
return rv;
}
if (!aLoadData->mWasAlternate) {
if (!aLoadData->ShouldDefer()) {
nsCOMPtr<nsIClassOfService> cos(do_QueryInterface(channel));
if (cos) {
cos->AddClassFlags(nsIClassOfService::Leader);
@ -1717,7 +1742,7 @@ Loader::SheetComplete(SheetLoadData* aLoadData, nsresult aStatus)
if (data->mObserver) {
LOG((" Notifying observer %p for data %p. wasAlternate: %d",
data->mObserver.get(), data, data->mWasAlternate));
data->mObserver->StyleSheetLoaded(data->mSheet, data->mWasAlternate,
data->mObserver->StyleSheetLoaded(data->mSheet, data->ShouldDefer(),
aStatus);
}
@ -1732,8 +1757,8 @@ Loader::SheetComplete(SheetLoadData* aLoadData, nsresult aStatus)
}
if (mSheets->mLoadingDatas.Count() == 0 && mSheets->mPendingDatas.Count() > 0) {
LOG((" No more loading sheets; starting alternates"));
StartAlternateLoads();
LOG((" No more loading sheets; starting deferred loads"));
StartDeferredLoads();
}
}
@ -1915,7 +1940,7 @@ Loader::LoadInlineStyle(nsIContent* aElement,
LOG((" Sheet is alternate: %d", static_cast<int>(isAlternate)));
PrepareSheet(sheet, aTitle, aMedia, nullptr, isAlternate);
auto matched = PrepareSheet(sheet, aTitle, aMedia, nullptr, isAlternate);
if (aElement->HasFlag(NODE_IS_IN_SHADOW_TREE)) {
ShadowRoot* containingShadow = aElement->GetContainingShadow();
@ -1940,6 +1965,7 @@ Loader::LoadInlineStyle(nsIContent* aElement,
SheetLoadData* data = new SheetLoadData(this, aTitle, nullptr, sheet,
owningElement,
isAlternate == IsAlternate::Yes,
matched == MediaMatched::Yes,
aObserver, nullptr,
static_cast<nsINode*>(aElement));
@ -1965,7 +1991,7 @@ Loader::LoadInlineStyle(nsIContent* aElement,
if (!completed) {
data->mMustNotify = true;
}
return LoadSheetResult { completed ? Completed::Yes : Completed::No, isAlternate };
return LoadSheetResult { completed ? Completed::Yes : Completed::No, isAlternate, matched };
}
Result<Loader::LoadSheetResult, nsresult>
@ -2038,7 +2064,7 @@ Loader::LoadStyleLink(nsIContent* aElement,
LOG((" Sheet is alternate: %d", static_cast<int>(isAlternate)));
PrepareSheet(sheet, aTitle, aMedia, nullptr, isAlternate);
auto matched = PrepareSheet(sheet, aTitle, aMedia, nullptr, isAlternate);
// FIXME(emilio, bug 1410578): Shadow DOM should be handled here too.
rv = InsertSheetInDoc(sheet, aElement, mDocument);
@ -2051,14 +2077,19 @@ Loader::LoadStyleLink(nsIContent* aElement,
if (state == eSheetComplete) {
LOG((" Sheet already complete: 0x%p", sheet.get()));
if (aObserver || !mObservers.IsEmpty() || owningElement) {
rv = PostLoadEvent(aURL, sheet, aObserver, isAlternate, owningElement);
rv = PostLoadEvent(aURL,
sheet,
aObserver,
isAlternate,
matched,
owningElement);
if (NS_FAILED(rv)) {
return Err(rv);
}
}
// The load hasn't been completed yet, will be done in PostLoadEvent.
return LoadSheetResult { Completed::No, isAlternate };
return LoadSheetResult { Completed::No, isAlternate, matched };
}
// Now we need to actually load it
@ -2066,15 +2097,21 @@ Loader::LoadStyleLink(nsIContent* aElement,
SheetLoadData* data = new SheetLoadData(this, aTitle, aURL, sheet,
owningElement,
isAlternate == IsAlternate::Yes,
matched == MediaMatched::Yes,
aObserver, principal, requestingNode);
NS_ADDREF(data);
// If we have to parse and it's an alternate non-inline, defer it
auto result = LoadSheetResult { Completed::No, isAlternate, matched };
MOZ_ASSERT(result.ShouldBlock() == !data->ShouldDefer(),
"These should better match!");
// If we have to parse and it's a non-blocking non-inline sheet, defer it.
if (aURL &&
state == eSheetNeedsParser &&
mSheets->mLoadingDatas.Count() != 0 &&
isAlternate == IsAlternate::Yes) {
LOG((" Deferring alternate sheet load"));
!result.ShouldBlock()) {
LOG((" Deferring sheet load"));
URIPrincipalReferrerPolicyAndCORSModeHashKey key(data->mURI,
data->mLoaderPrincipal,
data->mSheet->GetCORSMode(),
@ -2082,7 +2119,7 @@ Loader::LoadStyleLink(nsIContent* aElement,
mSheets->mPendingDatas.Put(&key, data);
data->mMustNotify = true;
return LoadSheetResult { Completed::No, isAlternate };
return result;
}
// Load completion will free the data
@ -2092,7 +2129,7 @@ Loader::LoadStyleLink(nsIContent* aElement,
}
data->mMustNotify = true;
return LoadSheetResult { Completed::No, isAlternate };
return result;
}
static bool
@ -2374,7 +2411,12 @@ Loader::InternalLoadNonDocumentSheet(nsIURI* aURL,
if (state == eSheetComplete) {
LOG((" Sheet already complete"));
if (aObserver || !mObservers.IsEmpty()) {
rv = PostLoadEvent(aURL, sheet, aObserver, IsAlternate::No, nullptr);
rv = PostLoadEvent(aURL,
sheet,
aObserver,
IsAlternate::No,
MediaMatched::Yes,
nullptr);
}
if (aSheet) {
sheet.swap(*aSheet);
@ -2411,6 +2453,7 @@ Loader::PostLoadEvent(nsIURI* aURI,
StyleSheet* aSheet,
nsICSSLoaderObserver* aObserver,
IsAlternate aWasAlternate,
MediaMatched aMediaMatched,
nsIStyleSheetLinkingElement* aElement)
{
LOG(("css::Loader::PostLoadEvent"));
@ -2424,6 +2467,7 @@ Loader::PostLoadEvent(nsIURI* aURI,
aSheet,
aElement,
aWasAlternate == IsAlternate::Yes,
aMediaMatched == MediaMatched::Yes,
aObserver,
nullptr,
mDocument);
@ -2573,7 +2617,7 @@ Loader::RemoveObserver(nsICSSLoaderObserver* aObserver)
}
void
Loader::StartAlternateLoads()
Loader::StartDeferredLoads()
{
NS_PRECONDITION(mSheets, "Don't call me!");
LoadDataArray arr(mSheets->mPendingDatas.Count());

View file

@ -192,6 +192,7 @@ class Loader final {
public:
typedef nsIStyleSheetLinkingElement::IsAlternate IsAlternate;
typedef nsIStyleSheetLinkingElement::MediaMatched MediaMatched;
typedef nsIStyleSheetLinkingElement::Completed Completed;
typedef nsIStyleSheetLinkingElement::Update LoadSheetResult;
@ -513,11 +514,11 @@ private:
// pass both.
//
// This method will set the sheet's enabled state based on aIsAlternate
void PrepareSheet(StyleSheet* aSheet,
const nsAString& aTitle,
const nsAString& aMediaString,
dom::MediaList* aMediaList,
IsAlternate);
MediaMatched PrepareSheet(StyleSheet* aSheet,
const nsAString& aTitle,
const nsAString& aMediaString,
dom::MediaList* aMediaList,
IsAlternate);
nsresult InsertSheetInDoc(StyleSheet* aSheet,
nsIContent* aLinkingContent,
@ -550,10 +551,11 @@ private:
StyleSheet* aSheet,
nsICSSLoaderObserver* aObserver,
IsAlternate aWasAlternate,
MediaMatched aMediaMatched,
nsIStyleSheetLinkingElement* aElement);
// Start the loads of all the sheets in mPendingDatas
void StartAlternateLoads();
void StartDeferredLoads();
// Handle an event posted by PostLoadEvent
void HandleLoadEvent(SheetLoadData* aEvent);

View file

@ -93,7 +93,7 @@ NS_IMPL_ISUPPORTS(PreloadedStyleSheet::StylesheetPreloadObserver,
NS_IMETHODIMP
PreloadedStyleSheet::StylesheetPreloadObserver::StyleSheetLoaded(
StyleSheet* aSheet, bool aWasAlternate, nsresult aStatus)
StyleSheet* aSheet, bool aWasDeferred, nsresult aStatus)
{
MOZ_DIAGNOSTIC_ASSERT(!mPreloadedSheet->mLoaded);
mPreloadedSheet->mLoaded = true;

View file

@ -50,6 +50,7 @@ public:
StyleSheet* aSheet,
nsIStyleSheetLinkingElement* aOwningElement,
bool aIsAlternate,
bool aMediaMatches,
nsICSSLoaderObserver* aObserver,
nsIPrincipal* aLoaderPrincipal,
nsINode* aRequestingNode);
@ -157,6 +158,10 @@ public:
// created.
bool mWasAlternate : 1;
// mMediaMatched is true if the sheet matched its medialist when the load data
// was created.
bool mMediaMatched : 1;
// mUseSystemPrincipal is true if the system principal should be used for
// this sheet, no matter what the channel principal is. Only true for sync
// loads.
@ -199,6 +204,11 @@ public:
// is non-null.
const Encoding* mPreloadEncoding;
bool ShouldDefer() const
{
return mWasAlternate || !mMediaMatched;
}
private:
void FireLoadEvent(nsIThreadInternal* aThread);
};

View file

@ -369,3 +369,4 @@ skip-if = toolkit == 'android' # TIMED_OUT for android
[test_first_line_restrictions.html]
[test_placeholder_restrictions.html]
[test_mql_event_listener_leaks.html]
[test_non_matching_sheet_media.html]

View file

@ -0,0 +1,30 @@
<!doctype html>
<!-- https://bugzilla.mozilla.org/show_bug.cgi?id=1386840 -->
<title>Test for Bug 1386840: non-matching media list doesn't block rendering</title>
<link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script>
var t = async_test("Test that <link> doesn't block rendering with non-matching media");
var loadFired = false;
var scriptExecuted = false;
function sheetLoaded() {
loadFired = true;
assert_true(scriptExecuted, "Shouldn't wait for load to execute script");
t.done();
}
</script>
<!--
NOTE(emilio): This can theoretically race if an HTTP packet boundary with a
very long delay came right after the link and before the script. If this
ever becomes a problem, the way to fix this is using document.write() as
explained in bug 1386840 comment 12.
-->
<link rel="stylesheet" href="data:text/css,foo {}" media="print" onload="t.step(sheetLoaded)">
<script>
t.step(function() {
scriptExecuted = true;
assert_false(loadFired, "Shouldn't have waited for load");
});
</script>