Diff
Modified: trunk/Source/WebCore/ChangeLog (288101 => 288102)
--- trunk/Source/WebCore/ChangeLog 2022-01-17 20:10:13 UTC (rev 288101)
+++ trunk/Source/WebCore/ChangeLog 2022-01-17 20:17:20 UTC (rev 288102)
@@ -1,3 +1,49 @@
+2022-01-17 Wenson Hsieh <wenson_hs...@apple.com>
+
+ ImageAnalysisQueue should analyze image elements that are loaded after the call to enqueueAllImages()
+ https://bugs.webkit.org/show_bug.cgi?id=233266
+ rdar://85731875
+
+ Reviewed by Darin Adler.
+
+ Make a few adjustments to ImageAnalysisQueue, such that it continues to analyze images in the document that have
+ finished loading after triggering the initial call to `enqueueAllImages()` in the document.
+
+ Test: ImageAnalysisTests.AnalyzeDynamicallyLoadedImages
+ ImageAnalysisTests.ResetImageAnalysisAfterNavigation
+
+ * page/ImageAnalysisQueue.cpp:
+ (WebCore::ImageAnalysisQueue::enqueueIfNeeded):
+
+ Pull logic for queueing an image element for analysis into a separate helper method; we only attempt to analyze
+ image elements that have successfully loaded and contain a `CachedImage` that is larger than an arbitrarily
+ chosen size of 20px by 20px.
+
+ (WebCore::ImageAnalysisQueue::enqueueAllImages):
+
+ Refactor this to use the new `enqueueIfNeeded` method above.
+
+ (WebCore::ImageAnalysisQueue::resumeProcessing):
+ (WebCore::ImageAnalysisQueue::clear):
+ * page/ImageAnalysisQueue.h:
+
+ Add a weak hashset of elements that we've already added to the image analysis queue at some point. This prevents
+ us from continually performing image analysis on the same image if it's only being removed and reinserted in the
+ document.
+
+ * page/Page.cpp:
+ (WebCore::Page::didCommitLoad):
+
+ Additionally reset the image analysis queue when committing page load, so that the image analysis queue doesn't
+ persist and continue analyzing images even after reloading the page (or navigating away).
+
+ (WebCore::Page::didFinishLoadingImageForElement):
+
+ If it exists, tell the page's image analysis queue to add the newly loaded image.
+
+ (WebCore::Page::resetImageAnalysisQueue):
+ * page/Page.h:
+
2022-01-17 Antoine Quint <grao...@webkit.org>
Crash may occur under ComputedStyleExtractor::propertyValue()
Modified: trunk/Source/WebCore/page/ImageAnalysisQueue.cpp (288101 => 288102)
--- trunk/Source/WebCore/page/ImageAnalysisQueue.cpp 2022-01-17 20:10:13 UTC (rev 288101)
+++ trunk/Source/WebCore/page/ImageAnalysisQueue.cpp 2022-01-17 20:17:20 UTC (rev 288102)
@@ -31,7 +31,7 @@
#include "Chrome.h"
#include "ChromeClient.h"
#include "HTMLCollection.h"
-#include "HTMLElement.h"
+#include "HTMLImageElement.h"
#include "ImageOverlay.h"
#include "RenderImage.h"
#include "Timer.h"
@@ -51,33 +51,38 @@
ImageAnalysisQueue::~ImageAnalysisQueue() = default;
-void ImageAnalysisQueue::enqueueAllImages(Document& document, const String& identifier)
+void ImageAnalysisQueue::enqueueIfNeeded(HTMLImageElement& element)
{
- if (!m_page)
+ if (!is<RenderImage>(element.renderer()))
return;
- // FIXME (233266): Analyze image elements that are loaded after we've enqueued all images in the document.
- auto imageIterator = document.images()->createIterator();
- for (RefPtr node = imageIterator.next(); node; node = imageIterator.next()) {
- if (!is<HTMLElement>(*node))
- continue;
+ auto& renderer = downcast<RenderImage>(*element.renderer());
+ auto* cachedImage = renderer.cachedImage();
+ if (!cachedImage || cachedImage->errorOccurred())
+ return;
- auto& element = downcast<HTMLElement>(*node);
- if (!is<RenderImage>(element.renderer()))
- continue;
+ if (renderer.size().width() < minimumWidthForAnalysis || renderer.size().height() < minimumHeightForAnalysis)
+ return;
- auto& renderImage = downcast<RenderImage>(*element.renderer());
- auto* cachedImage = renderImage.cachedImage();
- if (!cachedImage || cachedImage->errorOccurred())
- continue;
+ if (!m_queuedElements.add(element).isNewEntry)
+ return;
- if (renderImage.size().width() < minimumWidthForAnalysis || renderImage.size().height() < minimumHeightForAnalysis)
- continue;
+ m_queue.append({ element });
+ resumeProcessing();
+}
- m_queue.append({ WeakPtr { element }, identifier });
+void ImageAnalysisQueue::enqueueAllImages(Document& document, const String& identifier)
+{
+ if (!m_page)
+ return;
+
+ if (m_identifier != identifier) {
+ clear();
+ m_identifier = identifier;
}
- resumeProcessing();
+ for (auto& image : descendantsOfType<HTMLImageElement>(document))
+ enqueueIfNeeded(image);
}
void ImageAnalysisQueue::resumeProcessing()
@@ -86,7 +91,7 @@
return;
while (!m_queue.isEmpty() && m_pendingRequestCount < maximumPendingImageAnalysisCount) {
- auto [weakElement, identifier] = m_queue.takeFirst();
+ auto weakElement = m_queue.takeFirst();
RefPtr element = weakElement.get();
if (!element || !element->isConnected())
continue;
@@ -93,7 +98,7 @@
m_pendingRequestCount++;
m_page->resetTextRecognitionResult(*element);
- m_page->chrome().client().requestTextRecognition(*element, identifier, [this, page = m_page] (auto&&) {
+ m_page->chrome().client().requestTextRecognition(*element, m_identifier, [this, page = m_page] (auto&&) {
if (!page)
return;
@@ -112,6 +117,8 @@
m_pendingRequestCount = 0;
m_resumeProcessingTimer.stop();
m_queue.clear();
+ m_queuedElements.clear();
+ m_identifier = { };
}
} // namespace WebCore
Modified: trunk/Source/WebCore/page/ImageAnalysisQueue.h (288101 => 288102)
--- trunk/Source/WebCore/page/ImageAnalysisQueue.h 2022-01-17 20:10:13 UTC (rev 288101)
+++ trunk/Source/WebCore/page/ImageAnalysisQueue.h 2022-01-17 20:17:20 UTC (rev 288102)
@@ -29,12 +29,13 @@
#include <wtf/Deque.h>
#include <wtf/FastMalloc.h>
+#include <wtf/WeakHashSet.h>
#include <wtf/WeakPtr.h>
namespace WebCore {
class Document;
-class HTMLElement;
+class HTMLImageElement;
class Page;
class Timer;
@@ -45,19 +46,18 @@
~ImageAnalysisQueue();
WEBCORE_EXPORT void enqueueAllImages(Document&, const String& identifier);
- WEBCORE_EXPORT void clear();
+ void clear();
+ void enqueueIfNeeded(HTMLImageElement&);
+
private:
void resumeProcessing();
- struct Task {
- WeakPtr<HTMLElement> element;
- String identifier;
- };
-
+ String m_identifier;
WeakPtr<Page> m_page;
Timer m_resumeProcessingTimer;
- Deque<Task> m_queue;
+ WeakHashSet<HTMLImageElement> m_queuedElements;
+ Deque<WeakPtr<HTMLImageElement>> m_queue;
unsigned m_pendingRequestCount { 0 };
};
Modified: trunk/Source/WebCore/page/Page.cpp (288101 => 288102)
--- trunk/Source/WebCore/page/Page.cpp 2022-01-17 20:10:13 UTC (rev 288101)
+++ trunk/Source/WebCore/page/Page.cpp 2022-01-17 20:17:20 UTC (rev 288102)
@@ -1314,6 +1314,7 @@
#if ENABLE(IMAGE_ANALYSIS)
resetTextRecognitionResults();
+ resetImageAnalysisQueue();
#endif
}
@@ -3615,8 +3616,17 @@
return;
frame->editor().revealSelectionIfNeededAfterLoadingImageForElement(element);
- if (auto* page = frame->page(); element->document().frame() == frame)
+
+ if (element->document().frame() != frame)
+ return;
+
+ if (auto* page = frame->page()) {
+#if ENABLE(IMAGE_ANALYSIS)
+ if (auto* queue = page->imageAnalysisQueueIfExists())
+ queue->enqueueIfNeeded(element);
+#endif
page->chrome().client().didFinishLoadingImageForElement(element);
+ }
});
}
@@ -3756,6 +3766,12 @@
return *m_imageAnalysisQueue;
}
+void Page::resetImageAnalysisQueue()
+{
+ if (auto previousQueue = std::exchange(m_imageAnalysisQueue, { }))
+ previousQueue->clear();
+}
+
void Page::updateElementsWithTextRecognitionResults()
{
if (m_textRecognitionResults.isEmptyIgnoringNullReferences())
Modified: trunk/Source/WebCore/page/Page.h (288101 => 288102)
--- trunk/Source/WebCore/page/Page.h 2022-01-17 20:10:13 UTC (rev 288101)
+++ trunk/Source/WebCore/page/Page.h 2022-01-17 20:17:20 UTC (rev 288102)
@@ -915,6 +915,7 @@
WEBCORE_EXPORT bool hasCachedTextRecognitionResult(const HTMLElement&) const;
void cacheTextRecognitionResult(const HTMLElement&, const IntRect& containerRect, const TextRecognitionResult&);
void resetTextRecognitionResult(const HTMLElement&);
+ void resetImageAnalysisQueue();
#endif
WEBCORE_EXPORT PermissionController& permissionController();
Modified: trunk/Tools/ChangeLog (288101 => 288102)
--- trunk/Tools/ChangeLog 2022-01-17 20:10:13 UTC (rev 288101)
+++ trunk/Tools/ChangeLog 2022-01-17 20:17:20 UTC (rev 288102)
@@ -1,3 +1,24 @@
+2022-01-17 Wenson Hsieh <wenson_hs...@apple.com>
+
+ ImageAnalysisQueue should analyze image elements that are loaded after the call to enqueueAllImages()
+ https://bugs.webkit.org/show_bug.cgi?id=233266
+ rdar://85731875
+
+ Reviewed by Darin Adler.
+
+ Add new API tests to check that image elements that are created and inserted after kicking off the image
+ analysis queue are successfully added to the queue, and result in platform image analysis requests, and to also
+ verify that the image analysis queue is reset after navigation.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/ImageAnalysisTests.mm:
+ (TestWebKitAPI::processRequestWithResults):
+ (TestWebKitAPI::makeImageAnalysisRequestSwizzler):
+ (TestWebKitAPI::processRequestWithError):
+ (TestWebKitAPI::TEST):
+ (TestWebKitAPI::swizzledProcessRequestWithResults): Deleted.
+ (TestWebKitAPI::swizzledProcessRequestWithError): Deleted.
+ * TestWebKitAPI/Tests/WebKitCocoa/multiple-images.html:
+
2022-01-17 Sihui Liu <sihui_...@apple.com>
Add an API test to ensure indexedDB.databases() does not create files on disk
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ImageAnalysisTests.mm (288101 => 288102)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ImageAnalysisTests.mm 2022-01-17 20:10:13 UTC (rev 288101)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ImageAnalysisTests.mm 2022-01-17 20:17:20 UTC (rev 288102)
@@ -104,15 +104,21 @@
return adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 300, 300) configuration:configuration.get()]);
}
-static void swizzledProcessRequestWithResults(id, SEL, VKImageAnalyzerRequest *, void (^)(double progress), void (^completion)(VKImageAnalysis *, NSError *))
+static void processRequestWithResults(id, SEL, VKImageAnalyzerRequest *, void (^)(double progress), void (^completion)(VKImageAnalysis *, NSError *))
{
gDidProcessRequestCount++;
completion(createImageAnalysisWithSimpleFixedResults().get(), nil);
}
+template <typename FunctionType>
+InstanceMethodSwizzler makeImageAnalysisRequestSwizzler(FunctionType function)
+{
+ return InstanceMethodSwizzler { PAL::getVKImageAnalyzerClass(), @selector(processRequest:progressHandler:completionHandler:), reinterpret_cast<IMP>(function) };
+}
+
#if PLATFORM(IOS_FAMILY)
-static void swizzledProcessRequestWithError(id, SEL, VKImageAnalyzerRequest *, void (^)(double progress), void (^completion)(VKImageAnalysis *analysis, NSError *error))
+static void processRequestWithError(id, SEL, VKImageAnalyzerRequest *, void (^)(double progress), void (^completion)(VKImageAnalysis *analysis, NSError *error))
{
gDidProcessRequestCount++;
completion(nil, [NSError errorWithDomain:NSCocoaErrorDomain code:1 userInfo:nil]);
@@ -120,7 +126,7 @@
TEST(ImageAnalysisTests, DoNotAnalyzeImagesInEditableContent)
{
- InstanceMethodSwizzler imageAnalysisRequestSwizzler { PAL::getVKImageAnalyzerClass(), @selector(processRequest:progressHandler:completionHandler:), reinterpret_cast<IMP>(swizzledProcessRequestWithError) };
+ auto requestSwizzler = makeImageAnalysisRequestSwizzler(processRequestWithError);
auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 400, 400)]);
[webView _setEditable:YES];
@@ -130,7 +136,7 @@
TEST(ImageAnalysisTests, HandleImageAnalyzerErrors)
{
- InstanceMethodSwizzler imageAnalysisRequestSwizzler { PAL::getVKImageAnalyzerClass(), @selector(processRequest:progressHandler:completionHandler:), reinterpret_cast<IMP>(swizzledProcessRequestWithError) };
+ auto requestSwizzler = makeImageAnalysisRequestSwizzler(processRequestWithError);
auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 400, 400)]);
[webView synchronouslyLoadTestPageNamed:@"image"];
@@ -140,7 +146,7 @@
TEST(ImageAnalysisTests, DoNotCrashWhenHitTestingOutsideOfWebView)
{
- InstanceMethodSwizzler imageAnalysisRequestSwizzler { PAL::getVKImageAnalyzerClass(), @selector(processRequest:progressHandler:completionHandler:), reinterpret_cast<IMP>(swizzledProcessRequestWithError) };
+ auto requestSwizzler = makeImageAnalysisRequestSwizzler(processRequestWithError);
auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 400, 400)]);
[webView synchronouslyLoadTestPageNamed:@"image"];
@@ -151,7 +157,7 @@
TEST(ImageAnalysisTests, AvoidRedundantTextRecognitionRequests)
{
- InstanceMethodSwizzler imageAnalysisRequestSwizzler { PAL::getVKImageAnalyzerClass(), @selector(processRequest:progressHandler:completionHandler:), reinterpret_cast<IMP>(swizzledProcessRequestWithResults) };
+ auto requestSwizzler = makeImageAnalysisRequestSwizzler(processRequestWithResults);
auto webView = createWebViewWithTextRecognitionEnhancements();
[webView synchronouslyLoadTestPageNamed:@"image"];
@@ -166,7 +172,7 @@
TEST(ImageAnalysisTests, StartImageAnalysisWithoutIdentifier)
{
- InstanceMethodSwizzler imageAnalysisRequestSwizzler { PAL::getVKImageAnalyzerClass(), @selector(processRequest:progressHandler:completionHandler:), reinterpret_cast<IMP>(swizzledProcessRequestWithResults) };
+ auto requestSwizzler = makeImageAnalysisRequestSwizzler(processRequestWithResults);
auto webView = createWebViewWithTextRecognitionEnhancements();
[webView synchronouslyLoadTestPageNamed:@"multiple-images"];
@@ -179,6 +185,45 @@
EXPECT_WK_STREQ(overlayText, @"Foo bar");
}
+TEST(ImageAnalysisTests, AnalyzeDynamicallyLoadedImages)
+{
+ auto requestSwizzler = makeImageAnalysisRequestSwizzler(processRequestWithResults);
+
+ auto webView = createWebViewWithTextRecognitionEnhancements();
+ [webView synchronouslyLoadTestPageNamed:@"multiple-images"];
+ [webView _startImageAnalysis:nil];
+ [webView waitForImageAnalysisRequests:5];
+
+ [webView objectByEvaluatingJavaScript:@"appendImage('apple.gif')"];
+ [webView waitForImageAnalysisRequests:6];
+
+ [webView objectByEvaluatingJavaScript:@"appendImage('icon.png')"];
+ [webView waitForImageAnalysisRequests:7];
+
+ [webView objectByEvaluatingJavaScript:@"hideAllImages()"];
+ [webView waitForNextPresentationUpdate];
+ [webView objectByEvaluatingJavaScript:@"showAllImages()"];
+ [webView waitForNextPresentationUpdate];
+ EXPECT_EQ(gDidProcessRequestCount, 7U);
+}
+
+TEST(ImageAnalysisTests, ResetImageAnalysisAfterNavigation)
+{
+ auto requestSwizzler = makeImageAnalysisRequestSwizzler(processRequestWithResults);
+
+ auto webView = createWebViewWithTextRecognitionEnhancements();
+ [webView synchronouslyLoadTestPageNamed:@"multiple-images"];
+ [webView _startImageAnalysis:nil];
+ [webView waitForImageAnalysisRequests:5];
+
+ [webView synchronouslyLoadTestPageNamed:@"simple"];
+ [webView waitForNextPresentationUpdate];
+
+ [webView synchronouslyLoadTestPageNamed:@"multiple-images"];
+ [webView waitForNextPresentationUpdate];
+ EXPECT_EQ(gDidProcessRequestCount, 5U);
+}
+
} // namespace TestWebKitAPI
#endif // ENABLE(IMAGE_ANALYSIS)
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/multiple-images.html (288101 => 288102)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/multiple-images.html 2022-01-17 20:10:13 UTC (rev 288101)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/multiple-images.html 2022-01-17 20:17:20 UTC (rev 288102)
@@ -19,6 +19,23 @@
}
return result;
}
+
+ function appendImage(imageSource)
+ {
+ const image = document.createElement("img");
+ image.src = ""
+ document.body.appendChild(image);
+ }
+
+ function hideAllImages()
+ {
+ Array.from(document.images).forEach(image => image.style.setProperty("display", "none"));
+ }
+
+ function showAllImages()
+ {
+ Array.from(document.images).forEach(image => image.style.removeProperty("display"));
+ }
</script>
</head>
<body style='margin: 0px;'>
