Services overlay flashes a lot; should have some hysteresis before showing overlay
authortimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 7 Aug 2014 02:58:22 +0000 (02:58 +0000)
committertimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 7 Aug 2014 02:58:22 +0000 (02:58 +0000)
https://bugs.webkit.org/show_bug.cgi?id=135683
<rdar://problem/16878039>

Reviewed by Simon Fraser.

Don't show the highlight until it's been 200ms since the last change
in selection or change in which highlight is hovered, whichever was more recent.

* WebProcess/WebPage/ServicesOverlayController.h:
* WebProcess/WebPage/mac/ServicesOverlayController.mm:
(WebKit::ServicesOverlayController::ServicesOverlayController):
(WebKit::ServicesOverlayController::selectionRectsDidChange):
Keep track of when the selection last changed.

(WebKit::ServicesOverlayController::drawTelephoneNumberHighlightIfVisible):
Make establishHoveredTelephoneHighlight take a bool instead of Boolean.

(WebKit::ServicesOverlayController::mouseIsOverHighlight):
Factor mouseIsOverHighlight out of establishHoveredTelephoneHighlight and drawHighlight.

(WebKit::ServicesOverlayController::remainingTimeUntilHighlightShouldBeShown):
Return the amount of time until the highlight should be shown; this is
the maximum of (the difference between the last selection change and the timeout)
and (the difference between the last change in which highlight is hovered and the timeout).

Telephone number highlights are shown immediately, because they are already stable
by virtue of being expanded to include the entire telephone number.

(WebKit::ServicesOverlayController::repaintHighlightTimerFired):
(WebKit::ServicesOverlayController::drawHighlight):
If the highlight shouldn't be shown yet (because we haven't hit the two timeouts),
schedule a timer to repaint us around when we will hit the timeouts.

(WebKit::ServicesOverlayController::establishHoveredTelephoneHighlight):
(WebKit::ServicesOverlayController::mouseEvent):
Don't allow mouseUp to trigger the menu if we shouldn't be showing the overlay yet.

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

Source/WebKit2/ChangeLog
Source/WebKit2/WebProcess/WebPage/ServicesOverlayController.h
Source/WebKit2/WebProcess/WebPage/mac/ServicesOverlayController.mm

index 5a3384a8e8ef74c0884a5eaa9099154f17d36a9f..9b4aa02060cac8aab995e4d31449de2d0e5c024f 100644 (file)
@@ -1,3 +1,43 @@
+2014-08-06  Tim Horton  <timothy_horton@apple.com>
+
+        Services overlay flashes a lot; should have some hysteresis before showing overlay
+        https://bugs.webkit.org/show_bug.cgi?id=135683
+        <rdar://problem/16878039>
+
+        Reviewed by Simon Fraser.
+
+        Don't show the highlight until it's been 200ms since the last change
+        in selection or change in which highlight is hovered, whichever was more recent.
+
+        * WebProcess/WebPage/ServicesOverlayController.h:
+        * WebProcess/WebPage/mac/ServicesOverlayController.mm:
+        (WebKit::ServicesOverlayController::ServicesOverlayController):
+        (WebKit::ServicesOverlayController::selectionRectsDidChange):
+        Keep track of when the selection last changed.
+
+        (WebKit::ServicesOverlayController::drawTelephoneNumberHighlightIfVisible):
+        Make establishHoveredTelephoneHighlight take a bool instead of Boolean.
+
+        (WebKit::ServicesOverlayController::mouseIsOverHighlight):
+        Factor mouseIsOverHighlight out of establishHoveredTelephoneHighlight and drawHighlight.
+
+        (WebKit::ServicesOverlayController::remainingTimeUntilHighlightShouldBeShown):
+        Return the amount of time until the highlight should be shown; this is
+        the maximum of (the difference between the last selection change and the timeout)
+        and (the difference between the last change in which highlight is hovered and the timeout).
+
+        Telephone number highlights are shown immediately, because they are already stable
+        by virtue of being expanded to include the entire telephone number.
+
+        (WebKit::ServicesOverlayController::repaintHighlightTimerFired):
+        (WebKit::ServicesOverlayController::drawHighlight):
+        If the highlight shouldn't be shown yet (because we haven't hit the two timeouts),
+        schedule a timer to repaint us around when we will hit the timeouts.
+
+        (WebKit::ServicesOverlayController::establishHoveredTelephoneHighlight):
+        (WebKit::ServicesOverlayController::mouseEvent):
+        Don't allow mouseUp to trigger the menu if we shouldn't be showing the overlay yet.
+
 2014-08-06  Simon Fraser  <simon.fraser@apple.com>
 
         [iOS WK2] www.france24.com doesn't always load the page, sections stay white
index 8f30a9a1d665d5efa27a363abbf9fb3274ee48e2..2304782c5b5ba3b3012fd592cc7f583fc6b49bd9 100644 (file)
@@ -30,6 +30,7 @@
 
 #include "PageOverlay.h"
 #include <WebCore/Range.h>
+#include <WebCore/Timer.h>
 
 typedef void* DDHighlightRef;
 
@@ -79,17 +80,23 @@ private:
     void drawSelectionHighlight(WebCore::GraphicsContext&, const WebCore::IntRect& dirtyRect);
     void drawHighlight(DDHighlightRef, WebCore::GraphicsContext&);
 
-    void establishHoveredTelephoneHighlight(Boolean& onButton);
+    void establishHoveredTelephoneHighlight(bool& mouseIsOverButton);
     void maybeCreateSelectionHighlight();
 
     void clearSelectionHighlight();
     void clearHoveredTelephoneNumberHighlight();
 
+    bool mouseIsOverHighlight(DDHighlightRef, bool& mouseIsOverButton) const;
+    std::chrono::milliseconds remainingTimeUntilHighlightShouldBeShown() const;
+    void repaintHighlightTimerFired(WebCore::Timer<ServicesOverlayController>&);
+
     WebPage* m_webPage;
     PageOverlay* m_servicesOverlay;
     
     Vector<WebCore::LayoutRect> m_currentSelectionRects;
     RetainPtr<DDHighlightRef> m_selectionHighlight;
+    std::chrono::steady_clock::time_point m_lastSelectionChangeTime;
+    std::chrono::steady_clock::time_point m_lastHoveredHighlightChangeTime;
 
     Vector<RefPtr<WebCore::Range>> m_currentTelephoneNumberRanges;
     Vector<RetainPtr<DDHighlightRef>> m_telephoneNumberHighlights;
@@ -98,6 +105,8 @@ private:
     RetainPtr<DDHighlightRef> m_currentHoveredHighlight;
     RetainPtr<DDHighlightRef> m_currentMouseDownOnButtonHighlight;
 
+    WebCore::Timer<ServicesOverlayController> m_repaintHighlightTimer;
+
     WebCore::IntPoint m_mousePosition;
 };
 
index 32c7d94500167e4ea3c28b62c58ef7817f5e6a65..950ee6adab2309f953e86db06b0ad8a072184e17 100644 (file)
@@ -76,6 +76,7 @@ static IntRect textQuadsToBoundingRectForRange(Range& range)
 ServicesOverlayController::ServicesOverlayController(WebPage& webPage)
     : m_webPage(&webPage)
     , m_servicesOverlay(nullptr)
+    , m_repaintHighlightTimer(this, &ServicesOverlayController::repaintHighlightTimerFired)
 {
 }
 
@@ -216,6 +217,8 @@ void ServicesOverlayController::selectionRectsDidChange(const Vector<LayoutRect>
     clearSelectionHighlight();
     m_currentSelectionRects = rects;
 
+    m_lastSelectionChangeTime = std::chrono::steady_clock::now();
+
     compactRectsWithGapRects(m_currentSelectionRects, gapRects);
 
     // DataDetectors needs these reversed in order to place the arrow in the right location.
@@ -306,8 +309,8 @@ bool ServicesOverlayController::drawTelephoneNumberHighlightIfVisible(WebCore::G
 
     // Found out which - if any - telephone number is hovered.
     if (!m_hoveredTelephoneNumberData) {
-        Boolean onButton;
-        establishHoveredTelephoneHighlight(onButton);
+        bool mouseIsOverButton;
+        establishHoveredTelephoneHighlight(mouseIsOverButton);
     }
 
     // If a telephone number is actually hovered, draw it.
@@ -319,18 +322,52 @@ bool ServicesOverlayController::drawTelephoneNumberHighlightIfVisible(WebCore::G
     return false;
 }
 
+bool ServicesOverlayController::mouseIsOverHighlight(DDHighlightRef highlight, bool& mouseIsOverButton) const
+{
+    Boolean onButton;
+    bool hovered = DDHighlightPointIsOnHighlight(highlight, (CGPoint)m_mousePosition, &onButton);
+    mouseIsOverButton = onButton;
+    return hovered;
+}
+
+std::chrono::milliseconds ServicesOverlayController::remainingTimeUntilHighlightShouldBeShown() const
+{
+    // Highlight hysteresis is only for selection services, because telephone number highlights are already much more stable
+    // by virtue of being expanded to include the entire telephone number.
+    if (m_hoveredTelephoneNumberData)
+        return std::chrono::milliseconds::zero();
+
+    std::chrono::steady_clock::duration minimumTimeUntilHighlightShouldBeShown = 200_ms;
+
+    auto now = std::chrono::steady_clock::now();
+    auto timeSinceLastSelectionChange = now - m_lastSelectionChangeTime;
+    auto timeSinceMouseOverSelection = now - m_lastHoveredHighlightChangeTime;
+
+    return std::chrono::duration_cast<std::chrono::milliseconds>(std::max(minimumTimeUntilHighlightShouldBeShown - timeSinceLastSelectionChange, minimumTimeUntilHighlightShouldBeShown - timeSinceMouseOverSelection));
+}
+
+void ServicesOverlayController::repaintHighlightTimerFired(WebCore::Timer<ServicesOverlayController>&)
+{
+    if (m_servicesOverlay)
+        m_servicesOverlay->setNeedsDisplay();
+}
+
 void ServicesOverlayController::drawHighlight(DDHighlightRef highlight, WebCore::GraphicsContext& graphicsContext)
 {
     ASSERT(highlight);
 
-    Boolean onButton;
-    bool mouseIsOverHighlight = DDHighlightPointIsOnHighlight(highlight, (CGPoint)m_mousePosition, &onButton);
-
-    if (!mouseIsOverHighlight) {
+    bool mouseIsOverButton;
+    if (!mouseIsOverHighlight(highlight, mouseIsOverButton)) {
         LOG(Services, "ServicesOverlayController::drawHighlight - Mouse is not over highlight, so drawing nothing");
         return;
     }
 
+    auto remainingTimeUntilHighlightShouldBeShown = this->remainingTimeUntilHighlightShouldBeShown();
+    if (remainingTimeUntilHighlightShouldBeShown > std::chrono::steady_clock::duration::zero()) {
+        m_repaintHighlightTimer.startOneShot(remainingTimeUntilHighlightShouldBeShown);
+        return;
+    }
+
     CGContextRef cgContext = graphicsContext.platformContext();
     
     CGLayerRef highlightLayer = DDHighlightGetLayerWithContext(highlight, cgContext);
@@ -371,7 +408,7 @@ void ServicesOverlayController::clearHoveredTelephoneNumberHighlight()
     m_hoveredTelephoneNumberData = nullptr;
 }
 
-void ServicesOverlayController::establishHoveredTelephoneHighlight(Boolean& onButton)
+void ServicesOverlayController::establishHoveredTelephoneHighlight(bool& mouseIsOverButton)
 {
     ASSERT(m_currentTelephoneNumberRanges.size() == m_telephoneNumberHighlights.size());
 
@@ -395,7 +432,7 @@ void ServicesOverlayController::establishHoveredTelephoneHighlight(Boolean& onBu
             m_telephoneNumberHighlights[i] = adoptCF(DDHighlightCreateWithRectsInVisibleRectWithStyleAndDirection(nullptr, &cgRect, 1, viewForRange->boundsRect(), DDHighlightOutlineWithArrow, YES, NSWritingDirectionNatural, NO, YES));
         }
 
-        if (!DDHighlightPointIsOnHighlight(m_telephoneNumberHighlights[i].get(), (CGPoint)m_mousePosition, &onButton))
+        if (!mouseIsOverHighlight(m_telephoneNumberHighlights[i].get(), mouseIsOverButton))
             continue;
 
         if (!m_hoveredTelephoneNumberData || m_hoveredTelephoneNumberData->highlight != m_telephoneNumberHighlights[i])
@@ -406,7 +443,7 @@ void ServicesOverlayController::establishHoveredTelephoneHighlight(Boolean& onBu
     }
 
     clearHoveredTelephoneNumberHighlight();
-    onButton = false;
+    mouseIsOverButton = false;
 }
 
 void ServicesOverlayController::maybeCreateSelectionHighlight()
@@ -434,8 +471,8 @@ bool ServicesOverlayController::mouseEvent(PageOverlay*, const WebMouseEvent& ev
 
     DDHighlightRef oldHoveredHighlight = m_currentHoveredHighlight.get();
 
-    Boolean onButton = false;
-    establishHoveredTelephoneHighlight(onButton);
+    bool mouseIsOverButton = false;
+    establishHoveredTelephoneHighlight(mouseIsOverButton);
     if (m_hoveredTelephoneNumberData) {
         ASSERT(m_hoveredTelephoneNumberData->highlight);
         m_currentHoveredHighlight = m_hoveredTelephoneNumberData->highlight;
@@ -443,14 +480,16 @@ bool ServicesOverlayController::mouseEvent(PageOverlay*, const WebMouseEvent& ev
         if (!m_selectionHighlight)
             maybeCreateSelectionHighlight();
 
-        if (m_selectionHighlight && DDHighlightPointIsOnHighlight(m_selectionHighlight.get(), (CGPoint)m_mousePosition, &onButton))
+        if (m_selectionHighlight && mouseIsOverHighlight(m_selectionHighlight.get(), mouseIsOverButton))
             m_currentHoveredHighlight = m_selectionHighlight;
         else
             m_currentHoveredHighlight = nullptr;
     }
 
-    if (oldHoveredHighlight != m_currentHoveredHighlight)
+    if (oldHoveredHighlight != m_currentHoveredHighlight) {
+        m_lastHoveredHighlightChangeTime = std::chrono::steady_clock::now();
         m_servicesOverlay->setNeedsDisplay();
+    }
 
     // If this event has nothing to do with the left button, it clears the current mouse down tracking and we're done processing it.
     if (event.button() != WebMouseEvent::LeftButton) {
@@ -463,7 +502,7 @@ bool ServicesOverlayController::mouseEvent(PageOverlay*, const WebMouseEvent& ev
         RetainPtr<DDHighlightRef> mouseDownHighlight = WTF::move(m_currentMouseDownOnButtonHighlight);
 
         // If the mouse lifted while still over the highlight button that it went down on, then that is a click.
-        if (onButton && mouseDownHighlight) {
+        if (mouseIsOverButton && mouseDownHighlight && remainingTimeUntilHighlightShouldBeShown() <= std::chrono::steady_clock::duration::zero()) {
             handleClick(m_mousePosition, mouseDownHighlight.get());
             return true;
         }
@@ -474,7 +513,7 @@ bool ServicesOverlayController::mouseEvent(PageOverlay*, const WebMouseEvent& ev
     // Check and see if the mouse moved within the confines of the DD highlight button.
     if (event.type() == WebEvent::MouseMove) {
         // Moving with the mouse button down is okay as long as the mouse never leaves the highlight button.
-        if (m_currentMouseDownOnButtonHighlight && onButton)
+        if (m_currentMouseDownOnButtonHighlight && mouseIsOverButton)
             return true;
 
         m_currentMouseDownOnButtonHighlight = nullptr;
@@ -483,7 +522,7 @@ bool ServicesOverlayController::mouseEvent(PageOverlay*, const WebMouseEvent& ev
 
     // Check and see if the mouse went down over a DD highlight button.
     if (event.type() == WebEvent::MouseDown) {
-        if (m_currentHoveredHighlight && onButton) {
+        if (m_currentHoveredHighlight && mouseIsOverButton) {
             m_currentMouseDownOnButtonHighlight = m_currentHoveredHighlight;
             m_servicesOverlay->setNeedsDisplay();
             return true;