[UnifiedPDF] WebCore::DataDetectorHighlight should be decoupled from …

…the page

https://bugs.webkit.org/show_bug.cgi?id=270883
rdar://124490748

Reviewed by Aditya Keerthi.

Currently, DataDetectorHighlight instances require a WebCore::Page
object at construction time. This object is used for some IPC needs
such as for scheduling rendering updates and for creating graphics layers,
among other things. While having a Page object around is convenient, it
does not fit well when trying to integrate the DataDetectorHighlight
data type in the UnifiedPDF Data Detection code path.

This patch rips out the dependency of DataDetectorHighlight on
the page object, instead delegating its duties to the abstract interface
DataDetectorHighlightClient, which can be satisifed by downstream
overlay controllers, or even PDF plugins. We make this change in
anticipation of a forthcoming patch where we construct and display
DataDetectorHighlight instances in a UnifiedPDFPlugin-specific document
overlay controller. Furthermore, we also adopt the changes made to the
DataDetectorHighlightClient interface in two notable overlay controllers
- ImageOverlayController and ServicesOverlayController.

Furthermore, we make a few miscellaneous unified sources build fixes and
annotate a couple of `enum class` declarations with the tightest
possible data type representation.

* Source/WebCore/page/ImageOverlayController.h:
* Source/WebCore/page/PageOverlay.h:
* Source/WebCore/page/mac/ImageOverlayControllerMac.mm:
(WebCore::ImageOverlayController::updateDataDetectorHighlights):
(WebCore::ImageOverlayController::scheduleRenderingUpdate):
(WebCore::ImageOverlayController::deviceScaleFactor const):
(WebCore::ImageOverlayController::createGraphicsLayer):
* Source/WebCore/page/mac/ServicesOverlayController.h:
(WebCore::ServicesOverlayController::protectedPage const):
* Source/WebCore/page/mac/ServicesOverlayController.mm:
(WebCore::ServicesOverlayController::buildPhoneNumberHighlights):
(WebCore::ServicesOverlayController::buildSelectionHighlight):
(WebCore::ServicesOverlayController::scheduleRenderingUpdate):
(WebCore::ServicesOverlayController::deviceScaleFactor const):
(WebCore::ServicesOverlayController::createGraphicsLayer):
* Source/WebCore/platform/mac/DataDetectorHighlight.h:
(WebCore::DataDetectorHighlight::range const): Deleted.
* Source/WebCore/platform/mac/DataDetectorHighlight.mm:
(WebCore::DataDetectorHighlight::createForSelection):
(WebCore::DataDetectorHighlight::createForTelephoneNumber):
(WebCore::DataDetectorHighlight::createForImageOverlay):
(WebCore::DataDetectorHighlight::DataDetectorHighlight):
(WebCore::DataDetectorHighlight::invalidate):
(WebCore::DataDetectorHighlight::notifyFlushRequired):
(WebCore::DataDetectorHighlight::deviceScaleFactor const):
(WebCore::DataDetectorHighlight::isRangeSupportingType const):
(WebCore::DataDetectorHighlight::range const):
(WebCore::areEquivalent):
* Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/UnifiedPDFPlugin.h:
* Source/WebKit/WebProcess/WebPage/Cocoa/UnifiedTextReplacementController.mm:
(WebKit::UnifiedTextReplacementController::textReplacementSessionDidReceiveTextWithReplacementRange):
* Source/WebKit/WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemoteCustom.h:

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

Source/WebCore/page/ImageOverlayController.h

@@ -28,6 +28,7 @@
 #include "Color.h"
 #include "LayoutRect.h"
 #include "PageOverlay.h"
+#include <wtf/OptionSet.h>
 #include <wtf/Vector.h>
 #include <wtf/WeakPtr.h>
 
@@ -40,13 +41,16 @@ namespace WebCore {
 class Document;
 class Element;
 class GraphicsContext;
+class GraphicsLayer;
+class GraphicsLayerClient;
 class HTMLElement;
 class IntRect;
 class FloatQuad;
 class LocalFrame;
 class Page;
 class RenderElement;
 class WeakPtrImplWithEventTargetData;
+enum class RenderingUpdateStep : uint32_t;
 struct GapRects;
 
 class ImageOverlayController final : private PageOverlay::Client
@@ -85,7 +89,13 @@ class ImageOverlayController final : private PageOverlay::Client
     void clearDataDetectorHighlights();
     bool handleDataDetectorAction(const HTMLElement&, const IntPoint&);
 
+    // DataDetectorHighlightClient
+#if ENABLE(DATA_DETECTION)
     DataDetectorHighlight* activeHighlight() const final { return m_activeDataDetectorHighlight.get(); }
+    void scheduleRenderingUpdate(OptionSet<RenderingUpdateStep>) final;
+    float deviceScaleFactor() const final;
+    RefPtr<GraphicsLayer> createGraphicsLayer(GraphicsLayerClient&) final;
+#endif
 #endif
 
     void platformUpdateElementUnderMouse(LocalFrame&, Element* elementUnderMouse);

Source/WebCore/page/PageOverlay.h

@@ -63,7 +63,7 @@ class PageOverlay final : public RefCounted<PageOverlay>, public CanMakeWeakPtr<
         virtual Vector<String> copyAccessibilityAttributeNames(PageOverlay&, bool /* parameterizedNames */)  { return { }; }
     };
 
-    enum class OverlayType {
+    enum class OverlayType : bool {
         View, // Fixed to the view size; does not scale or scroll with the document, repaints on scroll.
         Document, // Scales and scrolls with the document.
     };
@@ -102,7 +102,7 @@ class PageOverlay final : public RefCounted<PageOverlay>, public CanMakeWeakPtr<
 
     Client& client() const { return m_client; }
 
-    enum class FadeMode { DoNotFade, Fade };
+    enum class FadeMode : bool { DoNotFade, Fade };
 
     OverlayType overlayType() { return m_overlayType; }
     AlwaysTileOverlayLayer alwaysTileOverlayLayer() { return m_alwaysTileOverlayLayer; }

Source/WebCore/page/mac/ImageOverlayControllerMac.mm

@@ -32,6 +32,8 @@
 #import "DataDetectionResultsStorage.h"
 #import "DataDetectorElementInfo.h"
 #import "ElementInlines.h"
+#import "GraphicsLayer.h"
+#import "GraphicsLayerClient.h"
 #import "HTMLElement.h"
 #import "HTMLNames.h"
 #import "ImageOverlay.h"
@@ -44,6 +46,8 @@
 #import "TypedElementDescendantIteratorInlines.h"
 #import <QuartzCore/QuartzCore.h>
 #import <wtf/HashSet.h>
+#import <wtf/OptionSet.h>
+#import <wtf/RefPtr.h>
 #import <wtf/text/StringToIntegerConversion.h>
 #import <pal/mac/DataDetectorsSoftLink.h>
 
@@ -87,7 +91,7 @@
 
         // FIXME: We should teach DataDetectorHighlight to render quads instead of always falling back to axis-aligned bounding rects.
         auto highlight = adoptCF(PAL::softLink_DataDetectors_DDHighlightCreateWithRectsInVisibleRectWithStyleScaleAndDirection(nullptr, &elementBounds, 1, mainFrameView->visibleContentRect(), static_cast<DDHighlightStyle>(DDHighlightStyleBubbleStandard) | static_cast<DDHighlightStyle>(DDHighlightStyleStandardIconArrow), YES, NSWritingDirectionNatural, NO, YES, 0));
-        return ContainerAndHighlight { element, DataDetectorHighlight::createForImageOverlay(*protectedPage(), *this, WTFMove(highlight), *makeRangeSelectingNode(element.get())) };
+        return ContainerAndHighlight { element, DataDetectorHighlight::createForImageOverlay(*this, WTFMove(highlight), *makeRangeSelectingNode(element.get())) };
     });
 }
 
@@ -234,6 +238,36 @@
     installPageOverlayIfNeeded();
 }
 
+#pragma mark - DataDetectorHighlightClient
+
+#if ENABLE(DATA_DETECTION)
+
+void ImageOverlayController::scheduleRenderingUpdate(OptionSet<RenderingUpdateStep> requestedSteps)
+{
+    if (!m_page)
+        return;
+
+    protectedPage()->scheduleRenderingUpdate(requestedSteps);
+}
+
+float ImageOverlayController::deviceScaleFactor() const
+{
+    if (!m_page)
+        return 1;
+
+    return protectedPage()->deviceScaleFactor();
+}
+
+RefPtr<GraphicsLayer> ImageOverlayController::createGraphicsLayer(GraphicsLayerClient& client)
+{
+    if (!m_page)
+        return nullptr;
+
+    return GraphicsLayer::create(protectedPage()->chrome().client().graphicsLayerFactory(), client);
+}
+
+#endif
+
 } // namespace WebCore
 
 #endif // PLATFORM(MAC)

Source/WebCore/page/mac/ServicesOverlayController.h

@@ -28,6 +28,7 @@
 #if (ENABLE(SERVICE_CONTROLS) || ENABLE(TELEPHONE_NUMBER_DETECTION)) && PLATFORM(MAC)
 
 #include "DataDetectorHighlight.h"
+#include "GraphicsLayer.h"
 #include "GraphicsLayerClient.h"
 #include "PageOverlay.h"
 #include "Timer.h"
@@ -40,6 +41,8 @@ namespace WebCore {
 class LayoutRect;
 class Page;
 
+enum class RenderingUpdateStep : uint32_t;
+
 struct GapRects;
 
 class ServicesOverlayController : private DataDetectorHighlightClient, private PageOverlay::Client {
@@ -74,7 +77,14 @@ class ServicesOverlayController : private DataDetectorHighlightClient, private P
 
     void determineActiveHighlight(bool& mouseIsOverButton);
     void clearActiveHighlight();
+
+#if ENABLE(DATA_DETECTION)
+    // DataDetectorHighlightClient
     DataDetectorHighlight* activeHighlight() const final { return m_activeHighlight.get(); }
+    void scheduleRenderingUpdate(OptionSet<RenderingUpdateStep>) final;
+    float deviceScaleFactor() const final;
+    RefPtr<GraphicsLayer> createGraphicsLayer(GraphicsLayerClient&) final;
+#endif
 
     DataDetectorHighlight* findTelephoneNumberHighlightContainingSelectionHighlight(DataDetectorHighlight&);
 
@@ -87,6 +97,7 @@ class ServicesOverlayController : private DataDetectorHighlightClient, private P
     Vector<SimpleRange> telephoneNumberRangesForFocusedFrame();
 
     Page& page() const { return m_page; }
+    Ref<Page> protectedPage() const { return m_page.get(); }
 
     SingleThreadWeakRef<Page> m_page;
     WeakPtr<PageOverlay> m_servicesOverlay;

Source/WebCore/page/mac/ServicesOverlayController.mm

@@ -379,7 +379,7 @@ static void compactRectsWithGapRects(Vector<LayoutRect>& rects, const Vector<Gap
 
         CGRect cgRect = rect;
         auto ddHighlight = adoptCF(PAL::softLink_DataDetectors_DDHighlightCreateWithRectsInVisibleRectWithStyleScaleAndDirection(nullptr, &cgRect, 1, mainFrameView.visibleContentRect(), static_cast<DDHighlightStyle>(DDHighlightStyleBubbleStandard) | static_cast<DDHighlightStyle>(DDHighlightStyleStandardIconArrow), YES, NSWritingDirectionNatural, NO, YES, 0));
-        auto highlight = DataDetectorHighlight::createForTelephoneNumber(page, *this, WTFMove(ddHighlight), WTFMove(range));
+        auto highlight = DataDetectorHighlight::createForTelephoneNumber(*this, WTFMove(ddHighlight), WTFMove(range));
         m_highlights.add(highlight.get());
         newPotentialHighlights.add(WTFMove(highlight));
     }
@@ -418,7 +418,7 @@ static void compactRectsWithGapRects(Vector<LayoutRect>& rects, const Vector<Gap
         if (!cgRects.isEmpty()) {
             CGRect visibleRect = mainFrameView->visibleContentRect();
             auto ddHighlight = adoptCF(PAL::softLink_DataDetectors_DDHighlightCreateWithRectsInVisibleRectWithStyleScaleAndDirection(nullptr, cgRects.begin(), cgRects.size(), visibleRect, static_cast<DDHighlightStyle>(DDHighlightStyleBubbleNone) | static_cast<DDHighlightStyle>(DDHighlightStyleStandardIconArrow) | static_cast<DDHighlightStyle>(DDHighlightStyleButtonShowAlways), YES, NSWritingDirectionNatural, NO, YES, 0));
-            auto highlight = DataDetectorHighlight::createForSelection(page, *this, WTFMove(ddHighlight), WTFMove(*selectionRange));
+            auto highlight = DataDetectorHighlight::createForSelection(*this, WTFMove(ddHighlight), WTFMove(*selectionRange));
             m_highlights.add(highlight.get());
             newPotentialHighlights.add(WTFMove(highlight));
         }
@@ -658,6 +658,27 @@ static void compactRectsWithGapRects(Vector<LayoutRect>& rects, const Vector<Gap
         page->chrome().client().handleTelephoneNumberClick(plainText(highlight.range()), windowPoint, frameView->contentsToWindow(focusedOrMainFrame->editor().firstRectForRange(highlight.range())));
 }
 
+#pragma mark - DataDetectorHighlightClient
+
+#if ENABLE(DATA_DETECTION)
+
+void ServicesOverlayController::scheduleRenderingUpdate(OptionSet<RenderingUpdateStep> requestedSteps)
+{
+    protectedPage()->scheduleRenderingUpdate(requestedSteps);
+}
+
+float ServicesOverlayController::deviceScaleFactor() const
+{
+    return protectedPage()->deviceScaleFactor();
+}
+
+RefPtr<GraphicsLayer> ServicesOverlayController::createGraphicsLayer(GraphicsLayerClient& client)
+{
+    return GraphicsLayer::create(protectedPage()->chrome().client().graphicsLayerFactory(), client);
+}
+
+#endif
+
 } // namespace WebKit
 
 #endif // (ENABLE(SERVICE_CONTROLS) || ENABLE(TELEPHONE_NUMBER_DETECTION)) && PLATFORM(MAC)

Source/WebCore/platform/mac/DataDetectorHighlight.h

@@ -36,36 +36,39 @@
 #import <wtf/RetainPtr.h>
 #import <wtf/WeakPtr.h>
 
-typedef struct __DDHighlight *DDHighlightRef;
+using DDHighlightRef = struct __DDHighlight*;
 
 namespace WebCore {
 
 class DataDetectorHighlight;
 class FloatRect;
 class GraphicsContext;
 class GraphicsLayer;
-class Page;
+
+enum class RenderingUpdateStep : uint32_t;
 
 class DataDetectorHighlightClient : public CanMakeWeakPtr<DataDetectorHighlightClient> {
 public:
-    virtual ~DataDetectorHighlightClient() = default;
-
-    virtual DataDetectorHighlight* activeHighlight() const = 0;
+    WEBCORE_EXPORT virtual ~DataDetectorHighlightClient() = default;
+    WEBCORE_EXPORT virtual DataDetectorHighlight* activeHighlight() const = 0;
+    WEBCORE_EXPORT virtual void scheduleRenderingUpdate(OptionSet<RenderingUpdateStep>) = 0;
+    WEBCORE_EXPORT virtual float deviceScaleFactor() const = 0;
+    WEBCORE_EXPORT virtual RefPtr<GraphicsLayer> createGraphicsLayer(GraphicsLayerClient&) = 0;
 };
 
 class DataDetectorHighlight : public RefCounted<DataDetectorHighlight>, private GraphicsLayerClient, public CanMakeWeakPtr<DataDetectorHighlight> {
     WTF_MAKE_NONCOPYABLE(DataDetectorHighlight);
 public:
-    static Ref<DataDetectorHighlight> createForSelection(Page&, DataDetectorHighlightClient&, RetainPtr<DDHighlightRef>&&, SimpleRange&&);
-    static Ref<DataDetectorHighlight> createForTelephoneNumber(Page&, DataDetectorHighlightClient&, RetainPtr<DDHighlightRef>&&, SimpleRange&&);
-    static Ref<DataDetectorHighlight> createForImageOverlay(Page&, DataDetectorHighlightClient&, RetainPtr<DDHighlightRef>&&, SimpleRange&&);
+    static Ref<DataDetectorHighlight> createForSelection(DataDetectorHighlightClient&, RetainPtr<DDHighlightRef>&&, SimpleRange&&);
+    static Ref<DataDetectorHighlight> createForTelephoneNumber(DataDetectorHighlightClient&, RetainPtr<DDHighlightRef>&&, SimpleRange&&);
+    static Ref<DataDetectorHighlight> createForImageOverlay(DataDetectorHighlightClient&, RetainPtr<DDHighlightRef>&&, SimpleRange&&);
 
     ~DataDetectorHighlight();
 
     void invalidate();
 
     DDHighlightRef highlight() const { return m_highlight.get(); }
-    const SimpleRange& range() const { return m_range; }
+    const SimpleRange& range() const;
     GraphicsLayer& layer() const { return m_graphicsLayer.get(); }
 
     enum class Type : uint8_t {
@@ -76,14 +79,15 @@ class DataDetectorHighlight : public RefCounted<DataDetectorHighlight>, private
     };
 
     Type type() const { return m_type; }
+    bool isRangeSupportingType() const;
 
     void fadeIn();
     void fadeOut();
 
     void setHighlight(DDHighlightRef);
 
 private:
-    DataDetectorHighlight(Page&, DataDetectorHighlightClient&, Type, RetainPtr<DDHighlightRef>&&, SimpleRange&&);
+    DataDetectorHighlight(DataDetectorHighlightClient&, Type, RetainPtr<DDHighlightRef>&&, std::optional<SimpleRange>&&);
 
     // GraphicsLayerClient
     void notifyFlushRequired(const GraphicsLayer*) override;
@@ -95,9 +99,8 @@ class DataDetectorHighlight : public RefCounted<DataDetectorHighlight>, private
     void didFinishFadeOutAnimation();
 
     WeakPtr<DataDetectorHighlightClient> m_client;
-    SingleThreadWeakPtr<Page> m_page;
     RetainPtr<DDHighlightRef> m_highlight;
-    SimpleRange m_range;
+    std::optional<SimpleRange> m_range;
     Ref<GraphicsLayer> m_graphicsLayer;
     Type m_type { Type::None };
 

Source/WebCore/platform/mac/DataDetectorHighlight.mm

@@ -35,7 +35,6 @@
 #import "GraphicsLayer.h"
 #import "GraphicsLayerFactory.h"
 #import "ImageBuffer.h"
-#import "Page.h"
 #import <wtf/Seconds.h>
 #import <pal/mac/DataDetectorsSoftLink.h>
 
@@ -44,30 +43,30 @@
 constexpr Seconds highlightFadeAnimationDuration = 300_ms;
 constexpr double highlightFadeAnimationFrameRate = 30;
 
-Ref<DataDetectorHighlight> DataDetectorHighlight::createForSelection(Page& page, DataDetectorHighlightClient& client, RetainPtr<DDHighlightRef>&& ddHighlight, SimpleRange&& range)
+Ref<DataDetectorHighlight> DataDetectorHighlight::createForSelection(DataDetectorHighlightClient& client, RetainPtr<DDHighlightRef>&& ddHighlight, SimpleRange&& range)
 {
-    return adoptRef(*new DataDetectorHighlight(page, client, DataDetectorHighlight::Type::Selection, WTFMove(ddHighlight), WTFMove(range)));
+    return adoptRef(*new DataDetectorHighlight(client, DataDetectorHighlight::Type::Selection, WTFMove(ddHighlight), { WTFMove(range) }));
 }
 
-Ref<DataDetectorHighlight> DataDetectorHighlight::createForTelephoneNumber(Page& page, DataDetectorHighlightClient& client, RetainPtr<DDHighlightRef>&& ddHighlight, SimpleRange&& range)
+Ref<DataDetectorHighlight> DataDetectorHighlight::createForTelephoneNumber(DataDetectorHighlightClient& client, RetainPtr<DDHighlightRef>&& ddHighlight, SimpleRange&& range)
 {
-    return adoptRef(*new DataDetectorHighlight(page, client, DataDetectorHighlight::Type::TelephoneNumber, WTFMove(ddHighlight), WTFMove(range)));
+    return adoptRef(*new DataDetectorHighlight(client, DataDetectorHighlight::Type::TelephoneNumber, WTFMove(ddHighlight), { WTFMove(range) }));
 }
 
-Ref<DataDetectorHighlight> DataDetectorHighlight::createForImageOverlay(Page& page, DataDetectorHighlightClient& client, RetainPtr<DDHighlightRef>&& ddHighlight, SimpleRange&& range)
+Ref<DataDetectorHighlight> DataDetectorHighlight::createForImageOverlay(DataDetectorHighlightClient& client, RetainPtr<DDHighlightRef>&& ddHighlight, SimpleRange&& range)
 {
-    return adoptRef(*new DataDetectorHighlight(page, client, DataDetectorHighlight::Type::ImageOverlay, WTFMove(ddHighlight), WTFMove(range)));
+    return adoptRef(*new DataDetectorHighlight(client, DataDetectorHighlight::Type::ImageOverlay, WTFMove(ddHighlight), { WTFMove(range) }));
 }
 
-DataDetectorHighlight::DataDetectorHighlight(Page& page, DataDetectorHighlightClient& client, Type type, RetainPtr<DDHighlightRef>&& ddHighlight, SimpleRange&& range)
+DataDetectorHighlight::DataDetectorHighlight(DataDetectorHighlightClient& client, Type type, RetainPtr<DDHighlightRef>&& ddHighlight, std::optional<SimpleRange>&& range)
     : m_client(client)
-    , m_page(page)
     , m_range(WTFMove(range))
-    , m_graphicsLayer(GraphicsLayer::create(page.chrome().client().graphicsLayerFactory(), *this))
+    , m_graphicsLayer(client.createGraphicsLayer(*this).releaseNonNull())
     , m_type(type)
     , m_fadeAnimationTimer(*this, &DataDetectorHighlight::fadeAnimationTimerFired)
 {
     ASSERT(ddHighlight);
+    ASSERT(isRangeSupportingType() == m_range.has_value());
 
     m_graphicsLayer->setDrawsContent(true);
 
@@ -106,15 +105,14 @@
     m_fadeAnimationTimer.stop();
     layer().removeFromParent();
     m_client = nullptr;
-    m_page = nullptr;
 }
 
 void DataDetectorHighlight::notifyFlushRequired(const GraphicsLayer*)
 {
-    if (!m_page)
+    if (!m_client)
         return;
 
-    m_page->scheduleRenderingUpdate(RenderingUpdateStep::LayerFlush);
+    m_client->scheduleRenderingUpdate(RenderingUpdateStep::LayerFlush);
 }
 
 void DataDetectorHighlight::paintContents(const GraphicsLayer*, GraphicsContext& graphicsContext, const FloatRect&, OptionSet<GraphicsLayerPaintBehavior>)
@@ -145,10 +143,28 @@
 
 float DataDetectorHighlight::deviceScaleFactor() const
 {
-    if (!m_page)
+    if (!m_client)
         return 1;
 
-    return m_page->deviceScaleFactor();
+    return m_client->deviceScaleFactor();
+}
+
+bool DataDetectorHighlight::isRangeSupportingType() const
+{
+    static constexpr OptionSet rangeSupportingHighlightTypes {
+        DataDetectorHighlight::Type::TelephoneNumber,
+        DataDetectorHighlight::Type::Selection,
+        DataDetectorHighlight::Type::ImageOverlay,
+    };
+
+    return rangeSupportingHighlightTypes.contains(m_type);
+}
+
+const SimpleRange& DataDetectorHighlight::range() const
+{
+    ASSERT(isRangeSupportingType());
+
+    return *m_range;
 }
 
 void DataDetectorHighlight::fadeAnimationTimerFired()
@@ -213,7 +229,10 @@ bool areEquivalent(const DataDetectorHighlight* a, const DataDetectorHighlight*
     if (!a || !b)
         return false;
 
-    return a->type() == b->type() && a->range() == b->range();
+    if (a->type() != b->type())
+        return false;
+
+    return !a->isRangeSupportingType() || a->range() == b->range();
 }
 
 } // namespace WebCore