Service overlays stay fixed when <iframe> scrolls
authortimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 15 Aug 2014 18:47:51 +0000 (18:47 +0000)
committertimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 15 Aug 2014 18:47:51 +0000 (18:47 +0000)
https://bugs.webkit.org/show_bug.cgi?id=135959
<rdar://problem/17957716>

Reviewed by Enrica Casucci.

* WebProcess/WebPage/PageOverlay.cpp:
(WebKit::PageOverlay::didScrollFrame):
* WebProcess/WebPage/PageOverlay.h:
(WebKit::PageOverlay::Client::didScrollFrame):
* WebProcess/WebPage/PageOverlayController.cpp:
(WebKit::PageOverlayController::didScrollFrame):
Push didScrollFrame down to the overlays.

* WebProcess/WebPage/ServicesOverlayController.h:
* WebProcess/WebPage/mac/ServicesOverlayController.mm:
(WebKit::ServicesOverlayController::Highlight::createForSelection):
Hold on to the selection's Range so we can use it to compare Highlights later.

(WebKit::ServicesOverlayController::Highlight::Highlight):
(WebKit::ServicesOverlayController::Highlight::setDDHighlight):
Factor the code to set up and paint the highlight out, so that we can
set a new DDHighlightRef on a Highlight and the layer moves/reshapes/repaints.

(WebKit::ServicesOverlayController::buildPhoneNumberHighlights):
(WebKit::ServicesOverlayController::buildSelectionHighlight):
(WebKit::ServicesOverlayController::replaceHighlightsOfTypePreservingEquivalentHighlights):
Factor replaceHighlightsOfTypePreservingEquivalentHighlights out
so that we can use it for buildSelectionHighlight as well.

Steal the DDHighlightRef from the new Highlight when re-using an old one
so that the newly computed rects are used instead of the old ones.

(WebKit::ServicesOverlayController::highlightsAreEquivalent):
We will always have a Range now, so we can always check equivalence using it.

(WebKit::ServicesOverlayController::didScrollFrame):
Rebuild all highlights upon subframe scroll, as they might have moved.
We could optimize this in the future, but for now it's cheap enough
and rare enough that it doesn't matter.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@172636 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit2/ChangeLog
Source/WebKit2/WebProcess/WebPage/PageOverlay.cpp
Source/WebKit2/WebProcess/WebPage/PageOverlay.h
Source/WebKit2/WebProcess/WebPage/PageOverlayController.cpp
Source/WebKit2/WebProcess/WebPage/ServicesOverlayController.h
Source/WebKit2/WebProcess/WebPage/mac/ServicesOverlayController.mm

index 41ad214043ed7953be5c4615ec00bb4752a6bf27..9c727d6573dc58bb5f21df284c2bc3603310968e 100644 (file)
@@ -1,3 +1,46 @@
+2014-08-15  Tim Horton  <timothy_horton@apple.com>
+
+        Service overlays stay fixed when <iframe> scrolls
+        https://bugs.webkit.org/show_bug.cgi?id=135959
+        <rdar://problem/17957716>
+
+        Reviewed by Enrica Casucci.
+
+        * WebProcess/WebPage/PageOverlay.cpp:
+        (WebKit::PageOverlay::didScrollFrame):
+        * WebProcess/WebPage/PageOverlay.h:
+        (WebKit::PageOverlay::Client::didScrollFrame):
+        * WebProcess/WebPage/PageOverlayController.cpp:
+        (WebKit::PageOverlayController::didScrollFrame):
+        Push didScrollFrame down to the overlays.
+
+        * WebProcess/WebPage/ServicesOverlayController.h:
+        * WebProcess/WebPage/mac/ServicesOverlayController.mm:
+        (WebKit::ServicesOverlayController::Highlight::createForSelection):
+        Hold on to the selection's Range so we can use it to compare Highlights later.
+
+        (WebKit::ServicesOverlayController::Highlight::Highlight):
+        (WebKit::ServicesOverlayController::Highlight::setDDHighlight):
+        Factor the code to set up and paint the highlight out, so that we can
+        set a new DDHighlightRef on a Highlight and the layer moves/reshapes/repaints.
+
+        (WebKit::ServicesOverlayController::buildPhoneNumberHighlights):
+        (WebKit::ServicesOverlayController::buildSelectionHighlight):
+        (WebKit::ServicesOverlayController::replaceHighlightsOfTypePreservingEquivalentHighlights):
+        Factor replaceHighlightsOfTypePreservingEquivalentHighlights out
+        so that we can use it for buildSelectionHighlight as well.
+
+        Steal the DDHighlightRef from the new Highlight when re-using an old one
+        so that the newly computed rects are used instead of the old ones.
+
+        (WebKit::ServicesOverlayController::highlightsAreEquivalent):
+        We will always have a Range now, so we can always check equivalence using it.
+
+        (WebKit::ServicesOverlayController::didScrollFrame):
+        Rebuild all highlights upon subframe scroll, as they might have moved.
+        We could optimize this in the future, but for now it's cheap enough
+        and rare enough that it doesn't matter.
+
 2014-08-15  Tim Horton  <timothy_horton@apple.com>
 
         REGRESSION (WebKit2 Gestures): White flash when swiping back to cnn.com's homepage from an article
index 8212ed2e3ac544d3a4525673933a70fc1892befe..8d881b62e7089bde8e53d3a7853bafa0eaf04382 100644 (file)
@@ -170,6 +170,11 @@ bool PageOverlay::mouseEvent(const WebMouseEvent& mouseEvent)
     return m_client->mouseEvent(this, mouseEvent);
 }
 
+void PageOverlay::didScrollFrame(Frame* frame)
+{
+    m_client->didScrollFrame(this, frame);
+}
+
 WKTypeRef PageOverlay::copyAccessibilityAttributeValue(WKStringRef attribute, WKTypeRef parameter)
 {
     return m_client->copyAccessibilityAttributeValue(this, attribute, parameter);
index 8025edf87a880956cc925b6cf9676cd84b545dd9..3225f0d83fd2e427db2c0d90d0e9b285d481b2a9 100644 (file)
 #include <wtf/RunLoop.h>
 
 namespace WebCore {
+class Frame;
 class GraphicsContext;
 class GraphicsLayer;
 }
 
 namespace WebKit {
 
+class WebFrame;
 class WebMouseEvent;
 class WebPage;
 
@@ -55,6 +57,7 @@ public:
         virtual void didMoveToWebPage(PageOverlay*, WebPage*) = 0;
         virtual void drawRect(PageOverlay*, WebCore::GraphicsContext&, const WebCore::IntRect& dirtyRect) = 0;
         virtual bool mouseEvent(PageOverlay*, const WebMouseEvent&) = 0;
+        virtual void didScrollFrame(PageOverlay*, WebCore::Frame*) { }
 
         virtual WKTypeRef copyAccessibilityAttributeValue(PageOverlay*, WKStringRef /* attribute */, WKTypeRef /* parameter */) { return 0; }
         virtual WKArrayRef copyAccessibilityAttributeNames(PageOverlay*, bool /* parameterizedNames */) { return 0; }
@@ -74,6 +77,7 @@ public:
 
     void drawRect(WebCore::GraphicsContext&, const WebCore::IntRect& dirtyRect);
     bool mouseEvent(const WebMouseEvent&);
+    void didScrollFrame(WebCore::Frame*);
 
     WKTypeRef copyAccessibilityAttributeValue(WKStringRef attribute, WKTypeRef parameter);
     WKArrayRef copyAccessibilityAttributeNames(bool parameterizedNames);
index eb666b902a43754e100fb905edd777def4a38e93..86457c59622a27d26efb36d65e01c4b606f580e3 100644 (file)
@@ -218,6 +218,7 @@ void PageOverlayController::didScrollFrame(Frame* frame)
     for (auto& overlayAndLayer : m_overlayGraphicsLayers) {
         if (overlayAndLayer.key->overlayType() == PageOverlay::OverlayType::View || !frame->isMainFrame())
             overlayAndLayer.value->setNeedsDisplay();
+        overlayAndLayer.key->didScrollFrame(frame);
     }
 }
 
index 43b86ab0d1f6807be35f5decde94bc44a2679514..8e7235a92f37d864de8b5e6f0b9fe34f5a7ff5d3 100644 (file)
@@ -59,7 +59,7 @@ private:
     class Highlight : public RefCounted<Highlight>, private WebCore::GraphicsLayerClient {
         WTF_MAKE_NONCOPYABLE(Highlight);
     public:
-        static PassRefPtr<Highlight> createForSelection(ServicesOverlayController&, RetainPtr<DDHighlightRef>);
+        static PassRefPtr<Highlight> createForSelection(ServicesOverlayController&, RetainPtr<DDHighlightRef>, PassRefPtr<WebCore::Range>);
         static PassRefPtr<Highlight> createForTelephoneNumber(ServicesOverlayController&, RetainPtr<DDHighlightRef>, PassRefPtr<WebCore::Range>);
         ~Highlight();
 
@@ -78,6 +78,8 @@ private:
         void fadeIn();
         void fadeOut();
 
+        void setDDHighlight(DDHighlightRef);
+
     private:
         explicit Highlight(ServicesOverlayController&, Type, RetainPtr<DDHighlightRef>, PassRefPtr<WebCore::Range>);
 
@@ -102,12 +104,14 @@ private:
     virtual void didMoveToWebPage(PageOverlay*, WebPage*) override;
     virtual void drawRect(PageOverlay*, WebCore::GraphicsContext&, const WebCore::IntRect& dirtyRect) override;
     virtual bool mouseEvent(PageOverlay*, const WebMouseEvent&) override;
+    virtual void didScrollFrame(PageOverlay*, WebCore::Frame*) override;
 
     void createOverlayIfNeeded();
     void handleClick(const WebCore::IntPoint&, Highlight&);
 
     void drawHighlight(Highlight&, WebCore::GraphicsContext&);
 
+    void replaceHighlightsOfTypePreservingEquivalentHighlights(HashSet<RefPtr<Highlight>>&, Highlight::Type);
     void removeAllPotentialHighlightsOfType(Highlight::Type);
     void buildPhoneNumberHighlights();
     void buildSelectionHighlight();
index 257beef5278ead5b95661977681a3ea7ff833e32..e7dc1f37bc7e116026e47e00ced2eb3dee92d1e1 100644 (file)
@@ -71,9 +71,9 @@ using namespace WebCore;
 
 namespace WebKit {
 
-PassRefPtr<ServicesOverlayController::Highlight> ServicesOverlayController::Highlight::createForSelection(ServicesOverlayController& controller, RetainPtr<DDHighlightRef> ddHighlight)
+PassRefPtr<ServicesOverlayController::Highlight> ServicesOverlayController::Highlight::createForSelection(ServicesOverlayController& controller, RetainPtr<DDHighlightRef> ddHighlight, PassRefPtr<Range> range)
 {
-    return adoptRef(new Highlight(controller, Type::Selection, ddHighlight, nullptr));
+    return adoptRef(new Highlight(controller, Type::Selection, ddHighlight, range));
 }
 
 PassRefPtr<ServicesOverlayController::Highlight> ServicesOverlayController::Highlight::createForTelephoneNumber(ServicesOverlayController& controller, RetainPtr<DDHighlightRef> ddHighlight, PassRefPtr<Range> range)
@@ -82,22 +82,18 @@ PassRefPtr<ServicesOverlayController::Highlight> ServicesOverlayController::High
 }
 
 ServicesOverlayController::Highlight::Highlight(ServicesOverlayController& controller, Type type, RetainPtr<DDHighlightRef> ddHighlight, PassRefPtr<WebCore::Range> range)
-    : m_ddHighlight(ddHighlight)
-    , m_range(range)
+    : m_range(range)
     , m_type(type)
     , m_controller(&controller)
 {
-    ASSERT(m_ddHighlight);
-    ASSERT(type != Type::TelephoneNumber || m_range);
+    ASSERT(ddHighlight);
+    ASSERT(m_range);
 
     DrawingArea* drawingArea = controller.webPage().drawingArea();
     m_graphicsLayer = GraphicsLayer::create(drawingArea ? drawingArea->graphicsLayerFactory() : nullptr, *this);
     m_graphicsLayer->setDrawsContent(true);
-    m_graphicsLayer->setNeedsDisplay();
 
-    CGRect highlightBoundingRect = DDHighlightGetBoundingRect(ddHighlight.get());
-    m_graphicsLayer->setPosition(FloatPoint(highlightBoundingRect.origin));
-    m_graphicsLayer->setSize(FloatSize(highlightBoundingRect.size));
+    setDDHighlight(ddHighlight.get());
 
     // Set directly on the PlatformCALayer so that when we leave the 'from' value implicit
     // in our animations, we get the right initial value regardless of flush timing.
@@ -112,6 +108,23 @@ ServicesOverlayController::Highlight::~Highlight()
         m_controller->willDestroyHighlight(this);
 }
 
+void ServicesOverlayController::Highlight::setDDHighlight(DDHighlightRef highlight)
+{
+    if (!m_controller)
+        return;
+
+    m_ddHighlight = highlight;
+
+    if (!m_ddHighlight)
+        return;
+
+    CGRect highlightBoundingRect = DDHighlightGetBoundingRect(m_ddHighlight.get());
+    m_graphicsLayer->setPosition(FloatPoint(highlightBoundingRect.origin));
+    m_graphicsLayer->setSize(FloatSize(highlightBoundingRect.size));
+
+    m_graphicsLayer->setNeedsDisplay();
+}
+
 void ServicesOverlayController::Highlight::invalidate()
 {
     layer()->removeFromParent();
@@ -486,34 +499,14 @@ void ServicesOverlayController::buildPhoneNumberHighlights()
         }
     }
 
-    // If any old Highlights are equivalent (by Range) to a new Highlight, reuse the old
-    // one so that any metadata is retained.
-    HashSet<RefPtr<Highlight>> reusedPotentialHighlights;
-
-    for (auto& oldHighlight : m_potentialHighlights) {
-        if (oldHighlight->type() != Highlight::Type::TelephoneNumber)
-            continue;
-
-        for (auto& newHighlight : newPotentialHighlights) {
-            if (highlightsAreEquivalent(oldHighlight.get(), newHighlight.get())) {
-                reusedPotentialHighlights.add(oldHighlight);
-                newPotentialHighlights.remove(newHighlight);
-                break;
-            }
-        }
-    }
-
-    removeAllPotentialHighlightsOfType(Highlight::Type::TelephoneNumber);
-
-    m_potentialHighlights.add(newPotentialHighlights.begin(), newPotentialHighlights.end());
-    m_potentialHighlights.add(reusedPotentialHighlights.begin(), reusedPotentialHighlights.end());
+    replaceHighlightsOfTypePreservingEquivalentHighlights(newPotentialHighlights, Highlight::Type::TelephoneNumber);
 
     didRebuildPotentialHighlights();
 }
 
 void ServicesOverlayController::buildSelectionHighlight()
 {
-    removeAllPotentialHighlightsOfType(Highlight::Type::Selection);
+    HashSet<RefPtr<Highlight>> newPotentialHighlights;
 
     Vector<CGRect> cgRects;
     cgRects.reserveCapacity(m_currentSelectionRects.size());
@@ -534,13 +527,43 @@ void ServicesOverlayController::buildSelectionHighlight()
             CGRect visibleRect = m_webPage.corePage()->mainFrame().view()->visibleContentRect();
             RetainPtr<DDHighlightRef> ddHighlight = adoptCF(DDHighlightCreateWithRectsInVisibleRectWithStyleAndDirection(nullptr, cgRects.begin(), cgRects.size(), visibleRect, DDHighlightNoOutlineWithArrow, YES, NSWritingDirectionNatural, NO, YES));
             
-            m_potentialHighlights.add(Highlight::createForSelection(*this, ddHighlight));
+            newPotentialHighlights.add(Highlight::createForSelection(*this, ddHighlight, selectionRange));
         }
     }
 
+    replaceHighlightsOfTypePreservingEquivalentHighlights(newPotentialHighlights, Highlight::Type::Selection);
+
     didRebuildPotentialHighlights();
 }
 
+void ServicesOverlayController::replaceHighlightsOfTypePreservingEquivalentHighlights(HashSet<RefPtr<Highlight>>& newPotentialHighlights, Highlight::Type type)
+{
+    // If any old Highlights are equivalent (by Range) to a new Highlight, reuse the old
+    // one so that any metadata is retained.
+    HashSet<RefPtr<Highlight>> reusedPotentialHighlights;
+
+    for (auto& oldHighlight : m_potentialHighlights) {
+        if (oldHighlight->type() != type)
+            continue;
+
+        for (auto& newHighlight : newPotentialHighlights) {
+            if (highlightsAreEquivalent(oldHighlight.get(), newHighlight.get())) {
+                oldHighlight->setDDHighlight(newHighlight->ddHighlight());
+
+                reusedPotentialHighlights.add(oldHighlight);
+                newPotentialHighlights.remove(newHighlight);
+
+                break;
+            }
+        }
+    }
+
+    removeAllPotentialHighlightsOfType(type);
+
+    m_potentialHighlights.add(newPotentialHighlights.begin(), newPotentialHighlights.end());
+    m_potentialHighlights.add(reusedPotentialHighlights.begin(), reusedPotentialHighlights.end());
+}
+
 bool ServicesOverlayController::hasRelevantSelectionServices()
 {
     return (m_isTextOnly && WebProcess::shared().hasSelectionServices()) || WebProcess::shared().hasRichContentServices();
@@ -590,7 +613,7 @@ bool ServicesOverlayController::highlightsAreEquivalent(const Highlight* a, cons
     if (!a || !b)
         return false;
 
-    if (a->type() == Highlight::Type::TelephoneNumber && b->type() == Highlight::Type::TelephoneNumber && areRangesEqual(a->range(), b->range()))
+    if (areRangesEqual(a->range(), b->range()))
         return true;
 
     return false;
@@ -740,6 +763,18 @@ bool ServicesOverlayController::mouseEvent(PageOverlay*, const WebMouseEvent& ev
     return false;
 }
 
+void ServicesOverlayController::didScrollFrame(PageOverlay*, Frame* frame)
+{
+    if (frame->isMainFrame())
+        return;
+
+    buildPhoneNumberHighlights();
+    buildSelectionHighlight();
+
+    bool mouseIsOverActiveHighlightButton;
+    determineActiveHighlight(mouseIsOverActiveHighlightButton);
+}
+
 void ServicesOverlayController::handleClick(const IntPoint& clickPoint, Highlight& highlight)
 {
     FrameView* frameView = m_webPage.mainFrameView();