- Revision
- 280296
- Author
- [email protected]
- Date
- 2021-07-26 10:15:51 -0700 (Mon, 26 Jul 2021)
Log Message
Cherry-pick r280271. rdar://problem/81117003
REGRESSION (r279751): WebContent process often crashes when hovering over content on apple.com
https://bugs.webkit.org/show_bug.cgi?id=228247
rdar://81010093
Reviewed by Tim Horton.
Source/WebCore:
Add an internal testing hook that can be used to trigger text recognition for the given element. While we should
eventually combine this with another testing hook to simulate VisionKit text recognition results, the new test
using this internal hook shouldn't make its way into VisionKit anyways, so this isn't necessary for now.
See WebKit ChangeLog for more details.
Test: fast/images/text-recognition/text-recognition-in-transparent-video.html
* testing/Internals.cpp:
(WebCore::Internals::requestTextRecognition):
* testing/Internals.h:
* testing/Internals.idl:
Source/WebKit:
After r279751, the snapshot fallback codepath I added in `createShareableBitmap` to handle the edge case of
fully transparent images causes us to now take snapshots when hovering over fully transparent video elements,
and attempt to recognize text in them. This is because RenderVideo is a RenderImage subclass without a cached
image, so we'll end up going down the transparent renderer codepath instead of bailing with a null bitmap.
However, since CachedImages are null for video elements, before we even get to VisionKit, we end up crashing
with a nullptr-deref inside `WebPage::requestTextRecognition`, which assumes that `RenderImage::cachedImage()`
is non-null.
To address this, we make two minor adjustments (see below).
* WebProcess/WebCoreSupport/ShareableBitmapUtilities.cpp:
(WebKit::createShareableBitmap):
Limit the snapshotting fallback to non-media images (i.e. non-RenderMedia).
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::requestTextRecognition):
Make this robust in the case where CachedImage is null, to avoid the possibility for similar crashes in the
future.
LayoutTests:
* fast/images/text-recognition/text-recognition-in-transparent-video-expected.txt: Added.
* fast/images/text-recognition/text-recognition-in-transparent-video.html: Added.
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@280271 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Modified Paths
Added Paths
Diff
Modified: branches/safari-612.1.25-branch/LayoutTests/ChangeLog (280295 => 280296)
--- branches/safari-612.1.25-branch/LayoutTests/ChangeLog 2021-07-26 17:15:09 UTC (rev 280295)
+++ branches/safari-612.1.25-branch/LayoutTests/ChangeLog 2021-07-26 17:15:51 UTC (rev 280296)
@@ -1,3 +1,71 @@
+2021-07-26 Russell Epstein <[email protected]>
+
+ Cherry-pick r280271. rdar://problem/81117003
+
+ REGRESSION (r279751): WebContent process often crashes when hovering over content on apple.com
+ https://bugs.webkit.org/show_bug.cgi?id=228247
+ rdar://81010093
+
+ Reviewed by Tim Horton.
+
+ Source/WebCore:
+
+ Add an internal testing hook that can be used to trigger text recognition for the given element. While we should
+ eventually combine this with another testing hook to simulate VisionKit text recognition results, the new test
+ using this internal hook shouldn't make its way into VisionKit anyways, so this isn't necessary for now.
+
+ See WebKit ChangeLog for more details.
+
+ Test: fast/images/text-recognition/text-recognition-in-transparent-video.html
+
+ * testing/Internals.cpp:
+ (WebCore::Internals::requestTextRecognition):
+ * testing/Internals.h:
+ * testing/Internals.idl:
+
+ Source/WebKit:
+
+ After r279751, the snapshot fallback codepath I added in `createShareableBitmap` to handle the edge case of
+ fully transparent images causes us to now take snapshots when hovering over fully transparent video elements,
+ and attempt to recognize text in them. This is because RenderVideo is a RenderImage subclass without a cached
+ image, so we'll end up going down the transparent renderer codepath instead of bailing with a null bitmap.
+
+ However, since CachedImages are null for video elements, before we even get to VisionKit, we end up crashing
+ with a nullptr-deref inside `WebPage::requestTextRecognition`, which assumes that `RenderImage::cachedImage()`
+ is non-null.
+
+ To address this, we make two minor adjustments (see below).
+
+ * WebProcess/WebCoreSupport/ShareableBitmapUtilities.cpp:
+ (WebKit::createShareableBitmap):
+
+ Limit the snapshotting fallback to non-media images (i.e. non-RenderMedia).
+
+ * WebProcess/WebPage/WebPage.cpp:
+ (WebKit::WebPage::requestTextRecognition):
+
+ Make this robust in the case where CachedImage is null, to avoid the possibility for similar crashes in the
+ future.
+
+ LayoutTests:
+
+ * fast/images/text-recognition/text-recognition-in-transparent-video-expected.txt: Added.
+ * fast/images/text-recognition/text-recognition-in-transparent-video.html: Added.
+
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@280271 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2021-07-23 Wenson Hsieh <[email protected]>
+
+ REGRESSION (r279751): WebContent process often crashes when hovering over content on apple.com
+ https://bugs.webkit.org/show_bug.cgi?id=228247
+ rdar://81010093
+
+ Reviewed by Tim Horton.
+
+ * fast/images/text-recognition/text-recognition-in-transparent-video-expected.txt: Added.
+ * fast/images/text-recognition/text-recognition-in-transparent-video.html: Added.
+
2021-07-21 Sihui Liu <[email protected]>
Update LayoutTests/TestExpectations for imported w3c IndexedDB tests
Added: branches/safari-612.1.25-branch/LayoutTests/fast/images/text-recognition/text-recognition-in-transparent-video-expected.txt (0 => 280296)
--- branches/safari-612.1.25-branch/LayoutTests/fast/images/text-recognition/text-recognition-in-transparent-video-expected.txt (rev 0)
+++ branches/safari-612.1.25-branch/LayoutTests/fast/images/text-recognition/text-recognition-in-transparent-video-expected.txt 2021-07-26 17:15:51 UTC (rev 280296)
@@ -0,0 +1,4 @@
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: branches/safari-612.1.25-branch/LayoutTests/fast/images/text-recognition/text-recognition-in-transparent-video.html (0 => 280296)
--- branches/safari-612.1.25-branch/LayoutTests/fast/images/text-recognition/text-recognition-in-transparent-video.html (rev 0)
+++ branches/safari-612.1.25-branch/LayoutTests/fast/images/text-recognition/text-recognition-in-transparent-video.html 2021-07-26 17:15:51 UTC (rev 280296)
@@ -0,0 +1,29 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+<style>
+img, video {
+ position: absolute;
+ top: 0;
+ left: 0;
+ width: 400px;
+ height: 400px;
+}
+
+video {
+ opacity: 0;
+}
+</style>
+</head>
+<body>
+<img src=""
+<video></video>
+<script>
+jsTestIsAsync = true;
+addEventListener("load", () => {
+ internals.requestTextRecognition(document.querySelector("video"), finishJSTest);
+});
+</script>
+</body>
+</html>
\ No newline at end of file
Modified: branches/safari-612.1.25-branch/Source/WebCore/ChangeLog (280295 => 280296)
--- branches/safari-612.1.25-branch/Source/WebCore/ChangeLog 2021-07-26 17:15:09 UTC (rev 280295)
+++ branches/safari-612.1.25-branch/Source/WebCore/ChangeLog 2021-07-26 17:15:51 UTC (rev 280296)
@@ -1,3 +1,81 @@
+2021-07-26 Russell Epstein <[email protected]>
+
+ Cherry-pick r280271. rdar://problem/81117003
+
+ REGRESSION (r279751): WebContent process often crashes when hovering over content on apple.com
+ https://bugs.webkit.org/show_bug.cgi?id=228247
+ rdar://81010093
+
+ Reviewed by Tim Horton.
+
+ Source/WebCore:
+
+ Add an internal testing hook that can be used to trigger text recognition for the given element. While we should
+ eventually combine this with another testing hook to simulate VisionKit text recognition results, the new test
+ using this internal hook shouldn't make its way into VisionKit anyways, so this isn't necessary for now.
+
+ See WebKit ChangeLog for more details.
+
+ Test: fast/images/text-recognition/text-recognition-in-transparent-video.html
+
+ * testing/Internals.cpp:
+ (WebCore::Internals::requestTextRecognition):
+ * testing/Internals.h:
+ * testing/Internals.idl:
+
+ Source/WebKit:
+
+ After r279751, the snapshot fallback codepath I added in `createShareableBitmap` to handle the edge case of
+ fully transparent images causes us to now take snapshots when hovering over fully transparent video elements,
+ and attempt to recognize text in them. This is because RenderVideo is a RenderImage subclass without a cached
+ image, so we'll end up going down the transparent renderer codepath instead of bailing with a null bitmap.
+
+ However, since CachedImages are null for video elements, before we even get to VisionKit, we end up crashing
+ with a nullptr-deref inside `WebPage::requestTextRecognition`, which assumes that `RenderImage::cachedImage()`
+ is non-null.
+
+ To address this, we make two minor adjustments (see below).
+
+ * WebProcess/WebCoreSupport/ShareableBitmapUtilities.cpp:
+ (WebKit::createShareableBitmap):
+
+ Limit the snapshotting fallback to non-media images (i.e. non-RenderMedia).
+
+ * WebProcess/WebPage/WebPage.cpp:
+ (WebKit::WebPage::requestTextRecognition):
+
+ Make this robust in the case where CachedImage is null, to avoid the possibility for similar crashes in the
+ future.
+
+ LayoutTests:
+
+ * fast/images/text-recognition/text-recognition-in-transparent-video-expected.txt: Added.
+ * fast/images/text-recognition/text-recognition-in-transparent-video.html: Added.
+
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@280271 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2021-07-23 Wenson Hsieh <[email protected]>
+
+ REGRESSION (r279751): WebContent process often crashes when hovering over content on apple.com
+ https://bugs.webkit.org/show_bug.cgi?id=228247
+ rdar://81010093
+
+ Reviewed by Tim Horton.
+
+ Add an internal testing hook that can be used to trigger text recognition for the given element. While we should
+ eventually combine this with another testing hook to simulate VisionKit text recognition results, the new test
+ using this internal hook shouldn't make its way into VisionKit anyways, so this isn't necessary for now.
+
+ See WebKit ChangeLog for more details.
+
+ Test: fast/images/text-recognition/text-recognition-in-transparent-video.html
+
+ * testing/Internals.cpp:
+ (WebCore::Internals::requestTextRecognition):
+ * testing/Internals.h:
+ * testing/Internals.idl:
+
2021-07-21 Alex Christensen <[email protected]>
Add linkedOnOrAfter check for r269162
Modified: branches/safari-612.1.25-branch/Source/WebCore/testing/Internals.cpp (280295 => 280296)
--- branches/safari-612.1.25-branch/Source/WebCore/testing/Internals.cpp 2021-07-26 17:15:09 UTC (rev 280295)
+++ branches/safari-612.1.25-branch/Source/WebCore/testing/Internals.cpp 2021-07-26 17:15:51 UTC (rev 280296)
@@ -5683,6 +5683,26 @@
#endif // ENABLE(IMAGE_ANALYSIS)
+void Internals::requestTextRecognition(Element& element, RefPtr<VoidCallback>&& callback)
+{
+ auto page = contextDocument()->page();
+ if (!page) {
+ if (callback)
+ callback->handleEvent();
+ }
+
+#if ENABLE(IMAGE_ANALYSIS)
+ page->chrome().client().requestTextRecognition(element, [callback = WTFMove(callback)] (auto&&) {
+ if (callback)
+ callback->handleEvent();
+ });
+#else
+ UNUSED_PARAM(element);
+ if (callback)
+ callback->handleEvent();
+#endif
+}
+
void Internals::installImageOverlay(Element& element, Vector<ImageOverlayLine>&& lines)
{
if (!is<HTMLElement>(element))
Modified: branches/safari-612.1.25-branch/Source/WebCore/testing/Internals.h (280295 => 280296)
--- branches/safari-612.1.25-branch/Source/WebCore/testing/Internals.h 2021-07-26 17:15:09 UTC (rev 280295)
+++ branches/safari-612.1.25-branch/Source/WebCore/testing/Internals.h 2021-07-26 17:15:51 UTC (rev 280296)
@@ -898,6 +898,7 @@
~ImageOverlayLine();
};
void installImageOverlay(Element&, Vector<ImageOverlayLine>&&);
+ void requestTextRecognition(Element&, RefPtr<VoidCallback>&&);
bool isSystemPreviewLink(Element&) const;
bool isSystemPreviewImage(Element&) const;
Modified: branches/safari-612.1.25-branch/Source/WebCore/testing/Internals.idl (280295 => 280296)
--- branches/safari-612.1.25-branch/Source/WebCore/testing/Internals.idl 2021-07-26 17:15:09 UTC (rev 280295)
+++ branches/safari-612.1.25-branch/Source/WebCore/testing/Internals.idl 2021-07-26 17:15:51 UTC (rev 280296)
@@ -920,6 +920,7 @@
boolean isSystemPreviewLink(Element element);
boolean isSystemPreviewImage(Element element);
+ undefined requestTextRecognition(Element element, VoidCallback callback);
undefined installImageOverlay(Element element, sequence<ImageOverlayLine> lines);
boolean usingAppleInternalSDK();
Modified: branches/safari-612.1.25-branch/Source/WebKit/ChangeLog (280295 => 280296)
--- branches/safari-612.1.25-branch/Source/WebKit/ChangeLog 2021-07-26 17:15:09 UTC (rev 280295)
+++ branches/safari-612.1.25-branch/Source/WebKit/ChangeLog 2021-07-26 17:15:51 UTC (rev 280296)
@@ -1,3 +1,90 @@
+2021-07-26 Russell Epstein <[email protected]>
+
+ Cherry-pick r280271. rdar://problem/81117003
+
+ REGRESSION (r279751): WebContent process often crashes when hovering over content on apple.com
+ https://bugs.webkit.org/show_bug.cgi?id=228247
+ rdar://81010093
+
+ Reviewed by Tim Horton.
+
+ Source/WebCore:
+
+ Add an internal testing hook that can be used to trigger text recognition for the given element. While we should
+ eventually combine this with another testing hook to simulate VisionKit text recognition results, the new test
+ using this internal hook shouldn't make its way into VisionKit anyways, so this isn't necessary for now.
+
+ See WebKit ChangeLog for more details.
+
+ Test: fast/images/text-recognition/text-recognition-in-transparent-video.html
+
+ * testing/Internals.cpp:
+ (WebCore::Internals::requestTextRecognition):
+ * testing/Internals.h:
+ * testing/Internals.idl:
+
+ Source/WebKit:
+
+ After r279751, the snapshot fallback codepath I added in `createShareableBitmap` to handle the edge case of
+ fully transparent images causes us to now take snapshots when hovering over fully transparent video elements,
+ and attempt to recognize text in them. This is because RenderVideo is a RenderImage subclass without a cached
+ image, so we'll end up going down the transparent renderer codepath instead of bailing with a null bitmap.
+
+ However, since CachedImages are null for video elements, before we even get to VisionKit, we end up crashing
+ with a nullptr-deref inside `WebPage::requestTextRecognition`, which assumes that `RenderImage::cachedImage()`
+ is non-null.
+
+ To address this, we make two minor adjustments (see below).
+
+ * WebProcess/WebCoreSupport/ShareableBitmapUtilities.cpp:
+ (WebKit::createShareableBitmap):
+
+ Limit the snapshotting fallback to non-media images (i.e. non-RenderMedia).
+
+ * WebProcess/WebPage/WebPage.cpp:
+ (WebKit::WebPage::requestTextRecognition):
+
+ Make this robust in the case where CachedImage is null, to avoid the possibility for similar crashes in the
+ future.
+
+ LayoutTests:
+
+ * fast/images/text-recognition/text-recognition-in-transparent-video-expected.txt: Added.
+ * fast/images/text-recognition/text-recognition-in-transparent-video.html: Added.
+
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@280271 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2021-07-23 Wenson Hsieh <[email protected]>
+
+ REGRESSION (r279751): WebContent process often crashes when hovering over content on apple.com
+ https://bugs.webkit.org/show_bug.cgi?id=228247
+ rdar://81010093
+
+ Reviewed by Tim Horton.
+
+ After r279751, the snapshot fallback codepath I added in `createShareableBitmap` to handle the edge case of
+ fully transparent images causes us to now take snapshots when hovering over fully transparent video elements,
+ and attempt to recognize text in them. This is because RenderVideo is a RenderImage subclass without a cached
+ image, so we'll end up going down the transparent renderer codepath instead of bailing with a null bitmap.
+
+ However, since CachedImages are null for video elements, before we even get to VisionKit, we end up crashing
+ with a nullptr-deref inside `WebPage::requestTextRecognition`, which assumes that `RenderImage::cachedImage()`
+ is non-null.
+
+ To address this, we make two minor adjustments (see below).
+
+ * WebProcess/WebCoreSupport/ShareableBitmapUtilities.cpp:
+ (WebKit::createShareableBitmap):
+
+ Limit the snapshotting fallback to non-media images (i.e. non-RenderMedia).
+
+ * WebProcess/WebPage/WebPage.cpp:
+ (WebKit::WebPage::requestTextRecognition):
+
+ Make this robust in the case where CachedImage is null, to avoid the possibility for similar crashes in the
+ future.
+
2021-07-23 Russell Epstein <[email protected]>
Cherry-pick r280183. rdar://problem/81027380
Modified: branches/safari-612.1.25-branch/Source/WebKit/WebProcess/WebCoreSupport/ShareableBitmapUtilities.cpp (280295 => 280296)
--- branches/safari-612.1.25-branch/Source/WebKit/WebProcess/WebCoreSupport/ShareableBitmapUtilities.cpp 2021-07-26 17:15:09 UTC (rev 280295)
+++ branches/safari-612.1.25-branch/Source/WebKit/WebProcess/WebCoreSupport/ShareableBitmapUtilities.cpp 2021-07-26 17:15:51 UTC (rev 280296)
@@ -44,7 +44,7 @@
{
Ref frame = renderImage.frame();
auto colorSpaceForBitmap = screenColorSpace(frame->mainFrame().view());
- if (!renderImage.opacity() && options.useSnapshotForTransparentImages == UseSnapshotForTransparentImages::Yes) {
+ if (!renderImage.isMedia() && !renderImage.opacity() && options.useSnapshotForTransparentImages == UseSnapshotForTransparentImages::Yes) {
auto snapshotRect = renderImage.absoluteBoundingBoxRect();
if (snapshotRect.isEmpty())
return { };
Modified: branches/safari-612.1.25-branch/Source/WebKit/WebProcess/WebPage/WebPage.cpp (280295 => 280296)
--- branches/safari-612.1.25-branch/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2021-07-26 17:15:09 UTC (rev 280295)
+++ branches/safari-612.1.25-branch/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2021-07-26 17:15:51 UTC (rev 280296)
@@ -7461,7 +7461,8 @@
completionHandlers.append(WTFMove(completion));
m_elementsPendingTextRecognition.append({ makeWeakPtr(element), WTFMove(completionHandlers) });
- auto imageURL = element.document().completeURL(renderImage.cachedImage()->url().string());
+ auto cachedImage = renderImage.cachedImage();
+ auto imageURL = cachedImage ? element.document().completeURL(cachedImage->url().string()) : URL { };
sendWithAsyncReply(Messages::WebPageProxy::RequestTextRecognition(WTFMove(imageURL), WTFMove(bitmapHandle)), [webPage = makeWeakPtr(*this), weakElement = makeWeakPtr(element)] (auto&& result) {
auto protectedPage = makeRefPtr(webPage.get());
if (!protectedPage)
