Title: [185778] trunk/Source/WebCore
Revision
185778
Author
timothy_hor...@apple.com
Date
2015-06-19 16:40:32 -0700 (Fri, 19 Jun 2015)

Log Message

Selection services cannot be invoked when force click is enabled
https://bugs.webkit.org/show_bug.cgi?id=146166
<rdar://problem/21468362>

Reviewed by Darin Adler.

* page/mac/ServicesOverlayController.h:
Turn Highlight::Type into something we can use for dirty flags.

* page/mac/ServicesOverlayController.mm:
(WebCore::ServicesOverlayController::Highlight::createForSelection):
(WebCore::ServicesOverlayController::Highlight::createForTelephoneNumber):
(WebCore::ServicesOverlayController::ServicesOverlayController):
(WebCore::ServicesOverlayController::selectionRectsDidChange):
(WebCore::ServicesOverlayController::selectedTelephoneNumberRangesChanged):
(WebCore::ServicesOverlayController::invalidateHighlightsOfType):
(WebCore::ServicesOverlayController::buildPotentialHighlightsIfNeeded):
(WebCore::ServicesOverlayController::remainingTimeUntilHighlightShouldBeShown):
(WebCore::ServicesOverlayController::buildPhoneNumberHighlights):
(WebCore::ServicesOverlayController::buildSelectionHighlight):
(WebCore::ServicesOverlayController::findTelephoneNumberHighlightContainingSelectionHighlight):
(WebCore::ServicesOverlayController::determineActiveHighlight):
(WebCore::ServicesOverlayController::didScrollFrame):
(WebCore::ServicesOverlayController::handleClick):
Coalesce highlight rebuilding so that things (like TextIndicator creation)
that change the selection and then reset it immediately don't cause us
to lose the active highlight.

This also means that if the selection changes multiple times in a runloop
(easily possible from script), we won't waste a lot of time rebuilding highlights.

(WebCore::ServicesOverlayController::didRebuildPotentialHighlights):
Merged into buildPotentialHighlightsIfNeeded.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (185777 => 185778)


--- trunk/Source/WebCore/ChangeLog	2015-06-19 23:35:53 UTC (rev 185777)
+++ trunk/Source/WebCore/ChangeLog	2015-06-19 23:40:32 UTC (rev 185778)
@@ -1,3 +1,39 @@
+2015-06-19  Tim Horton  <timothy_hor...@apple.com>
+
+        Selection services cannot be invoked when force click is enabled
+        https://bugs.webkit.org/show_bug.cgi?id=146166
+        <rdar://problem/21468362>
+
+        Reviewed by Darin Adler.
+
+        * page/mac/ServicesOverlayController.h:
+        Turn Highlight::Type into something we can use for dirty flags.
+
+        * page/mac/ServicesOverlayController.mm:
+        (WebCore::ServicesOverlayController::Highlight::createForSelection):
+        (WebCore::ServicesOverlayController::Highlight::createForTelephoneNumber):
+        (WebCore::ServicesOverlayController::ServicesOverlayController):
+        (WebCore::ServicesOverlayController::selectionRectsDidChange):
+        (WebCore::ServicesOverlayController::selectedTelephoneNumberRangesChanged):
+        (WebCore::ServicesOverlayController::invalidateHighlightsOfType):
+        (WebCore::ServicesOverlayController::buildPotentialHighlightsIfNeeded):
+        (WebCore::ServicesOverlayController::remainingTimeUntilHighlightShouldBeShown):
+        (WebCore::ServicesOverlayController::buildPhoneNumberHighlights):
+        (WebCore::ServicesOverlayController::buildSelectionHighlight):
+        (WebCore::ServicesOverlayController::findTelephoneNumberHighlightContainingSelectionHighlight):
+        (WebCore::ServicesOverlayController::determineActiveHighlight):
+        (WebCore::ServicesOverlayController::didScrollFrame):
+        (WebCore::ServicesOverlayController::handleClick):
+        Coalesce highlight rebuilding so that things (like TextIndicator creation)
+        that change the selection and then reset it immediately don't cause us
+        to lose the active highlight.
+
+        This also means that if the selection changes multiple times in a runloop
+        (easily possible from script), we won't waste a lot of time rebuilding highlights.
+
+        (WebCore::ServicesOverlayController::didRebuildPotentialHighlights):
+        Merged into buildPotentialHighlightsIfNeeded.
+
 2015-06-19  Matt Baker  <mattba...@apple.com>
 
         Web Inspector: TimelineAgent needs to handle nested runloops

Modified: trunk/Source/WebCore/page/mac/ServicesOverlayController.h (185777 => 185778)


--- trunk/Source/WebCore/page/mac/ServicesOverlayController.h	2015-06-19 23:35:53 UTC (rev 185777)
+++ trunk/Source/WebCore/page/mac/ServicesOverlayController.h	2015-06-19 23:40:32 UTC (rev 185778)
@@ -67,10 +67,11 @@
         Range* range() const { return m_range.get(); }
         GraphicsLayer* layer() const { return m_graphicsLayer.get(); }
 
-        enum class Type {
-            TelephoneNumber,
-            Selection
+        enum {
+            TelephoneNumberType = 1 << 0,
+            SelectionType = 1 << 1,
         };
+        typedef uint8_t Type;
         Type type() const { return m_type; }
 
         void fadeIn();
@@ -108,11 +109,13 @@
 
     void drawHighlight(Highlight&, GraphicsContext&);
 
+    void invalidateHighlightsOfType(Highlight::Type);
+    void buildPotentialHighlightsIfNeeded();
+
     void replaceHighlightsOfTypePreservingEquivalentHighlights(HashSet<RefPtr<Highlight>>&, Highlight::Type);
     void removeAllPotentialHighlightsOfType(Highlight::Type);
     void buildPhoneNumberHighlights();
     void buildSelectionHighlight();
-    void didRebuildPotentialHighlights();
 
     void determineActiveHighlight(bool& mouseIsOverButton);
     void clearActiveHighlight();
@@ -137,7 +140,7 @@
     MainFrame& mainFrame() const { return m_mainFrame; }
 
     MainFrame& m_mainFrame;
-    PageOverlay* m_servicesOverlay;
+    PageOverlay* m_servicesOverlay { nullptr };
 
     RefPtr<Highlight> m_activeHighlight;
     RefPtr<Highlight> m_nextActiveHighlight;
@@ -148,8 +151,10 @@
 
     // FIXME: These should move onto Highlight.
     Vector<LayoutRect> m_currentSelectionRects;
-    bool m_isTextOnly;
+    bool m_isTextOnly { false };
 
+    Highlight::Type m_dirtyHighlightTypes { 0 };
+
     std::chrono::steady_clock::time_point m_lastSelectionChangeTime;
     std::chrono::steady_clock::time_point m_nextActiveHighlightChangeTime;
     std::chrono::steady_clock::time_point m_lastMouseUpTime;
@@ -158,6 +163,7 @@
     IntPoint m_mousePosition;
 
     Timer m_determineActiveHighlightTimer;
+    Timer m_buildHighlightsTimer;
 };
 
 } // namespace WebKit

Modified: trunk/Source/WebCore/page/mac/ServicesOverlayController.mm (185777 => 185778)


--- trunk/Source/WebCore/page/mac/ServicesOverlayController.mm	2015-06-19 23:35:53 UTC (rev 185777)
+++ trunk/Source/WebCore/page/mac/ServicesOverlayController.mm	2015-06-19 23:40:32 UTC (rev 185778)
@@ -57,12 +57,12 @@
 
 Ref<ServicesOverlayController::Highlight> ServicesOverlayController::Highlight::createForSelection(ServicesOverlayController& controller, RetainPtr<DDHighlightRef> ddHighlight, PassRefPtr<Range> range)
 {
-    return adoptRef(*new Highlight(controller, Type::Selection, ddHighlight, range));
+    return adoptRef(*new Highlight(controller, Highlight::SelectionType, ddHighlight, range));
 }
 
 Ref<ServicesOverlayController::Highlight> ServicesOverlayController::Highlight::createForTelephoneNumber(ServicesOverlayController& controller, RetainPtr<DDHighlightRef> ddHighlight, PassRefPtr<Range> range)
 {
-    return adoptRef(*new Highlight(controller, Type::TelephoneNumber, ddHighlight, range));
+    return adoptRef(*new Highlight(controller, Highlight::TelephoneNumberType, ddHighlight, range));
 }
 
 ServicesOverlayController::Highlight::Highlight(ServicesOverlayController& controller, Type type, RetainPtr<DDHighlightRef> ddHighlight, PassRefPtr<WebCore::Range> range)
@@ -204,9 +204,8 @@
 
 ServicesOverlayController::ServicesOverlayController(MainFrame& mainFrame)
     : m_mainFrame(mainFrame)
-    , m_servicesOverlay(nullptr)
-    , m_isTextOnly(false)
     , m_determineActiveHighlightTimer(*this, &ServicesOverlayController::determineActiveHighlightTimerFired)
+    , m_buildHighlightsTimer(*this, &ServicesOverlayController::buildPotentialHighlightsIfNeeded)
 {
 }
 
@@ -379,8 +378,7 @@
     m_currentSelectionRects.reverse();
 
     LOG(Services, "ServicesOverlayController - Selection rects changed - Now have %lu\n", rects.size());
-
-    buildSelectionHighlight();
+    invalidateHighlightsOfType(Highlight::SelectionType);
 #else
     UNUSED_PARAM(rects);
     UNUSED_PARAM(gapRects);
@@ -392,10 +390,47 @@
 {
 #if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED > 1090
     LOG(Services, "ServicesOverlayController - Telephone number ranges changed\n");
-    buildPhoneNumberHighlights();
+    invalidateHighlightsOfType(Highlight::TelephoneNumberType);
 #endif
 }
 
+void ServicesOverlayController::invalidateHighlightsOfType(Highlight::Type type)
+{
+    if (!m_mainFrame.settings().serviceControlsEnabled())
+        return;
+
+    m_dirtyHighlightTypes |= type;
+    m_buildHighlightsTimer.startOneShot(0);
+}
+
+void ServicesOverlayController::buildPotentialHighlightsIfNeeded()
+{
+    if (!m_dirtyHighlightTypes)
+        return;
+
+    if (m_dirtyHighlightTypes & Highlight::TelephoneNumberType)
+        buildPhoneNumberHighlights();
+
+    if (m_dirtyHighlightTypes & Highlight::SelectionType)
+        buildSelectionHighlight();
+
+    m_dirtyHighlightTypes = 0;
+
+    if (m_potentialHighlights.isEmpty()) {
+        if (m_servicesOverlay)
+            m_mainFrame.pageOverlayController().uninstallPageOverlay(m_servicesOverlay, PageOverlay::FadeMode::DoNotFade);
+        return;
+    }
+
+    if (telephoneNumberRangesForFocusedFrame().isEmpty() && !hasRelevantSelectionServices())
+        return;
+
+    createOverlayIfNeeded();
+
+    bool mouseIsOverButton;
+    determineActiveHighlight(mouseIsOverButton);
+}
+
 bool ServicesOverlayController::mouseIsOverHighlight(Highlight& highlight, bool& mouseIsOverButton) const
 {
     Boolean onButton;
@@ -419,7 +454,7 @@
     // 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. However, we will still avoid highlighting
     // telephone numbers while the mouse is down.
-    if (highlight->type() == Highlight::Type::TelephoneNumber)
+    if (highlight->type() == Highlight::TelephoneNumberType)
         return mousePressed ? minimumTimeUntilHighlightShouldBeShown : 0_ms;
 
     auto now = std::chrono::steady_clock::now();
@@ -465,16 +500,12 @@
 
 void ServicesOverlayController::buildPhoneNumberHighlights()
 {
-    if (!m_mainFrame.settings().serviceControlsEnabled())
-        return;
-
     Vector<RefPtr<Range>> phoneNumberRanges;
     for (Frame* frame = &m_mainFrame; frame; frame = frame->tree().traverseNext())
         phoneNumberRanges.appendVector(frame->editor().detectedTelephoneNumberRanges());
 
     if (phoneNumberRanges.isEmpty()) {
-        removeAllPotentialHighlightsOfType(Highlight::Type::TelephoneNumber);
-        didRebuildPotentialHighlights();
+        removeAllPotentialHighlightsOfType(Highlight::TelephoneNumberType);
         return;
     }
 
@@ -505,19 +536,13 @@
         newPotentialHighlights.add(Highlight::createForTelephoneNumber(*this, ddHighlight, range));
     }
 
-    replaceHighlightsOfTypePreservingEquivalentHighlights(newPotentialHighlights, Highlight::Type::TelephoneNumber);
-
-    didRebuildPotentialHighlights();
+    replaceHighlightsOfTypePreservingEquivalentHighlights(newPotentialHighlights, Highlight::TelephoneNumberType);
 }
 
 void ServicesOverlayController::buildSelectionHighlight()
 {
-    if (!m_mainFrame.settings().serviceControlsEnabled())
-        return;
-
     if (m_currentSelectionRects.isEmpty()) {
-        removeAllPotentialHighlightsOfType(Highlight::Type::Selection);
-        didRebuildPotentialHighlights();
+        removeAllPotentialHighlightsOfType(Highlight::SelectionType);
         return;
     }
 
@@ -555,9 +580,7 @@
         }
     }
 
-    replaceHighlightsOfTypePreservingEquivalentHighlights(newPotentialHighlights, Highlight::Type::Selection);
-
-    didRebuildPotentialHighlights();
+    replaceHighlightsOfTypePreservingEquivalentHighlights(newPotentialHighlights, Highlight::SelectionType);
 }
 
 void ServicesOverlayController::replaceHighlightsOfTypePreservingEquivalentHighlights(HashSet<RefPtr<Highlight>>& newPotentialHighlights, Highlight::Type type)
@@ -595,23 +618,6 @@
     return false;
 }
 
-void ServicesOverlayController::didRebuildPotentialHighlights()
-{
-    if (m_potentialHighlights.isEmpty()) {
-        if (m_servicesOverlay)
-            m_mainFrame.pageOverlayController().uninstallPageOverlay(m_servicesOverlay, PageOverlay::FadeMode::DoNotFade);
-        return;
-    }
-
-    if (telephoneNumberRangesForFocusedFrame().isEmpty() && !hasRelevantSelectionServices())
-        return;
-
-    createOverlayIfNeeded();
-
-    bool mouseIsOverButton;
-    determineActiveHighlight(mouseIsOverButton);
-}
-
 void ServicesOverlayController::createOverlayIfNeeded()
 {
     if (m_servicesOverlay)
@@ -650,7 +656,7 @@
 
 ServicesOverlayController::Highlight* ServicesOverlayController::findTelephoneNumberHighlightContainingSelectionHighlight(Highlight& selectionHighlight)
 {
-    if (selectionHighlight.type() != Highlight::Type::Selection)
+    if (selectionHighlight.type() != Highlight::SelectionType)
         return nullptr;
 
     Page* page = m_mainFrame.page();
@@ -666,7 +672,7 @@
         return nullptr;
 
     for (auto& highlight : m_potentialHighlights) {
-        if (highlight->type() != Highlight::Type::TelephoneNumber)
+        if (highlight->type() != Highlight::TelephoneNumberType)
             continue;
 
         if (highlight->range()->contains(*activeSelectionRange))
@@ -678,15 +684,17 @@
 
 void ServicesOverlayController::determineActiveHighlight(bool& mouseIsOverActiveHighlightButton)
 {
+    buildPotentialHighlightsIfNeeded();
+
     mouseIsOverActiveHighlightButton = false;
 
     RefPtr<Highlight> newActiveHighlight;
 
     for (auto& highlight : m_potentialHighlights) {
-        if (highlight->type() == Highlight::Type::Selection) {
+        if (highlight->type() == Highlight::SelectionType) {
             // If we've already found a new active highlight, and it's
             // a telephone number highlight, prefer that over this selection highlight.
-            if (newActiveHighlight && newActiveHighlight->type() == Highlight::Type::TelephoneNumber)
+            if (newActiveHighlight && newActiveHighlight->type() == Highlight::TelephoneNumberType)
                 continue;
 
             // If this highlight has no compatible services, it can't be active.
@@ -705,7 +713,7 @@
 
     // If our new active highlight is a selection highlight that is completely contained
     // by one of the phone number highlights, we'll make the phone number highlight active even if it's not hovered.
-    if (newActiveHighlight && newActiveHighlight->type() == Highlight::Type::Selection) {
+    if (newActiveHighlight && newActiveHighlight->type() == Highlight::SelectionType) {
         if (Highlight* containedTelephoneNumberHighlight = findTelephoneNumberHighlightContainingSelectionHighlight(*newActiveHighlight)) {
             newActiveHighlight = containedTelephoneNumberHighlight;
 
@@ -801,8 +809,9 @@
     if (frame.isMainFrame())
         return;
 
-    buildPhoneNumberHighlights();
-    buildSelectionHighlight();
+    invalidateHighlightsOfType(Highlight::TelephoneNumberType);
+    invalidateHighlightsOfType(Highlight::SelectionType);
+    buildPotentialHighlightsIfNeeded();
 
     bool mouseIsOverActiveHighlightButton;
     determineActiveHighlight(mouseIsOverActiveHighlightButton);
@@ -820,7 +829,7 @@
 
     IntPoint windowPoint = frameView->contentsToWindow(clickPoint);
 
-    if (highlight.type() == Highlight::Type::Selection) {
+    if (highlight.type() == Highlight::SelectionType) {
         auto telephoneNumberRanges = telephoneNumberRangesForFocusedFrame();
         Vector<String> selectedTelephoneNumbers;
         selectedTelephoneNumbers.reserveCapacity(telephoneNumberRanges.size());
@@ -828,7 +837,7 @@
             selectedTelephoneNumbers.append(range->text());
 
         page->chrome().client().handleSelectionServiceClick(page->focusController().focusedOrMainFrame().selection(), selectedTelephoneNumbers, windowPoint);
-    } else if (highlight.type() == Highlight::Type::TelephoneNumber)
+    } else if (highlight.type() == Highlight::TelephoneNumberType)
         page->chrome().client().handleTelephoneNumberClick(highlight.range()->text(), windowPoint);
 }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to