Skip to content
Permalink
Browse files
ImageAnalysisTests.AnalyzeDynamicallyLoadedImages is a flaky timeout
https://bugs.webkit.org/show_bug.cgi?id=240847
rdar://93268890

Reviewed by Tim Horton.

This API test was created to verify that ImageAnalysisQueue detects and enqueues images that are
inserted into the document via script, after `-_startImageAnalysis:` has been invoked. Currently, it
occasionally times out, when the logic in `ImageAnalysisQueue::enqueueIfNeeded` to reject tiny
(< 20 by 20) images from the analysis queue falsely prevents us from enqueueing the newly inserted
image element.

Note that `enqueueIfNeeded` is called after the image has finished loading (i.e., dispatched its
"load" event for the new image). However, in this state, the image element's renderer hasn't
necessarily been laid out yet since its image source has changed, which means that the actual size
of the renderer might still be empty (0 by 0) when invoking `enqueueIfNeeded`. Since we currently
consult `RenderBox::size()` on the `RenderImage` in this method, we end up bailing too early.

Instead of checking the size of the renderer, we should be checking the size of the image instead
since the image itself is what's used to generate the bitmap image, over which we'll eventually run
image analysis. Unlike the renderer, this cached image's size doesn't depend on layout, and is
guaranteed to be up to date when the image has finished loading. This also allows us to simplify the
logic here -- since null `WebCore::Image`s are empty, checking that both the width and height of the
image are at least 20 is sufficient to avoid queueing null images for analysis.

* Source/WebCore/page/ImageAnalysisQueue.cpp:
(WebCore::ImageAnalysisQueue::enqueueIfNeeded):

Canonical link: https://commits.webkit.org/250915@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@294747 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
whsieh committed May 24, 2022
1 parent b85b20b commit 74f96353e90c396b3644befc25f8a1fbe7dadd81
Showing 1 changed file with 1 addition and 4 deletions.
@@ -65,10 +65,7 @@ void ImageAnalysisQueue::enqueueIfNeeded(HTMLImageElement& element)
return;

auto* image = cachedImage->image();
if (!image || image->isNull())
return;

if (renderer.size().width() < minimumWidthForAnalysis || renderer.size().height() < minimumHeightForAnalysis)
if (!image || image->width() < minimumWidthForAnalysis || image->height() < minimumHeightForAnalysis)
return;

bool shouldAddToQueue = [&] {

0 comments on commit 74f9635

Please sign in to comment.