mirror of
https://github.com/mozilla/gecko-dev.git
synced 2025-02-03 12:35:58 +00:00
Bug 1838045 - Fix load event on cached inline style sheets. r=mstange
Turns out we do fire load events on these (I thought they were only for sheets with imports). Simplify a bit some related code for links. Callers / observers need to deal with SheetComplete being called sync anyways because of this: https://searchfox.org/mozilla-central/rev/962a843f6d96283c45162c788dc72bf217fca7df/layout/style/Loader.cpp#1873 So this removes the need for SheetLoadData to be a runnable etc. Differential Revision: https://phabricator.services.mozilla.com/D180840
This commit is contained in:
Emilio Cobos Álvarez
parent
8107df398a
commit
7b001e69d1
@ -21,7 +21,6 @@
|
||||
#include "mozilla/SchedulerGroup.h"
|
||||
#include "mozilla/URLPreloader.h"
|
||||
#include "nsIChildChannel.h"
|
||||
#include "nsIRunnable.h"
|
||||
#include "nsISupportsPriority.h"
|
||||
#include "nsITimedChannel.h"
|
||||
#include "nsICachingChannel.h"
|
||||
@ -269,7 +268,7 @@ static NotNull<const Encoding*> GetFallbackEncoding(
|
||||
/********************************
|
||||
* SheetLoadData implementation *
|
||||
********************************/
|
||||
NS_IMPL_ISUPPORTS(SheetLoadData, nsIRunnable, nsIThreadObserver)
|
||||
NS_IMPL_ISUPPORTS(SheetLoadData, nsIThreadObserver)
|
||||
|
||||
SheetLoadData::SheetLoadData(css::Loader* aLoader, const nsAString& aTitle,
|
||||
nsIURI* aURI, StyleSheet* aSheet, bool aSyncLoad,
|
||||
@ -399,12 +398,6 @@ SheetLoadData::~SheetLoadData() {
|
||||
"dropping the load");
|
||||
}
|
||||
|
||||
NS_IMETHODIMP
|
||||
SheetLoadData::Run() {
|
||||
mLoader->HandleLoadEvent(*this);
|
||||
return NS_OK;
|
||||
}
|
||||
|
||||
NS_IMETHODIMP
|
||||
SheetLoadData::OnDispatchedEvent() { return NS_OK; }
|
||||
|
||||
@ -1782,8 +1775,8 @@ Result<Loader::LoadSheetResult, nsresult> Loader::LoadInlineStyle(
|
||||
if (isWorthCaching) {
|
||||
sheet = LookupInlineSheetInCache(aBuffer, sheetPrincipal);
|
||||
}
|
||||
const bool sheetFromCache = !!sheet;
|
||||
if (!sheet) {
|
||||
const bool isSheetFromCache = !!sheet;
|
||||
if (!isSheetFromCache) {
|
||||
sheet = MakeRefPtr<StyleSheet>(eAuthorSheetFeatures, aInfo.mCORSMode,
|
||||
SRIMetadata{});
|
||||
sheet->SetURIs(sheetURI, originalURI, baseURI);
|
||||
@ -1796,32 +1789,26 @@ Result<Loader::LoadSheetResult, nsresult> Loader::LoadInlineStyle(
|
||||
|
||||
auto matched = PrepareSheet(*sheet, aInfo.mTitle, aInfo.mMedia, nullptr,
|
||||
isAlternate, aInfo.mIsExplicitlyEnabled);
|
||||
|
||||
if (auto* linkStyle = LinkStyle::FromNodeOrNull(aInfo.mContent)) {
|
||||
if (auto* linkStyle = LinkStyle::FromNode(*aInfo.mContent)) {
|
||||
linkStyle->SetStyleSheet(sheet);
|
||||
}
|
||||
if (sheet->IsComplete()) {
|
||||
MOZ_ASSERT(sheet->GetOwnerNode() == aInfo.mContent);
|
||||
InsertSheetInTree(*sheet);
|
||||
}
|
||||
MOZ_ASSERT(sheet->IsComplete() == isSheetFromCache);
|
||||
|
||||
Completed completed;
|
||||
if (sheetFromCache) {
|
||||
MOZ_ASSERT(sheet->IsComplete());
|
||||
completed = Completed::Yes;
|
||||
if (dom::Document* doc = GetDocument()) {
|
||||
// We post these events for devtools, even though the applicable state has
|
||||
// not actually changed, to make the cache not observable.
|
||||
doc->PostStyleSheetApplicableStateChangeEvent(*sheet);
|
||||
}
|
||||
} else {
|
||||
auto data = MakeRefPtr<SheetLoadData>(
|
||||
this, aInfo.mTitle, nullptr, sheet, false, aInfo.mContent, isAlternate,
|
||||
matched, StylePreloadKind::None, aObserver, principal,
|
||||
aInfo.mReferrerInfo);
|
||||
MOZ_ASSERT(data->GetRequestingNode() == aInfo.mContent);
|
||||
data->mLineNumber = aLineNumber;
|
||||
auto data = MakeRefPtr<SheetLoadData>(
|
||||
this, aInfo.mTitle, nullptr, sheet, /* aSyncLoad = */ false,
|
||||
aInfo.mContent, isAlternate, matched, StylePreloadKind::None, aObserver,
|
||||
principal, aInfo.mReferrerInfo);
|
||||
MOZ_ASSERT(data->GetRequestingNode() == aInfo.mContent);
|
||||
data->mLineNumber = aLineNumber;
|
||||
|
||||
if (isSheetFromCache) {
|
||||
MOZ_ASSERT(sheet->IsComplete());
|
||||
MOZ_ASSERT(sheet->GetOwnerNode() == aInfo.mContent);
|
||||
completed = Completed::Yes;
|
||||
InsertSheetInTree(*sheet);
|
||||
NotifyOfCachedLoad(std::move(data));
|
||||
} else {
|
||||
// Parse completion releases the load data.
|
||||
//
|
||||
// Note that we need to parse synchronously, since the web expects that the
|
||||
@ -1830,9 +1817,7 @@ Result<Loader::LoadSheetResult, nsresult> Loader::LoadInlineStyle(
|
||||
NS_ConvertUTF16toUTF8 utf8(aBuffer);
|
||||
completed = ParseSheet(utf8, *data, AllowAsyncParse::No);
|
||||
if (completed == Completed::Yes) {
|
||||
// TODO(emilio): Try to cache sheets with @import rules, maybe? Should be
|
||||
// a matter of scheduling the load event appropriately...
|
||||
if (isWorthCaching && sheet->ChildSheets().IsEmpty()) {
|
||||
if (isWorthCaching) {
|
||||
mInlineSheets.InsertOrUpdate(aBuffer, std::move(sheet));
|
||||
}
|
||||
} else {
|
||||
@ -1925,10 +1910,7 @@ Result<Loader::LoadSheetResult, nsresult> Loader::LoadStyleLink(
|
||||
if (auto* linkStyle = LinkStyle::FromNodeOrNull(aInfo.mContent)) {
|
||||
linkStyle->SetStyleSheet(sheet);
|
||||
}
|
||||
if (sheet->IsComplete()) {
|
||||
MOZ_ASSERT(sheet->GetOwnerNode() == aInfo.mContent);
|
||||
InsertSheetInTree(*sheet);
|
||||
}
|
||||
MOZ_ASSERT(sheet->IsComplete() == (state == SheetState::Complete));
|
||||
|
||||
// We may get here with no content for Link: headers for example.
|
||||
MOZ_ASSERT(!aInfo.mContent || LinkStyle::FromNode(*aInfo.mContent),
|
||||
@ -1944,19 +1926,10 @@ Result<Loader::LoadSheetResult, nsresult> Loader::LoadStyleLink(
|
||||
|
||||
if (state == SheetState::Complete) {
|
||||
LOG((" Sheet already complete: 0x%p", sheet.get()));
|
||||
if (aObserver || !mObservers.IsEmpty() || aInfo.mContent) {
|
||||
rv = PostLoadEvent(std::move(data));
|
||||
if (NS_FAILED(rv)) {
|
||||
return Err(rv);
|
||||
}
|
||||
} else {
|
||||
// We don't have to notify anyone of this load, as it was complete, so
|
||||
// drop it intentionally.
|
||||
data->mIntentionallyDropped = true;
|
||||
}
|
||||
|
||||
// The load hasn't been completed yet, will be done in PostLoadEvent.
|
||||
return LoadSheetResult{Completed::No, isAlternate, matched};
|
||||
MOZ_ASSERT(sheet->GetOwnerNode() == aInfo.mContent);
|
||||
InsertSheetInTree(*sheet);
|
||||
NotifyOfCachedLoad(std::move(data));
|
||||
return LoadSheetResult{Completed::Yes, isAlternate, matched};
|
||||
}
|
||||
|
||||
// Now we need to actually load it.
|
||||
@ -2179,16 +2152,7 @@ Result<RefPtr<StyleSheet>, nsresult> Loader::InternalLoadNonDocumentSheet(
|
||||
MOZ_ASSERT(data->GetRequestingNode() == mDocument);
|
||||
if (state == SheetState::Complete) {
|
||||
LOG((" Sheet already complete"));
|
||||
if (aObserver || !mObservers.IsEmpty()) {
|
||||
rv = PostLoadEvent(std::move(data));
|
||||
if (NS_FAILED(rv)) {
|
||||
return Err(rv);
|
||||
}
|
||||
} else {
|
||||
// We don't have to notify anyone of this load, as it was complete, so
|
||||
// drop it intentionally.
|
||||
data->mIntentionallyDropped = true;
|
||||
}
|
||||
NotifyOfCachedLoad(std::move(data));
|
||||
return sheet;
|
||||
}
|
||||
|
||||
@ -2202,63 +2166,29 @@ Result<RefPtr<StyleSheet>, nsresult> Loader::InternalLoadNonDocumentSheet(
|
||||
return sheet;
|
||||
}
|
||||
|
||||
nsresult Loader::PostLoadEvent(RefPtr<SheetLoadData> aLoadData) {
|
||||
void Loader::NotifyOfCachedLoad(RefPtr<SheetLoadData> aLoadData) {
|
||||
LOG(("css::Loader::PostLoadEvent"));
|
||||
mPostedEvents.AppendElement(aLoadData);
|
||||
MOZ_ASSERT(aLoadData->mSheet->IsComplete(),
|
||||
"Only expected to be used for cached sheets");
|
||||
// If we get to this code, the stylesheet loaded correctly at some point, so
|
||||
// we can just schedule a load event and don't need to touch the data's
|
||||
// mLoadFailed.
|
||||
// Note that we do this here and not from inside our SheetComplete so that we
|
||||
// don't end up running the load event more async than needed.
|
||||
MOZ_ASSERT(!aLoadData->mLoadFailed, "Why are we marked as failed?");
|
||||
aLoadData->mSheetAlreadyComplete = true;
|
||||
|
||||
nsresult rv;
|
||||
RefPtr<SheetLoadData> runnable(aLoadData);
|
||||
if (mDocument) {
|
||||
rv = mDocument->Dispatch(TaskCategory::Other, runnable.forget());
|
||||
} else if (mDocGroup) {
|
||||
rv = mDocGroup->Dispatch(TaskCategory::Other, runnable.forget());
|
||||
} else {
|
||||
rv = SchedulerGroup::Dispatch(TaskCategory::Other, runnable.forget());
|
||||
// We need to check mURI to match DecrementOngoingLoadCount().
|
||||
if (aLoadData->mURI && aLoadData->BlocksLoadEvent()) {
|
||||
IncrementOngoingLoadCount();
|
||||
}
|
||||
|
||||
if (NS_FAILED(rv)) {
|
||||
NS_WARNING("failed to dispatch stylesheet load event");
|
||||
mPostedEvents.RemoveElement(aLoadData);
|
||||
} else {
|
||||
if (aLoadData->BlocksLoadEvent()) {
|
||||
IncrementOngoingLoadCount();
|
||||
}
|
||||
|
||||
// We want to notify the observer for this data.
|
||||
aLoadData->mMustNotify = true;
|
||||
aLoadData->mSheetAlreadyComplete = true;
|
||||
|
||||
// If we get to this code, aSheet loaded correctly at some point, so
|
||||
// we can just schedule a load event and don't need to touch the
|
||||
// data's mLoadFailed. Note that we do this here and not from
|
||||
// inside our SheetComplete so that we don't end up running the load
|
||||
// event async.
|
||||
MOZ_ASSERT(!aLoadData->mLoadFailed, "Why are we marked as failed?");
|
||||
aLoadData->ScheduleLoadEventIfNeeded();
|
||||
}
|
||||
|
||||
return rv;
|
||||
}
|
||||
|
||||
void Loader::HandleLoadEvent(SheetLoadData& aEvent) {
|
||||
// XXXbz can't assert this yet.... May not have an observer because
|
||||
// we're unblocking the parser
|
||||
// NS_ASSERTION(aEvent->mObserver, "Must have observer");
|
||||
NS_ASSERTION(aEvent.mSheet, "Must have sheet");
|
||||
|
||||
mPostedEvents.RemoveElement(&aEvent);
|
||||
SheetComplete(aEvent, NS_OK);
|
||||
SheetComplete(*aLoadData, NS_OK);
|
||||
}
|
||||
|
||||
void Loader::Stop() {
|
||||
if (mSheets) {
|
||||
mSheets->CancelLoadsForLoader(*this);
|
||||
}
|
||||
|
||||
auto arr = std::move(mPostedEvents);
|
||||
for (auto& data : arr) {
|
||||
data->Cancel();
|
||||
}
|
||||
}
|
||||
|
||||
bool Loader::HasPendingLoads() { return mOngoingLoadCount; }
|
||||
@ -2325,8 +2255,6 @@ size_t Loader::SizeOfIncludingThis(MallocSizeOf aMallocSizeOf) const {
|
||||
|
||||
// Measurement of the following members may be added later if DMD finds it is
|
||||
// worthwhile:
|
||||
// - mPostedEvents: transient, and should be small
|
||||
//
|
||||
// The following members aren't measured:
|
||||
// - mDocument, because it's a weak backpointer
|
||||
|
||||
|
||||
@ -449,8 +449,6 @@ class Loader final {
|
||||
// selected and aHasAlternateRel is false.
|
||||
IsAlternate IsAlternateSheet(const nsAString& aTitle, bool aHasAlternateRel);
|
||||
|
||||
typedef nsTArray<RefPtr<SheetLoadData>> LoadDataArray;
|
||||
|
||||
// Measure our size.
|
||||
size_t SizeOfIncludingThis(MallocSizeOf aMallocSizeOf) const;
|
||||
|
||||
@ -546,17 +544,12 @@ class Loader final {
|
||||
|
||||
RefPtr<StyleSheet> LookupInlineSheetInCache(const nsAString&, nsIPrincipal*);
|
||||
|
||||
// Post a load event for aObserver to be notified about aSheet. The
|
||||
// notification will be sent with status NS_OK unless the load event is
|
||||
// canceled at some point (in which case it will be sent with
|
||||
// NS_BINDING_ABORTED).
|
||||
nsresult PostLoadEvent(RefPtr<SheetLoadData>);
|
||||
// Synchronously notify of a cached load data.
|
||||
void NotifyOfCachedLoad(RefPtr<SheetLoadData>);
|
||||
|
||||
// Start the loads of all the sheets in mPendingDatas
|
||||
void StartDeferredLoads();
|
||||
|
||||
void HandleLoadEvent(SheetLoadData&);
|
||||
|
||||
// Note: LoadSheet is responsible for setting the sheet to complete on
|
||||
// failure.
|
||||
enum class PendingLoad { No, Yes };
|
||||
@ -607,10 +600,6 @@ class Loader final {
|
||||
|
||||
RefPtr<SharedStyleSheetCache> mSheets;
|
||||
|
||||
// The array of posted stylesheet loaded events (SheetLoadDatas) we have.
|
||||
// Note that these are rare.
|
||||
LoadDataArray mPostedEvents;
|
||||
|
||||
// Our array of "global" observers
|
||||
nsTObserverArray<nsCOMPtr<nsICSSLoaderObserver>> mObservers;
|
||||
|
||||
|
||||
@ -90,7 +90,8 @@ void SharedStyleSheetCache::InsertIfNeeded(css::SheetLoadData& aData) {
|
||||
|
||||
if (!aData.mURI) {
|
||||
LOG(" Inline or constructable style sheet, bailing");
|
||||
// Inline sheet caching happens in Loader::mInlineSheets.
|
||||
// Inline sheet caching happens in Loader::mInlineSheets, where we still
|
||||
// have the input text available.
|
||||
// Constructable sheets are not worth caching, they're always unique.
|
||||
return;
|
||||
}
|
||||
@ -156,7 +157,6 @@ void SharedStyleSheetCache::LoadCompletedInternal(
|
||||
data->mLoader->InsertSheetInTree(*data->mSheet);
|
||||
}
|
||||
data->mSheet->SetComplete();
|
||||
data->ScheduleLoadEventIfNeeded();
|
||||
} else if (data->mSheet->IsApplicable()) {
|
||||
if (dom::Document* doc = data->mLoader->GetDocument()) {
|
||||
// We post these events for devtools, even though the applicable state
|
||||
@ -164,7 +164,7 @@ void SharedStyleSheetCache::LoadCompletedInternal(
|
||||
doc->PostStyleSheetApplicableStateChangeEvent(*data->mSheet);
|
||||
}
|
||||
}
|
||||
|
||||
data->ScheduleLoadEventIfNeeded();
|
||||
aDatasToNotify.AppendElement(data);
|
||||
|
||||
NS_ASSERTION(!data->mParentData || data->mParentData->mPendingChildren != 0,
|
||||
|
||||
@ -40,7 +40,6 @@ static_assert(eAuthorSheetFeatures == 0 && eUserSheetFeatures == 1 &&
|
||||
class SheetLoadData final
|
||||
: public PreloaderBase,
|
||||
public SharedSubResourceCacheLoadingValueBase<SheetLoadData>,
|
||||
public nsIRunnable,
|
||||
public nsIThreadObserver {
|
||||
using MediaMatched = dom::LinkStyle::MediaMatched;
|
||||
using IsAlternate = dom::LinkStyle::IsAlternate;
|
||||
@ -93,7 +92,6 @@ class SheetLoadData final
|
||||
nsIChannel* aChannel);
|
||||
|
||||
NS_DECL_ISUPPORTS
|
||||
NS_DECL_NSIRUNNABLE
|
||||
NS_DECL_NSITHREADOBSERVER
|
||||
|
||||
css::Loader& Loader() { return *mLoader; }
|
||||
@ -268,7 +266,7 @@ class SheetLoadData final
|
||||
|
||||
private:
|
||||
const SheetLoadData& RootLoadData() const {
|
||||
auto* top = this;
|
||||
const auto* top = this;
|
||||
while (top->mParentData) {
|
||||
top = top->mParentData;
|
||||
}
|
||||
@ -287,7 +285,7 @@ using SheetLoadDataHolder = nsMainThreadPtrHolder<SheetLoadData>;
|
||||
* This method handles that.
|
||||
*/
|
||||
inline nsISupports* ToSupports(mozilla::css::SheetLoadData* p) {
|
||||
return NS_ISUPPORTS_CAST(nsIRunnable*, p);
|
||||
return NS_ISUPPORTS_CAST(nsIThreadObserver*, p);
|
||||
}
|
||||
|
||||
#endif // mozilla_css_SheetLoadData_h
|
||||
|
||||
@ -29,6 +29,7 @@ const anchor = document.getElementById('anchor');
|
||||
const span = document.getElementById('span');
|
||||
|
||||
test(() => {
|
||||
anchor.getBoundingClientRect();
|
||||
anchor.classList.add('active');
|
||||
assert_equals(getComputedStyle(span).color, 'rgb(255, 0, 0)',
|
||||
'The child of a transitioning element should inherit its'
|
||||
|
||||
@ -0,0 +1,23 @@
|
||||
<!doctype html>
|
||||
<meta charset="utf-8">
|
||||
<title>style elements fire load events properly</title>
|
||||
<link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">
|
||||
<link rel="author" title="Mozilla" href="https://mozilla.org">
|
||||
<link rel="help" href="https://html.spec.whatwg.org/#update-a-style-block">
|
||||
<script src="/resources/testharness.js"></script>
|
||||
<script src="/resources/testharnessreport.js"></script>
|
||||
<script>
|
||||
let NUM_LOADS = 0;
|
||||
</script>
|
||||
<style onload="++NUM_LOADS"></style>
|
||||
<style onload="++NUM_LOADS">:root { background-color: lime }</style>
|
||||
<style onload="++NUM_LOADS">:root { background-color: lime }</style> <!-- Intentionally the same -->
|
||||
<script>
|
||||
async_test(function(t) {
|
||||
assert_equals(document.styleSheets.length, 3, "Should expose the three stylesheets to the OM sync");
|
||||
assert_equals(NUM_LOADS, 0, "Should not fire load event sync");
|
||||
window.addEventListener("load", t.step_func_done(() => {
|
||||
assert_equals(NUM_LOADS, 3, "Load event should've fired for all nodes");
|
||||
}));
|
||||
});
|
||||
</script>
|
||||
Loading…
x
Reference in New Issue
Block a user