mozilla
/
gecko-dev
зеркало из https://github.com/mozilla/gecko-dev.git
1
0
Форкнуть 0
Код Задачи Пакеты Проекты Релизы Вики Активность

Bug 1912115 - HTMLImageElement shouldn't be considered broken once a load task is ongoing. r=smaug 

Clean-up some legacy nsImageLoadingContent state tracking while at it:

 * mLoading was not used to set ElementState anymore, and was only
   checked on the "loading" attribute handling, in what seems to be a
   poor man's way of preventing lazy loading to start once an image load
   is ongoing? However that should already be covered by the image load
   not being sync and checking mLazyLoad itself.

 * The depth tracking was unneeded, image notifications have been all
   async for ages.

Move some bools into nsImageLoadingContent to pack it better.

Differential Revision: https://phabricator.services.mozilla.com/D218971
This commit is contained in:
Emilio Cobos Álvarez 2024-08-11 15:59:48 +00:00
Родитель 5b039f4aa8
Коммит 9e39b3b02f
5 изменённых файлов: 97 добавлений и 129 удалений
Показать статистику Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

77
dom/base/nsImageLoadingContent.cpp
Просмотреть файл

@ -104,14 +104,13 @@ nsImageLoadingContent::nsImageLoadingContent()
mOutstandingDecodePromises(0),
mRequestGeneration(0),
mLoadingEnabled(true),
mLoading(false),
mUseUrgentStartForChannel(false),
mLazyLoading(false),
mStateChangerDepth(0),
mHasPendingLoadTask(false),
mSyncDecodingHint(false),
mInDocResponsiveContent(false),
mCurrentRequestRegistered(false),
mPendingRequestRegistered(false),
mIsStartingImageLoad(false),
mSyncDecodingHint(false) {
mPendingRequestRegistered(false) {
if (!nsContentUtils::GetImgLoaderForChannel(nullptr, nullptr)) {
mLoadingEnabled = false;
}
@ -174,12 +173,6 @@ void nsImageLoadingContent::Notify(imgIRequest* aRequest, int32_t aType,
}
}
if (aType == imgINotificationObserver::SIZE_AVAILABLE) {
// Have to check for state changes here, since we might have been in
// the LOADING state before.
UpdateImageState(true);
}
if (aType == imgINotificationObserver::LOAD_COMPLETE) {
uint32_t reqStatus;
aRequest->GetImageStatus(&reqStatus);
@ -216,7 +209,6 @@ void nsImageLoadingContent::Notify(imgIRequest* aRequest, int32_t aType,
if (container) {
container->PropagateUseCounters(GetOurOwnerDoc());
}
UpdateImageState(true);
}
}
@ -238,9 +230,6 @@ void nsImageLoadingContent::OnLoadComplete(imgIRequest* aRequest,
return;
}
// Our state may change. Watch it.
AutoStateChanger changer(this, true);
// If the pending request is loaded, switch to it.
if (aRequest == mPendingRequest) {
MakePendingRequestCurrent();
@ -260,6 +249,7 @@ void nsImageLoadingContent::OnLoadComplete(imgIRequest* aRequest,
MaybeResolveDecodePromises();
LargestContentfulPaint::MaybeProcessImageForElementTiming(mCurrentRequest,
element);
UpdateImageState(true);
}
void nsImageLoadingContent::OnUnlockedDraw() {
@ -952,8 +942,7 @@ nsImageLoadingContent::LoadImageWithChannel(nsIChannel* aChannel,
// XXX what about shouldProcess?
// Our state might change. Watch it.
AutoStateChanger changer(this, true);
auto updateStateOnExit = MakeScopeExit([&] { UpdateImageState(true); });
// Do the load.
nsCOMPtr<nsIURI> uri;
aChannel->GetOriginalURI(getter_AddRefs(uri));
@ -1026,11 +1015,6 @@ nsresult nsImageLoadingContent::LoadImage(nsIURI* aNewURI, bool aForce,
nsLoadFlags aLoadFlags,
Document* aDocument,
nsIPrincipal* aTriggeringPrincipal) {
MOZ_ASSERT(!mIsStartingImageLoad, "some evil code is reentering LoadImage.");
if (mIsStartingImageLoad) {
return NS_OK;
}
// Pending load/error events need to be canceled in some situations. This
// is not documented in the spec, but can cause site compat problems if not
// done. See bug 1309461 and https://github.com/whatwg/html/issues/1872.
@ -1065,9 +1049,6 @@ nsresult nsImageLoadingContent::LoadImage(nsIURI* aNewURI, bool aForce,
}
}
AutoRestore<bool> guard(mIsStartingImageLoad);
mIsStartingImageLoad = true;
// Data documents, or documents from DOMParser shouldn't perform image
// loading.
//
@ -1099,7 +1080,7 @@ nsresult nsImageLoadingContent::LoadImage(nsIURI* aNewURI, bool aForce,
}
// From this point on, our image state could change. Watch it.
AutoStateChanger changer(this, aNotify);
auto updateStateOnExit = MakeScopeExit([&] { UpdateImageState(aNotify); });
// Sanity check.
//
@ -1333,49 +1314,29 @@ CSSIntSize nsImageLoadingContent::GetWidthHeightForImage() {
}
void nsImageLoadingContent::UpdateImageState(bool aNotify) {
if (mStateChangerDepth > 0) {
// Ignore this call; we'll update our state when the outermost state changer
// is destroyed. Need this to work around the fact that some ImageLib
// stuff is actually sync and hence we can get OnStopDecode called while
// we're still under LoadImage, and OnStopDecode doesn't know anything about
// aNotify.
// XXX - This machinery should be removed after bug 521604.
return;
}
Element* thisElement = AsContent()->AsElement();
mLoading = false;
Element::AutoStateChangeNotifier notifier(*thisElement, aNotify);
thisElement->RemoveStatesSilently(ElementState::BROKEN);
// If we were blocked, we're broken, so are we if we don't have an image
// request at all or the image has errored.
if (!mCurrentRequest) {
if (!mLazyLoading) {
// In case of non-lazy loading, no current request means error, since we
// weren't disabled or suppressed
thisElement->AddStatesSilently(ElementState::BROKEN);
RejectDecodePromises(NS_ERROR_DOM_IMAGE_BROKEN);
const bool isBroken = [&] {
if (mLazyLoading || mHasPendingLoadTask) {
return false;
}
if (!mCurrentRequest) {
return true;
}
} else {
uint32_t currentLoadStatus;
nsresult rv = mCurrentRequest->GetImageStatus(&currentLoadStatus);
if (NS_FAILED(rv) || (currentLoadStatus & imgIRequest::STATUS_ERROR)) {
thisElement->AddStatesSilently(ElementState::BROKEN);
RejectDecodePromises(NS_ERROR_DOM_IMAGE_BROKEN);
} else if (!(currentLoadStatus & imgIRequest::STATUS_SIZE_AVAILABLE)) {
mLoading = true;
}
return NS_FAILED(rv) || currentLoadStatus & imgIRequest::STATUS_ERROR;
}();
thisElement->SetStates(ElementState::BROKEN, isBroken, aNotify);
if (isBroken) {
RejectDecodePromises(NS_ERROR_DOM_IMAGE_BROKEN);
}
}
void nsImageLoadingContent::CancelImageRequests(bool aNotify) {
RejectDecodePromises(NS_ERROR_DOM_IMAGE_INVALID_REQUEST);
AutoStateChanger changer(this, aNotify);
ClearPendingRequest(NS_BINDING_ABORTED, Some(OnNonvisible::DiscardImages));
ClearCurrentRequest(NS_BINDING_ABORTED, Some(OnNonvisible::DiscardImages));
UpdateImageState(aNotify);
}
Document* nsImageLoadingContent::GetOurOwnerDoc() {

55
dom/base/nsImageLoadingContent.h
Просмотреть файл

@ -330,25 +330,6 @@ class nsImageLoadingContent : public nsIImageLoadingContent {
~ScriptedImageObserver();
};
/**
* Struct to report state changes
*/
struct AutoStateChanger {
AutoStateChanger(nsImageLoadingContent* aImageContent, bool aNotify)
: mImageContent(aImageContent), mNotify(aNotify) {
mImageContent->mStateChangerDepth++;
}
~AutoStateChanger() {
mImageContent->mStateChangerDepth--;
mImageContent->UpdateImageState(mNotify);
}
nsImageLoadingContent* mImageContent;
bool mNotify;
};
friend struct AutoStateChanger;
/**
* Method to fire an event once we know what's going on with the image load.
*
@ -550,15 +531,8 @@ class nsImageLoadingContent : public nsIImageLoadingContent {
*/
uint32_t mRequestGeneration;
bool mLoadingEnabled : 1;
protected:
/**
* The state we had the last time we checked whether we needed to notify the
* document of a state change. These are maintained by UpdateImageState.
*/
bool mLoading : 1;
bool mLoadingEnabled : 1;
/**
* Flag to indicate whether the channel should be mark as urgent-start.
* It should be set in *Element and passed to nsContentUtils::LoadImage.
@ -570,29 +544,20 @@ class nsImageLoadingContent : public nsIImageLoadingContent {
// Represents the image is deferred loading until this element gets visible.
bool mLazyLoading : 1;
private:
/* The number of nested AutoStateChangers currently tracking our state. */
uint8_t mStateChangerDepth;
// Whether we have a pending load task scheduled (HTMLImageElement only).
bool mHasPendingLoadTask : 1;
// If true, force frames to synchronously decode images on draw.
bool mSyncDecodingHint : 1;
// Whether we're in the doc responsive content set (HTMLImageElement only).
bool mInDocResponsiveContent : 1;
private:
// Flags to indicate whether each of the current and pending requests are
// registered with the refresh driver.
bool mCurrentRequestRegistered;
bool mPendingRequestRegistered;
// TODO:
// Bug 1353685: Should ServiceWorker call SetBlockedRequest?
//
// This member is used in SetBlockedRequest, if it's true, then this call is
// triggered from LoadImage.
// If this is false, it means this call is from other places like
// ServiceWorker, then we will ignore call to SetBlockedRequest for now.
//
// Also we use this variable to check if some evil code is reentering
// LoadImage.
bool mIsStartingImageLoad;
// If true, force frames to synchronously decode images on draw.
bool mSyncDecodingHint;
};
#endif // nsImageLoadingContent_h__

53
dom/html/HTMLImageElement.cpp
Просмотреть файл

@ -92,7 +92,7 @@ class ImageLoadTask final : public MicroTaskRunnable {
void Run(AutoSlowOperation& aAso) override {
if (mElement->mPendingImageLoadTask == this) {
JSCallingLocation::AutoFallback fallback(&mCallingLocation);
mElement->mPendingImageLoadTask = nullptr;
mElement->ClearImageLoadTask();
mElement->mUseUrgentStartForChannel = mUseUrgentStartForChannel;
mElement->LoadSelectedImage(mAlwaysLoad);
}
@ -327,7 +327,7 @@ void HTMLImageElement::AfterSetAttr(int32_t aNameSpaceID, nsAtom* aName,
}
bool forceReload = false;
if (aName == nsGkAtoms::loading && !mLoading) {
if (aName == nsGkAtoms::loading) {
if (aValue && Loading(aValue->GetEnumValue()) == Loading::Lazy) {
SetLazyLoading();
} else if (aOldValue &&
@ -387,7 +387,7 @@ void HTMLImageElement::AfterSetAttr(int32_t aNameSpaceID, nsAtom* aName,
// get bound to a tree.
if (forceReload) {
mUseUrgentStartForChannel = UserActivation::IsHandlingUserInput();
UpdateSourceSyncAndQueueImageTask(true);
UpdateSourceSyncAndQueueImageTask(true, aNotify);
}
return nsGenericHTMLElement::AfterSetAttr(
@ -427,7 +427,7 @@ void HTMLImageElement::AfterMaybeChangeAttr(
mResponsiveSelector->SetDefaultSource(mSrcURI, mSrcTriggeringPrincipal);
}
mUseUrgentStartForChannel = UserActivation::IsHandlingUserInput();
UpdateSourceSyncAndQueueImageTask(true);
UpdateSourceSyncAndQueueImageTask(true, aNotify);
}
void HTMLImageElement::GetEventTargetParent(EventChainPreVisitor& aVisitor) {
@ -486,7 +486,7 @@ nsresult HTMLImageElement::BindToTree(BindContext& aContext, nsINode& aParent) {
mInDocResponsiveContent = true;
}
mUseUrgentStartForChannel = UserActivation::IsHandlingUserInput();
UpdateSourceSyncAndQueueImageTask(false);
UpdateSourceSyncAndQueueImageTask(false, /* aNotify = */ false);
}
return NS_OK;
}
@ -570,7 +570,7 @@ void HTMLImageElement::NodeInfoChanged(Document* aOldDoc) {
// per spec,
// https://html.spec.whatwg.org/#reacting-to-dom-mutations, and
// https://html.spec.whatwg.org/#reacting-to-environment-changes.
UpdateSourceSyncAndQueueImageTask(true);
UpdateSourceSyncAndQueueImageTask(true, /* aNotify = */ false);
}
// static
@ -647,7 +647,7 @@ nsresult HTMLImageElement::CopyInnerTo(HTMLImageElement* aDest) {
// doing the image load because we passed in false for aNotify. But we
// really do want it to do the load, so set it up to happen once the cloning
// reaches a stable state.
aDest->UpdateSourceSyncAndQueueImageTask(false);
aDest->UpdateSourceSyncAndQueueImageTask(false, /* aNotify = */ false);
return NS_OK;
}
@ -700,9 +700,14 @@ void HTMLImageElement::ClearForm(bool aRemoveFromForm) {
mForm = nullptr;
}
void HTMLImageElement::ClearImageLoadTask() {
mPendingImageLoadTask = nullptr;
mHasPendingLoadTask = false;
}
// Roughly corresponds to https://html.spec.whatwg.org/#update-the-image-data
void HTMLImageElement::UpdateSourceSyncAndQueueImageTask(
bool aAlwaysLoad, const HTMLSourceElement* aSkippedSource) {
bool aAlwaysLoad, bool aNotify, const HTMLSourceElement* aSkippedSource) {
// Per spec, when updating the image data or reacting to environment
// changes, we always run the full selection (including selecting the source
// element and the best fit image from srcset) even if it doesn't directly
@ -751,17 +756,17 @@ void HTMLImageElement::UpdateSourceSyncAndQueueImageTask(
// possible instead. This prevents unsound state changes from frame
// construction and such.
nsContentUtils::AddScriptRunner(
NewRunnableMethod<bool, HTMLSourceElement*>(
NewRunnableMethod<bool, bool, HTMLSourceElement*>(
"HTMLImageElement::UpdateSourceSyncAndQueueImageTask", this,
&HTMLImageElement::UpdateSourceSyncAndQueueImageTask, aAlwaysLoad,
nullptr));
/* aNotify = */ true, nullptr));
return;
}
if (mLazyLoading && mSrcURI) {
StopLazyLoading(StartLoad::No);
}
mPendingImageLoadTask = nullptr;
ClearImageLoadTask();
LoadSelectedImage(alwaysLoad);
return;
}
@ -776,6 +781,9 @@ void HTMLImageElement::UpdateSourceSyncAndQueueImageTask(
RefPtr task = new ImageLoadTask(this, alwaysLoad, mUseUrgentStartForChannel);
mPendingImageLoadTask = task;
mHasPendingLoadTask = true;
// We might have just become non-broken.
UpdateImageState(aNotify);
// The task checks this to determine if it was the last queued event, and so
// earlier tasks are implicitly canceled.
CycleCollectedJSContext::Get()->DispatchToMicroTask(task.forget());
@ -842,6 +850,9 @@ void HTMLImageElement::LoadSelectedImage(bool aAlwaysLoad) {
// to nsImageFrame::NotifyNewCurrentRequest, which takes care of that for
// us.
SetDensity(currentDensity);
// If we're (re-)loading a broken image, we might have just become broken
// again.
UpdateImageState(true);
return;
}
@ -891,7 +902,7 @@ void HTMLImageElement::PictureSourceSrcsetChanged(nsIContent* aSourceNode,
// This always triggers the image update steps per the spec, even if we are
// not using this source.
UpdateSourceSyncAndQueueImageTask(true);
UpdateSourceSyncAndQueueImageTask(true, aNotify);
}
void HTMLImageElement::PictureSourceSizesChanged(nsIContent* aSourceNode,
@ -911,7 +922,7 @@ void HTMLImageElement::PictureSourceSizesChanged(nsIContent* aSourceNode,
// This always triggers the image update steps per the spec, even if
// we are not using this source.
UpdateSourceSyncAndQueueImageTask(true);
UpdateSourceSyncAndQueueImageTask(true, aNotify);
}
void HTMLImageElement::PictureSourceMediaOrTypeChanged(nsIContent* aSourceNode,
@ -921,7 +932,7 @@ void HTMLImageElement::PictureSourceMediaOrTypeChanged(nsIContent* aSourceNode,
// This always triggers the image update steps per the spec, even if
// we are not switching to/from this source
UpdateSourceSyncAndQueueImageTask(true);
UpdateSourceSyncAndQueueImageTask(true, aNotify);
}
void HTMLImageElement::PictureSourceDimensionChanged(
@ -943,14 +954,14 @@ void HTMLImageElement::PictureSourceAdded(bool aNotify,
MOZ_ASSERT(!aSourceNode || IsPreviousSibling(aSourceNode, this),
"Should not be getting notifications for non-previous-siblings");
UpdateSourceSyncAndQueueImageTask(true);
UpdateSourceSyncAndQueueImageTask(true, aNotify);
}
void HTMLImageElement::PictureSourceRemoved(bool aNotify,
HTMLSourceElement* aSourceNode) {
MOZ_ASSERT(!aSourceNode || IsPreviousSibling(aSourceNode, this),
"Should not be getting notifications for non-previous-siblings");
UpdateSourceSyncAndQueueImageTask(true, aSourceNode);
UpdateSourceSyncAndQueueImageTask(true, aNotify, aSourceNode);
}
bool HTMLImageElement::UpdateResponsiveSource(
@ -1163,9 +1174,9 @@ bool HTMLImageElement::SelectSourceForTagWithAttrs(
}
void HTMLImageElement::DestroyContent() {
// Clear mPendingImageLoadTask to avoid running LoadSelectedImage() after
// getting destroyed.
mPendingImageLoadTask = nullptr;
// Clear the load task to avoid running LoadSelectedImage() after getting
// destroyed.
ClearImageLoadTask();
mResponsiveSelector = nullptr;
@ -1174,7 +1185,7 @@ void HTMLImageElement::DestroyContent() {
}
void HTMLImageElement::MediaFeatureValuesChanged() {
UpdateSourceSyncAndQueueImageTask(false);
UpdateSourceSyncAndQueueImageTask(false, /* aNotify = */ true);
}
bool HTMLImageElement::ShouldLoadImage() const {
@ -1211,7 +1222,7 @@ void HTMLImageElement::StopLazyLoading(StartLoad aStartLoad) {
}
if (aStartLoad == StartLoad::Yes) {
UpdateSourceSyncAndQueueImageTask(true);
UpdateSourceSyncAndQueueImageTask(true, /* aNotify = */ true);
}
}

10
dom/html/HTMLImageElement.h
Просмотреть файл

@ -267,7 +267,11 @@ class HTMLImageElement final : public nsGenericHTMLElement,
// Update the responsive source synchronously and queues a task to run
// LoadSelectedImage pending stable state.
void UpdateSourceSyncAndQueueImageTask(
bool aAlwaysLoad, const HTMLSourceElement* aSkippedSource = nullptr);
bool aAlwaysLoad, bool aNotify,
const HTMLSourceElement* aSkippedSource = nullptr);
// Clears the current image load task.
void ClearImageLoadTask();
// True if we have a srcset attribute or a <picture> parent, regardless of if
// any valid responsive sources were parsed from either.
@ -396,9 +400,6 @@ class HTMLImageElement final : public nsGenericHTMLElement,
void SetResponsiveSelector(RefPtr<ResponsiveImageSelector>&& aSource);
void SetDensity(double aDensity);
// Queue an image load task (via microtask).
void QueueImageLoadTask(bool aAlwaysLoad);
RefPtr<ImageLoadTask> mPendingImageLoadTask;
nsCOMPtr<nsIURI> mSrcURI;
nsCOMPtr<nsIPrincipal> mSrcTriggeringPrincipal;
@ -408,7 +409,6 @@ class HTMLImageElement final : public nsGenericHTMLElement,
nsCOMPtr<nsIURI> mLastSelectedSource;
// Last pixel density that was selected.
double mCurrentDensity = 1.0;
bool mInDocResponsiveContent = false;
};
} // namespace dom

31
testing/web-platform/tests/html/semantics/embedded-content/the-img-element/update-the-image-data/not-broken-while-load-task-scheduled.html Normal file
Просмотреть файл

@ -0,0 +1,31 @@
<!doctype html>
<title>Image shouldn't be broken while load task is scheduled.</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<body>
<script>
async function run_test(prop) {
let img = new Image();
img.width = 50;
img.height = 50;
img.alt = "Anything non-empty";
img[prop] = `/images/green.png?not-broken-while-load-task-scheduled-` + Math.random();
let load = new Promise(r => img.addEventListener("load", r, { once: true }));
document.body.appendChild(img);
assert_equals(img.clientWidth, 50, "clientWidth");
assert_equals(img.clientHeight, 50, "clientHeight");
assert_equals(getComputedStyle(img).height, "50px", "Computed height");
assert_equals(getComputedStyle(img).width, "50px", "Computed height");
await load;
assert_equals(img.clientWidth, 50, "clientWidth");
assert_equals(img.clientHeight, 50, "clientHeight");
assert_equals(getComputedStyle(img).height, "50px", "Computed height");
assert_equals(getComputedStyle(img).width, "50px", "Computed height");
}
promise_test(() => run_test("src"));
promise_test(() => run_test("srcset"));
</script>