Title: [172636] trunk/Source/WebKit2
Revision
172636
Author
timothy_hor...@apple.com
Date
2014-08-15 11:47:51 -0700 (Fri, 15 Aug 2014)

Log Message

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.

Modified Paths

Diff

Modified: trunk/Source/WebKit2/ChangeLog (172635 => 172636)


--- trunk/Source/WebKit2/ChangeLog	2014-08-15 18:42:24 UTC (rev 172635)
+++ trunk/Source/WebKit2/ChangeLog	2014-08-15 18:47:51 UTC (rev 172636)
@@ -1,5 +1,48 @@
 2014-08-15  Tim Horton  <timothy_hor...@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_hor...@apple.com>
+
         REGRESSION (WebKit2 Gestures): White flash when swiping back to cnn.com's homepage from an article
         https://bugs.webkit.org/show_bug.cgi?id=135951
         <rdar://problem/18006149>

Modified: trunk/Source/WebKit2/WebProcess/WebPage/PageOverlay.cpp (172635 => 172636)


--- trunk/Source/WebKit2/WebProcess/WebPage/PageOverlay.cpp	2014-08-15 18:42:24 UTC (rev 172635)
+++ trunk/Source/WebKit2/WebProcess/WebPage/PageOverlay.cpp	2014-08-15 18:47:51 UTC (rev 172636)
@@ -170,6 +170,11 @@
     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);

Modified: trunk/Source/WebKit2/WebProcess/WebPage/PageOverlay.h (172635 => 172636)


--- trunk/Source/WebKit2/WebProcess/WebPage/PageOverlay.h	2014-08-15 18:42:24 UTC (rev 172635)
+++ trunk/Source/WebKit2/WebProcess/WebPage/PageOverlay.h	2014-08-15 18:47:51 UTC (rev 172636)
@@ -34,12 +34,14 @@
 #include <wtf/RunLoop.h>
 
 namespace WebCore {
+class Frame;
 class GraphicsContext;
 class GraphicsLayer;
 }
 
 namespace WebKit {
 
+class WebFrame;
 class WebMouseEvent;
 class WebPage;
 
@@ -55,6 +57,7 @@
         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 @@
 
     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);

Modified: trunk/Source/WebKit2/WebProcess/WebPage/PageOverlayController.cpp (172635 => 172636)


--- trunk/Source/WebKit2/WebProcess/WebPage/PageOverlayController.cpp	2014-08-15 18:42:24 UTC (rev 172635)
+++ trunk/Source/WebKit2/WebProcess/WebPage/PageOverlayController.cpp	2014-08-15 18:47:51 UTC (rev 172636)
@@ -218,6 +218,7 @@
     for (auto& overlayAndLayer : m_overlayGraphicsLayers) {
         if (overlayAndLayer.key->overlayType() == PageOverlay::OverlayType::View || !frame->isMainFrame())
             overlayAndLayer.value->setNeedsDisplay();
+        overlayAndLayer.key->didScrollFrame(frame);
     }
 }
 

Modified: trunk/Source/WebKit2/WebProcess/WebPage/ServicesOverlayController.h (172635 => 172636)


--- trunk/Source/WebKit2/WebProcess/WebPage/ServicesOverlayController.h	2014-08-15 18:42:24 UTC (rev 172635)
+++ trunk/Source/WebKit2/WebProcess/WebPage/ServicesOverlayController.h	2014-08-15 18:47:51 UTC (rev 172636)
@@ -59,7 +59,7 @@
     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 @@
         void fadeIn();
         void fadeOut();
 
+        void setDDHighlight(DDHighlightRef);
+
     private:
         explicit Highlight(ServicesOverlayController&, Type, RetainPtr<DDHighlightRef>, PassRefPtr<WebCore::Range>);
 
@@ -102,12 +104,14 @@
     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();

Modified: trunk/Source/WebKit2/WebProcess/WebPage/mac/ServicesOverlayController.mm (172635 => 172636)


--- trunk/Source/WebKit2/WebProcess/WebPage/mac/ServicesOverlayController.mm	2014-08-15 18:42:24 UTC (rev 172635)
+++ trunk/Source/WebKit2/WebProcess/WebPage/mac/ServicesOverlayController.mm	2014-08-15 18:47:51 UTC (rev 172636)
@@ -71,9 +71,9 @@
 
 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 @@
 }
 
 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 @@
         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 @@
         }
     }
 
-    // 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;
+    replaceHighlightsOfTypePreservingEquivalentHighlights(newPotentialHighlights, Highlight::Type::TelephoneNumber);
 
-    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());
-
     didRebuildPotentialHighlights();
 }
 
 void ServicesOverlayController::buildSelectionHighlight()
 {
-    removeAllPotentialHighlightsOfType(Highlight::Type::Selection);
+    HashSet<RefPtr<Highlight>> newPotentialHighlights;
 
     Vector<CGRect> cgRects;
     cgRects.reserveCapacity(m_currentSelectionRects.size());
@@ -534,13 +527,43 @@
             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 @@
     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 @@
     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();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to