Skip to content
This repository was archived by the owner on Jul 9, 2025. It is now read-only.

Commit 972aad4

Browse files
committed
Revert "Bug 1967531 - Unify SVG and HTML image loading r=emilio" for causing wpt failures in image-embedding-nesteder-data-url.html.
This reverts commit cc3e042e222da5f74ea64fb84902e7d08828cf42.
1 parent 1ea666e commit 972aad4

File tree

10 files changed

+235
-199
lines changed

10 files changed

+235
-199
lines changed

dom/base/nsImageLoadingContent.cpp

Lines changed: 2 additions & 114 deletions
Original file line numberDiff line numberDiff line change
@@ -91,52 +91,14 @@ static void PrintReqURL(imgIRequest* req) {
9191
}
9292
#endif /* DEBUG_chb */
9393

94-
class ImageLoadTask : public MicroTaskRunnable {
95-
public:
96-
ImageLoadTask(nsImageLoadingContent* aElement, bool aAlwaysLoad,
97-
bool aUseUrgentStartForChannel)
98-
: mElement(aElement),
99-
mDocument(aElement->AsContent()->OwnerDoc()),
100-
mAlwaysLoad(aAlwaysLoad),
101-
mUseUrgentStartForChannel(aUseUrgentStartForChannel) {
102-
mDocument->BlockOnload();
103-
}
104-
105-
void Run(AutoSlowOperation& aAso) override {
106-
if (mElement->mPendingImageLoadTask == this) {
107-
JSCallingLocation::AutoFallback fallback(&mCallingLocation);
108-
mElement->mUseUrgentStartForChannel = mUseUrgentStartForChannel;
109-
mElement->ClearImageLoadTask();
110-
mElement->LoadSelectedImage(mAlwaysLoad, /* aStopLazyLoading = */ false);
111-
}
112-
mDocument->UnblockOnload(false);
113-
}
114-
115-
bool Suppressed() override {
116-
nsIGlobalObject* global = mElement->AsContent()->GetOwnerGlobal();
117-
return global && global->IsInSyncOperation();
118-
}
119-
120-
bool AlwaysLoad() const { return mAlwaysLoad; }
121-
122-
private:
123-
~ImageLoadTask() = default;
124-
const RefPtr<nsImageLoadingContent> mElement;
125-
const RefPtr<dom::Document> mDocument;
126-
const JSCallingLocation mCallingLocation{JSCallingLocation::Get()};
127-
const bool mAlwaysLoad;
128-
// True if we want to set nsIClassOfService::UrgentStart to the channel to get
129-
// the response ASAP for better user responsiveness.
130-
const bool mUseUrgentStartForChannel;
131-
};
132-
13394
nsImageLoadingContent::nsImageLoadingContent()
13495
: mObserverList(nullptr),
13596
mOutstandingDecodePromises(0),
13697
mRequestGeneration(0),
13798
mLoadingEnabled(true),
13899
mUseUrgentStartForChannel(false),
139100
mLazyLoading(false),
101+
mHasPendingLoadTask(false),
140102
mSyncDecodingHint(false),
141103
mInDocResponsiveContent(false),
142104
mCurrentRequestRegistered(false),
@@ -167,80 +129,6 @@ nsImageLoadingContent::~nsImageLoadingContent() {
167129
MOZ_ASSERT(mDecodePromises.IsEmpty(), "Decode promises still unfulfilled?");
168130
}
169131

170-
void nsImageLoadingContent::QueueImageTask(
171-
nsIURI* aSrcURI, nsIPrincipal* aSrcTriggeringPrincipal, bool aForceAsync,
172-
bool aAlwaysLoad, bool aNotify) {
173-
// If loading is temporarily disabled, we don't want to queue tasks that may
174-
// then run when loading is re-enabled.
175-
// Roughly step 1 and 2.
176-
// FIXME(emilio): Would be great to do this more per-spec. We don't cancel
177-
// existing loads etc.
178-
if (!LoadingEnabled() || !GetOurOwnerDoc()->ShouldLoadImages()) {
179-
return;
180-
}
181-
182-
// Ensure that we don't overwrite a previous load request that requires
183-
// a complete load to occur.
184-
const bool alwaysLoad = aAlwaysLoad || (mPendingImageLoadTask &&
185-
mPendingImageLoadTask->AlwaysLoad());
186-
187-
// Steps 5 and 7 (sync cache check for src).
188-
const bool shouldLoadSync = [&] {
189-
if (aForceAsync) {
190-
return false;
191-
}
192-
if (!aSrcURI) {
193-
// NOTE(emilio): we need to also do a sync check for empty / invalid src,
194-
// see https://github.com/whatwg/html/issues/2429
195-
// But do it sync only when there's a current request.
196-
return !!mCurrentRequest;
197-
}
198-
return nsContentUtils::IsImageAvailable(
199-
AsContent(), aSrcURI, aSrcTriggeringPrincipal, GetCORSMode());
200-
}();
201-
202-
if (shouldLoadSync) {
203-
if (!nsContentUtils::IsSafeToRunScript()) {
204-
// If not safe to run script, we should do the sync load task as soon as
205-
// possible instead. This prevents unsound state changes from frame
206-
// construction and such.
207-
void (nsImageLoadingContent::*fp)(nsIURI*, nsIPrincipal*, bool, bool,
208-
bool) =
209-
&nsImageLoadingContent::QueueImageTask;
210-
nsContentUtils::AddScriptRunner(
211-
NewRunnableMethod<nsIURI*, nsIPrincipal*, bool, bool, bool>(
212-
"nsImageLoadingContent::QueueImageTask", this, fp, aSrcURI,
213-
aSrcTriggeringPrincipal, aForceAsync, aAlwaysLoad,
214-
/* aNotify = */ true));
215-
return;
216-
}
217-
218-
ClearImageLoadTask();
219-
LoadSelectedImage(alwaysLoad, mLazyLoading && aSrcURI);
220-
return;
221-
}
222-
223-
if (mLazyLoading) {
224-
// This check is not in the spec, but it is just a performance optimization.
225-
// The reasoning for why it is sound is that we early-return from the image
226-
// task when lazy loading, and that StopLazyLoading makes us queue a new
227-
// task (which will implicitly cancel all the pre-existing tasks).
228-
return;
229-
}
230-
231-
RefPtr task = new ImageLoadTask(this, alwaysLoad, mUseUrgentStartForChannel);
232-
mPendingImageLoadTask = task;
233-
// We might have just become non-broken.
234-
UpdateImageState(aNotify);
235-
// The task checks this to determine if it was the last queued event, and so
236-
// earlier tasks are implicitly canceled.
237-
CycleCollectedJSContext::Get()->DispatchToMicroTask(task.forget());
238-
}
239-
240-
void nsImageLoadingContent::ClearImageLoadTask() {
241-
mPendingImageLoadTask = nullptr;
242-
}
243-
244132
/*
245133
* imgINotificationObserver impl
246134
*/
@@ -1421,7 +1309,7 @@ CSSIntSize nsImageLoadingContent::GetWidthHeightForImage() {
14211309
void nsImageLoadingContent::UpdateImageState(bool aNotify) {
14221310
Element* thisElement = AsContent()->AsElement();
14231311
const bool isBroken = [&] {
1424-
if (mLazyLoading || mPendingImageLoadTask) {
1312+
if (mLazyLoading || mHasPendingLoadTask) {
14251313
return false;
14261314
}
14271315
if (!mCurrentRequest) {

dom/base/nsImageLoadingContent.h

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ class nsIURI;
3131
class nsPresContext;
3232
class nsIContent;
3333
class imgRequestProxy;
34-
class ImageLoadTask;
3534

3635
namespace mozilla {
3736
class AsyncEventDispatcher;
@@ -52,7 +51,6 @@ enum class FetchPriority : uint8_t;
5251

5352
class nsImageLoadingContent : public nsIImageLoadingContent {
5453
protected:
55-
friend class ImageLoadTask;
5654
template <typename T>
5755
using Maybe = mozilla::Maybe<T>;
5856
using Nothing = mozilla::Nothing;
@@ -539,18 +537,6 @@ class nsImageLoadingContent : public nsIImageLoadingContent {
539537
uint32_t mRequestGeneration;
540538

541539
protected:
542-
void QueueImageTask(nsIURI* aURI, nsIPrincipal* aSrcTriggeringPrincipal,
543-
bool aForceAsync, bool aAlwaysLoad, bool aNotify);
544-
void QueueImageTask(nsIURI* aURI, bool aAlwaysLoad, bool aNotify) {
545-
QueueImageTask(aURI, nullptr, false, aAlwaysLoad, aNotify);
546-
}
547-
548-
void ClearImageLoadTask();
549-
550-
virtual void LoadSelectedImage(bool aAlwaysLoad, bool aStopLazyLoading) = 0;
551-
552-
RefPtr<ImageLoadTask> mPendingImageLoadTask;
553-
554540
bool mLoadingEnabled : 1;
555541
/**
556542
* Flag to indicate whether the channel should be mark as urgent-start.
@@ -563,6 +549,9 @@ class nsImageLoadingContent : public nsIImageLoadingContent {
563549
// Represents the image is deferred loading until this element gets visible.
564550
bool mLazyLoading : 1;
565551

552+
// Whether we have a pending load task scheduled (HTMLImageElement only).
553+
bool mHasPendingLoadTask : 1;
554+
566555
// If true, force frames to synchronously decode images on draw.
567556
bool mSyncDecodingHint : 1;
568557

dom/html/HTMLImageElement.cpp

Lines changed: 120 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@
3838
#include "imgINotificationObserver.h"
3939
#include "imgRequestProxy.h"
4040

41+
#include "mozilla/CycleCollectedJSContext.h"
42+
4143
#include "mozilla/EventDispatcher.h"
4244
#include "mozilla/MappedDeclarationsBuilder.h"
4345
#include "mozilla/Maybe.h"
@@ -74,6 +76,48 @@ static bool IsPreviousSibling(const nsINode* aSubject, const nsINode* aNode) {
7476

7577
namespace mozilla::dom {
7678

79+
// Calls LoadSelectedImage on host element unless it has been superseded or
80+
// canceled -- this is the synchronous section of "update the image data".
81+
// https://html.spec.whatwg.org/#update-the-image-data
82+
class ImageLoadTask final : public MicroTaskRunnable {
83+
public:
84+
ImageLoadTask(HTMLImageElement* aElement, bool aAlwaysLoad,
85+
bool aUseUrgentStartForChannel)
86+
: mElement(aElement),
87+
mDocument(aElement->OwnerDoc()),
88+
mAlwaysLoad(aAlwaysLoad),
89+
mUseUrgentStartForChannel(aUseUrgentStartForChannel) {
90+
mDocument->BlockOnload();
91+
}
92+
93+
void Run(AutoSlowOperation& aAso) override {
94+
if (mElement->mPendingImageLoadTask == this) {
95+
JSCallingLocation::AutoFallback fallback(&mCallingLocation);
96+
mElement->ClearImageLoadTask();
97+
mElement->mUseUrgentStartForChannel = mUseUrgentStartForChannel;
98+
mElement->LoadSelectedImage(mAlwaysLoad);
99+
}
100+
mDocument->UnblockOnload(false);
101+
}
102+
103+
bool Suppressed() override {
104+
nsIGlobalObject* global = mElement->GetOwnerGlobal();
105+
return global && global->IsInSyncOperation();
106+
}
107+
108+
bool AlwaysLoad() const { return mAlwaysLoad; }
109+
110+
private:
111+
~ImageLoadTask() = default;
112+
const RefPtr<HTMLImageElement> mElement;
113+
const RefPtr<Document> mDocument;
114+
const JSCallingLocation mCallingLocation{JSCallingLocation::Get()};
115+
const bool mAlwaysLoad;
116+
// True if we want to set nsIClassOfService::UrgentStart to the channel to get
117+
// the response ASAP for better user responsiveness.
118+
const bool mUseUrgentStartForChannel;
119+
};
120+
77121
HTMLImageElement::HTMLImageElement(
78122
already_AddRefed<mozilla::dom::NodeInfo>&& aNodeInfo)
79123
: nsGenericHTMLElement(std::move(aNodeInfo)) {
@@ -686,6 +730,11 @@ void HTMLImageElement::ClearForm(bool aRemoveFromForm) {
686730
mForm = nullptr;
687731
}
688732

733+
void HTMLImageElement::ClearImageLoadTask() {
734+
mPendingImageLoadTask = nullptr;
735+
mHasPendingLoadTask = false;
736+
}
737+
689738
// Roughly corresponds to https://html.spec.whatwg.org/#update-the-image-data
690739
void HTMLImageElement::UpdateSourceSyncAndQueueImageTask(
691740
bool aAlwaysLoad, bool aNotify, const HTMLSourceElement* aSkippedSource) {
@@ -702,9 +751,72 @@ void HTMLImageElement::UpdateSourceSyncAndQueueImageTask(
702751
// Spec issue: https://github.com/whatwg/html/issues/8207.
703752
UpdateResponsiveSource(aSkippedSource);
704753

705-
nsImageLoadingContent::QueueImageTask(mSrcURI, mSrcTriggeringPrincipal,
706-
HaveSrcsetOrInPicture(), aAlwaysLoad,
707-
aNotify);
754+
// If loading is temporarily disabled, we don't want to queue tasks that may
755+
// then run when loading is re-enabled.
756+
// Roughly step 1 and 2.
757+
// FIXME(emilio): Would be great to do this more per-spec. We don't cancel
758+
// existing loads etc.
759+
if (!LoadingEnabled() || !ShouldLoadImage()) {
760+
return;
761+
}
762+
763+
// Ensure that we don't overwrite a previous load request that requires
764+
// a complete load to occur.
765+
const bool alwaysLoad = aAlwaysLoad || (mPendingImageLoadTask &&
766+
mPendingImageLoadTask->AlwaysLoad());
767+
768+
// Steps 5 and 7 (sync cache check for src).
769+
const bool shouldLoadSync = [&] {
770+
if (HaveSrcsetOrInPicture()) {
771+
return false;
772+
}
773+
if (!mSrcURI) {
774+
// NOTE(emilio): we need to also do a sync check for empty / invalid src,
775+
// see https://github.com/whatwg/html/issues/2429
776+
// But do it sync only when there's a current request.
777+
return !!mCurrentRequest;
778+
}
779+
return nsContentUtils::IsImageAvailable(
780+
this, mSrcURI, mSrcTriggeringPrincipal, GetCORSMode());
781+
}();
782+
783+
if (shouldLoadSync) {
784+
if (!nsContentUtils::IsSafeToRunScript()) {
785+
// If not safe to run script, we should do the sync load task as soon as
786+
// possible instead. This prevents unsound state changes from frame
787+
// construction and such.
788+
nsContentUtils::AddScriptRunner(
789+
NewRunnableMethod<bool, bool, HTMLSourceElement*>(
790+
"HTMLImageElement::UpdateSourceSyncAndQueueImageTask", this,
791+
&HTMLImageElement::UpdateSourceSyncAndQueueImageTask, aAlwaysLoad,
792+
/* aNotify = */ true, nullptr));
793+
return;
794+
}
795+
796+
if (mLazyLoading && mSrcURI) {
797+
StopLazyLoading(StartLoad::No);
798+
}
799+
ClearImageLoadTask();
800+
LoadSelectedImage(alwaysLoad);
801+
return;
802+
}
803+
804+
if (mLazyLoading) {
805+
// This check is not in the spec, but it is just a performance optimization.
806+
// The reasoning for why it is sound is that we early-return from the image
807+
// task when lazy loading, and that StopLazyLoading makes us queue a new
808+
// task (which will implicitly cancel all the pre-existing tasks).
809+
return;
810+
}
811+
812+
RefPtr task = new ImageLoadTask(this, alwaysLoad, mUseUrgentStartForChannel);
813+
mPendingImageLoadTask = task;
814+
mHasPendingLoadTask = true;
815+
// We might have just become non-broken.
816+
UpdateImageState(aNotify);
817+
// The task checks this to determine if it was the last queued event, and so
818+
// earlier tasks are implicitly canceled.
819+
CycleCollectedJSContext::Get()->DispatchToMicroTask(task.forget());
708820
}
709821

710822
bool HTMLImageElement::HaveSrcsetOrInPicture() const {
@@ -722,19 +834,14 @@ bool HTMLImageElement::SelectedSourceMatchesLast(nsIURI* aSelectedSource) {
722834
equal;
723835
}
724836

725-
void HTMLImageElement::LoadSelectedImage(bool aAlwaysLoad,
726-
bool aStopLazyLoading) {
837+
void HTMLImageElement::LoadSelectedImage(bool aAlwaysLoad) {
727838
// In responsive mode, we have to make sure we ran the full selection
728839
// algorithm before loading the selected image.
729840
// Use this assertion to catch any cases we missed.
730841
MOZ_ASSERT(!UpdateResponsiveSource(),
731842
"The image source should be the same because we update the "
732843
"responsive source synchronously");
733844

734-
if (aStopLazyLoading) {
735-
StopLazyLoading(StartLoad::No);
736-
}
737-
738845
// The density is default to 1.0 for the src attribute case.
739846
double currentDensity = mResponsiveSelector
740847
? mResponsiveSelector->GetSelectedImageDensity()
@@ -1111,6 +1218,10 @@ void HTMLImageElement::MediaFeatureValuesChanged() {
11111218
UpdateSourceSyncAndQueueImageTask(false, /* aNotify = */ true);
11121219
}
11131220

1221+
bool HTMLImageElement::ShouldLoadImage() const {
1222+
return OwnerDoc()->ShouldLoadImages();
1223+
}
1224+
11141225
void HTMLImageElement::SetLazyLoading() {
11151226
if (mLazyLoading) {
11161227
return;

0 commit comments

Comments
 (0)