UnifiedPDF: Find highlights sometimes appear in the wrong place if sc…

…rolling was needed

https://bugs.webkit.org/show_bug.cgi?id=271087
rdar://123462967

Reviewed by Abrar Rahman Protyasha and Simon Fraser.

Fix five bugs in one fell swoop:

1) Find highlights were sometimes painted in the wrong location, often overlapping UI

We weren't treating our programmatic scrolls as programmatic, letting them be
async and then making the TextIndicator in the wrong location (before the scroll
was synchronized).

Fix this by marking the scrolls as programmatic explicitly. Factor this out for
use of the whole plugin.

2) Find highlights were sometimes blank, especially when far from the current scroll position

We can't use async rendering for text indicator snapshots, because we can't repaint
them when the tiles come in. Make async rendering opt-in, and only opt-in when we're
painting the contents layer, not for other calls into painting code.

3) Find highlights didn't scroll to reveal when their top left corner was visible,
even if most of the match was off screen.

Adopt correct rect-revelation logic, as used to reveal form fields, and share with that code.

4) Software-painted snaphots are often blank
5) Printed PDFs in subframes are rasterized instead of remaining vectors

These are both async rendering regressions that are fixed by the fix for (2).

* Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/UnifiedPDFPlugin.h:
* Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/UnifiedPDFPlugin.mm:
(WebKit::UnifiedPDFPlugin::paintContents):
Opt contents layer painting into async rendering.

(WebKit::UnifiedPDFPlugin::paintPDFContent):
Make async rendering opt-in.

(WebKit::UnifiedPDFPlugin::scrollToPointInContentsSpace):
Factor out this somewhat messy code to temporarily change the currentScrollType
on our ScrollableArea to programmatic.

(WebKit::UnifiedPDFPlugin::revealRectInContentsSpace):
Factor out the logic to reveal a rectangle.

(WebKit::UnifiedPDFPlugin::scrollToPointInPage):
(WebKit::UnifiedPDFPlugin::scrollToPage):
Adopt scrollToPointInContentsSpace.

(WebKit::UnifiedPDFPlugin::findString):
(WebKit::UnifiedPDFPlugin::setActiveAnnotation):
Adopt revealRectInContentsSpace. In the find case, this fixes (3).

Canonical link: https://commits.webkit.org/276216@main

Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/UnifiedPDFPlugin.h

@@ -344,8 +344,9 @@ class UnifiedPDFPlugin final : public PDFPluginBase, public WebCore::GraphicsLay
     // Package up the data needed to paint a set of pages for the given clip, for use by UnifiedPDFPlugin::paintPDFContent and async rendering.
     PDFPageCoverage pageCoverageForRect(const WebCore::FloatRect& clipRect) const;
 
-    enum class PaintingBehavior : uint8_t { All, PageContentsOnly };
-    void paintPDFContent(WebCore::GraphicsContext&, const WebCore::FloatRect& clipRect, PaintingBehavior = PaintingBehavior::All);
+    enum class PaintingBehavior : bool { All, PageContentsOnly };
+    enum class AllowsAsyncRendering : bool { No, Yes };
+    void paintPDFContent(WebCore::GraphicsContext&, const WebCore::FloatRect& clipRect, PaintingBehavior = PaintingBehavior::All, AllowsAsyncRendering = AllowsAsyncRendering::No);
 
     void ensureLayers();
     void updatePageBackgroundLayers();
@@ -377,6 +378,8 @@ class UnifiedPDFPlugin final : public PDFPluginBase, public WebCore::GraphicsLay
     WebCore::ScrollingCoordinator* scrollingCoordinator();
     void createScrollingNodeIfNecessary();
 
+    void revealRectInContentsSpace(WebCore::FloatRect);
+    void scrollToPointInContentsSpace(WebCore::FloatPoint);
     void scrollToPDFDestination(PDFDestination *);
     void scrollToPointInPage(WebCore::FloatPoint pointInPDFPageSpace, PDFDocumentLayout::PageIndex);
     void scrollToPage(PDFDocumentLayout::PageIndex);

Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/UnifiedPDFPlugin.mm

@@ -706,7 +706,7 @@ static String mutationObserverNotificationString()
     if (layer != m_contentsLayer.get())
         return;
 
-    paintPDFContent(context, clipRect);
+    paintPDFContent(context, clipRect, PaintingBehavior::All, AllowsAsyncRendering::Yes);
 }
 
 PDFPageCoverage UnifiedPDFPlugin::pageCoverageForRect(const FloatRect& clipRect) const
@@ -747,7 +747,7 @@ static String mutationObserverNotificationString()
     return pageCoverage;
 }
 
-void UnifiedPDFPlugin::paintPDFContent(GraphicsContext& context, const FloatRect& clipRect, PaintingBehavior behavior)
+void UnifiedPDFPlugin::paintPDFContent(GraphicsContext& context, const FloatRect& clipRect, PaintingBehavior behavior, AllowsAsyncRendering allowsAsyncRendering)
 {
     if (m_size.isEmpty() || documentSize().isEmpty())
         return;
@@ -789,7 +789,9 @@ static String mutationObserverNotificationString()
             context.fillRect(pageDestinationRect, Color::white);
         }
 
-        RefPtr asyncRenderer = asyncRendererIfExists();
+        RefPtr<AsyncPDFRenderer> asyncRenderer;
+        if (allowsAsyncRendering == AllowsAsyncRendering::Yes)
+            asyncRenderer = asyncRendererIfExists();
         if (asyncRenderer) {
             auto pageBoundsInPaintingCoordinates = pageDestinationRect;
             pageBoundsInPaintingCoordinates.scale(documentScale);
@@ -2090,6 +2092,21 @@ static bool isContextMenuEvent(const WebMouseEvent& event)
     setNeedsRepaintInDocumentRect(repaintRequirements, annotationRect);
 }
 
+void UnifiedPDFPlugin::scrollToPointInContentsSpace(FloatPoint pointInContentsSpace)
+{
+    auto oldScrollType = currentScrollType();
+    setCurrentScrollType(ScrollType::Programmatic);
+    scrollToPositionWithoutAnimation(roundedIntPoint(pointInContentsSpace));
+    setCurrentScrollType(oldScrollType);
+}
+
+void UnifiedPDFPlugin::revealRectInContentsSpace(FloatRect rectInContentsSpace)
+{
+    auto pluginRectInContentsCoordinates = convertDown(CoordinateSpace::Plugin, CoordinateSpace::ScrolledContents, FloatRect { { 0, 0 }, size() });
+    auto rectToExpose = getRectToExposeForScrollIntoView(LayoutRect(pluginRectInContentsCoordinates), LayoutRect(rectInContentsSpace), ScrollAlignment::alignCenterIfNeeded, ScrollAlignment::alignCenterIfNeeded, std::nullopt);
+    scrollToPointInContentsSpace(rectToExpose.location());
+}
+
 void UnifiedPDFPlugin::scrollToPDFDestination(PDFDestination *destination)
 {
     auto unspecifiedValue = get_PDFKit_kPDFDestinationUnspecifiedValue();
@@ -2106,8 +2123,7 @@ static bool isContextMenuEvent(const WebMouseEvent& event)
 
 void UnifiedPDFPlugin::scrollToPointInPage(FloatPoint pointInPDFPageSpace, PDFDocumentLayout::PageIndex pageIndex)
 {
-    auto pointInContentsSpace = convertUp(CoordinateSpace::PDFPage, CoordinateSpace::ScrolledContents, pointInPDFPageSpace, pageIndex);
-    scrollToPositionWithoutAnimation(roundedIntPoint(pointInContentsSpace));
+    scrollToPointInContentsSpace(convertUp(CoordinateSpace::PDFPage, CoordinateSpace::ScrolledContents, pointInPDFPageSpace, pageIndex));
 }
 
 void UnifiedPDFPlugin::scrollToPage(PDFDocumentLayout::PageIndex pageIndex)
@@ -2116,7 +2132,8 @@ static bool isContextMenuEvent(const WebMouseEvent& event)
 
     auto pageBounds = m_documentLayout.layoutBoundsForPageAtIndex(pageIndex);
     auto boundsInScrolledContents = convertUp(CoordinateSpace::PDFDocumentLayout, CoordinateSpace::ScrolledContents, pageBounds);
-    scrollToPositionWithoutAnimation(boundsInScrolledContents.location());
+
+    scrollToPointInContentsSpace(boundsInScrolledContents.location());
 }
 
 void UnifiedPDFPlugin::scrollToFragmentIfNeeded()
@@ -2905,10 +2922,8 @@ static NSStringCompareOptions compareOptionsForFindOptions(WebCore::FindOptions
     if (!firstPageIndex)
         return false;
 
-    auto selectionScrolledContentsSpaceBounds = convertUp(CoordinateSpace::PDFPage, CoordinateSpace::ScrolledContents, FloatRect { [selection boundsForPage:firstPageForSelection.get()] }, *firstPageIndex);
-    auto currentScrollPositionY = scrollPosition().y();
-    if (selectionScrolledContentsSpaceBounds.y() < currentScrollPositionY || selectionScrolledContentsSpaceBounds.y() > currentScrollPositionY + size().height())
-        scrollToPositionWithoutAnimation(selectionScrolledContentsSpaceBounds.location());
+    auto selectionBoundsInContentsCoordinates = convertUp(CoordinateSpace::PDFPage, CoordinateSpace::ScrolledContents, FloatRect { [selection boundsForPage:firstPageForSelection.get()] }, *firstPageIndex);
+    revealRectInContentsSpace(selectionBoundsInContentsCoordinates);
 
     setCurrentSelection(WTFMove(selection));
     return true;
@@ -3286,9 +3301,7 @@ static NSStringCompareOptions compareOptionsForFindOptions(WebCore::FindOptions
         protectedActiveAnnotation()->attach(m_annotationContainer.get());
 
         auto newAnnotationRectInContentsCoordinates = convertUp(CoordinateSpace::PDFDocumentLayout, CoordinateSpace::ScrolledContents, documentRectForAnnotation(protectedActiveAnnotation()->annotation()));
-        auto pluginRectInContentsCoordinates = convertDown(CoordinateSpace::Plugin, CoordinateSpace::ScrolledContents, FloatRect { { 0, 0 }, size() });
-        auto rectToExpose = getRectToExposeForScrollIntoView(LayoutRect(pluginRectInContentsCoordinates), LayoutRect(newAnnotationRectInContentsCoordinates), ScrollAlignment::alignCenterIfNeeded, ScrollAlignment::alignCenterIfNeeded, std::nullopt);
-        scrollToPositionWithoutAnimation(rectToExpose.location());
+        revealRectInContentsSpace(newAnnotationRectInContentsCoordinates);
     } else
         m_activeAnnotation = nullptr;
 #endif