Skip to content

Commit

Permalink
Avoid calling selectImageSource when attribute value didn't change
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=267315

Reviewed by Alex Christensen.

This PR adds a fast path for setting src to the same value in HTMLImageElement::attributeChanged.
In this case, we check whether image loading is observable or not (e.g. has a service worker),
and it if isn't, we avoid going through the full image loading path and simply fire the load event instead.

* Source/WebCore/html/HTMLImageElement.cpp:
(WebCore::HTMLImageElement::attributeChanged):
* Source/WebCore/loader/ImageLoader.cpp:
(WebCore::ImageLoader::updateFromElementIgnoringPreviousErrorToSameValue):
* Source/WebCore/loader/ImageLoader.h:

Canonical link: https://commits.webkit.org/273148@main
  • Loading branch information
rniwa committed Jan 17, 2024
1 parent 99370f4 commit a9623f4
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 3 deletions.
5 changes: 4 additions & 1 deletion Source/WebCore/html/HTMLImageElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,10 @@ void HTMLImageElement::attributeChanged(const QualifiedName& name, const AtomStr
case AttributeNames::srcAttr:
case AttributeNames::srcsetAttr:
case AttributeNames::sizesAttr:
selectImageSource(RelevantMutation::Yes);
if (oldValue != newValue)
selectImageSource(RelevantMutation::Yes);
else
m_imageLoader->updateFromElementIgnoringPreviousErrorToSameValue();
break;
case AttributeNames::usemapAttr:
if (isInTreeScope() && !m_parsedUsemap.isNull())
Expand Down
13 changes: 13 additions & 0 deletions Source/WebCore/loader/ImageLoader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,19 @@ void ImageLoader::updateFromElementIgnoringPreviousError(RelevantMutation releva
updateFromElement(relevantMutation);
}

void ImageLoader::updateFromElementIgnoringPreviousErrorToSameValue()
{
if ((m_image && !m_image->allowsCaching()) || !m_failedLoadURL.isEmpty() || element().document().activeServiceWorker()) {
updateFromElementIgnoringPreviousError(RelevantMutation::Yes);
return;
}
if (m_hasPendingLoadEvent || !m_pendingURL.isEmpty())
return;
ASSERT(m_image);
m_hasPendingLoadEvent = true;
notifyFinished(*m_image, NetworkLoadMetrics { });
}

static inline void resolvePromises(Vector<RefPtr<DeferredPromise>>& promises)
{
ASSERT(!promises.isEmpty());
Expand Down
6 changes: 4 additions & 2 deletions Source/WebCore/loader/ImageLoader.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,12 @@ class ImageLoader : public CachedImageClient {
// loading if a load hasn't already been started.
void updateFromElement(RelevantMutation = RelevantMutation::No);

// This function should be called whenever the 'src' attribute is set, even if its value
// doesn't change; starts new load unconditionally (matches Firefox and Opera behavior).
// This function should be called whenever the 'src' attribute is set.
// Starts new load unconditionally (matches Firefox and Opera behavior).
void updateFromElementIgnoringPreviousError(RelevantMutation = RelevantMutation::No);

void updateFromElementIgnoringPreviousErrorToSameValue();

void elementDidMoveToNewDocument(Document&);

Element& element() { return m_element; }
Expand Down

0 comments on commit a9623f4

Please sign in to comment.