DataDetectors PageOverlay callbacks can come in to the wrong overlay
authortimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 17 Nov 2014 20:27:55 +0000 (20:27 +0000)
committertimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 17 Nov 2014 20:27:55 +0000 (20:27 +0000)
https://bugs.webkit.org/show_bug.cgi?id=138801
<rdar://problem/18991369>

Reviewed by Anders Carlsson.

We can call dataDetectorsDidHideUI on the wrong (or no) PageOverlay,
because it's possible to have another performActionMenuHitTestAtLocation
between the first one and the dataDetectorsDid*UI that correspond to it.

To make sure that the callbacks come to the right PageOverlay, plumb
an integer ID through all of the callbacks and find the overlay based on that.

* Shared/mac/ActionMenuHitTestResult.h:
Store the ID of the PageOverlay that overrode data detection.

* UIProcess/mac/WKActionMenuController.mm:
(-[WKActionMenuController _defaultMenuItemsForDataDetectedText]):
Send the ID back to the Web process when replying from DD callbacks.

* WebProcess/WebPage/WebPage.h:
* WebProcess/WebPage/WebPage.messages.in:
* WebProcess/WebPage/mac/WebPageMac.mm:
(WebKit::WebPage::performActionMenuHitTestAtLocation):
Store the ID of the PageOverlay that overrode data detection instead of
keeping the PageOverlay itself.

(WebKit::WebPage::dataDetectorsDidPresentUI):
(WebKit::WebPage::dataDetectorsDidChangeUI):
(WebKit::WebPage::dataDetectorsDidHideUI):
Find and reply to the correct page overlay.

* page/PageOverlay.cpp:
(WebCore::generatePageOverlayID):
(WebCore::PageOverlay::PageOverlay):
* page/PageOverlay.h:
Give each PageOverlay a unique ID.

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

Source/WebCore/ChangeLog
Source/WebCore/page/PageOverlay.cpp
Source/WebCore/page/PageOverlay.h
Source/WebKit2/ChangeLog
Source/WebKit2/Shared/mac/ActionMenuHitTestResult.h
Source/WebKit2/UIProcess/mac/WKActionMenuController.mm
Source/WebKit2/WebProcess/WebPage/WebPage.h
Source/WebKit2/WebProcess/WebPage/WebPage.messages.in
Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm

index 81c59ac17dc56f121be535d5f3f0aefa829b07ca..4a804c58d6cd969437c17c35817f2db1738f6c63 100644 (file)
@@ -1,3 +1,17 @@
+2014-11-17  Tim Horton  <timothy_horton@apple.com>
+
+        DataDetectors PageOverlay callbacks can come in to the wrong overlay
+        https://bugs.webkit.org/show_bug.cgi?id=138801
+        <rdar://problem/18991369>
+
+        Reviewed by Anders Carlsson.
+
+        * page/PageOverlay.cpp:
+        (WebCore::generatePageOverlayID):
+        (WebCore::PageOverlay::PageOverlay):
+        * page/PageOverlay.h:
+        Give each PageOverlay a unique ID.
+
 2014-11-17  Javier Fernandez  <jfernandez@igalia.com>
 
         [CSS Grid Layout] Upgrade align-self and align-items parsing to CSS 3
index b51d46448703ea1c9b8279852319f138b410ee8d..254d89b67f33d0e4c914c2b286e824c5cfd8933a 100644 (file)
@@ -40,6 +40,12 @@ namespace WebCore {
 static const double fadeAnimationDuration = 0.2;
 static const double fadeAnimationFrameRate = 30;
 
+static PageOverlay::PageOverlayID generatePageOverlayID()
+{
+    static PageOverlay::PageOverlayID pageOverlayID;
+    return ++pageOverlayID;
+}
+
 PassRefPtr<PageOverlay> PageOverlay::create(Client& client, OverlayType overlayType)
 {
     return adoptRef(new PageOverlay(client, overlayType));
@@ -55,6 +61,7 @@ PageOverlay::PageOverlay(Client& client, OverlayType overlayType)
     , m_fractionFadedIn(1)
     , m_overlayType(overlayType)
     , m_backgroundColor(Color::transparent)
+    , m_pageOverlayID(generatePageOverlayID())
 {
 }
 
index 70febc8cba2e57b91d0df62480e0436599c307b2..e8da7797236f250b77a242b9883a6a2b8cc88fdd 100644 (file)
@@ -74,6 +74,9 @@ public:
 
     PageOverlayController* controller() const;
 
+    typedef uint64_t PageOverlayID;
+    virtual PageOverlayID pageOverlayID() const { return m_pageOverlayID; }
+
     void setPage(Page*);
     void setNeedsDisplay(const IntRect& dirtyRect);
     void setNeedsDisplay();
@@ -134,6 +137,7 @@ private:
     IntRect m_overrideFrame;
 
     RGBA32 m_backgroundColor;
+    PageOverlayID m_pageOverlayID;
 };
 
 } // namespace WebKit
index 992c70bd4dbc14e1aacd58c0b39c351d31ef1f2b..3adeb0a5157ff62af58d440103d4a7a8210fddfd 100644 (file)
@@ -1,3 +1,37 @@
+2014-11-17  Tim Horton  <timothy_horton@apple.com>
+
+        DataDetectors PageOverlay callbacks can come in to the wrong overlay
+        https://bugs.webkit.org/show_bug.cgi?id=138801
+        <rdar://problem/18991369>
+
+        Reviewed by Anders Carlsson.
+
+        We can call dataDetectorsDidHideUI on the wrong (or no) PageOverlay,
+        because it's possible to have another performActionMenuHitTestAtLocation
+        between the first one and the dataDetectorsDid*UI that correspond to it.
+
+        To make sure that the callbacks come to the right PageOverlay, plumb
+        an integer ID through all of the callbacks and find the overlay based on that.
+
+        * Shared/mac/ActionMenuHitTestResult.h:
+        Store the ID of the PageOverlay that overrode data detection.
+
+        * UIProcess/mac/WKActionMenuController.mm:
+        (-[WKActionMenuController _defaultMenuItemsForDataDetectedText]):
+        Send the ID back to the Web process when replying from DD callbacks.
+
+        * WebProcess/WebPage/WebPage.h:
+        * WebProcess/WebPage/WebPage.messages.in:
+        * WebProcess/WebPage/mac/WebPageMac.mm:
+        (WebKit::WebPage::performActionMenuHitTestAtLocation):
+        Store the ID of the PageOverlay that overrode data detection instead of
+        keeping the PageOverlay itself.
+
+        (WebKit::WebPage::dataDetectorsDidPresentUI):
+        (WebKit::WebPage::dataDetectorsDidChangeUI):
+        (WebKit::WebPage::dataDetectorsDidHideUI):
+        Find and reply to the correct page overlay.
+
 2014-11-17  Beth Dakin  <bdakin@apple.com>
 
         Whitespace action menus should focus the HitTestResult and create an insertion 
index ae4a08a78e739ab3495c06c551eeda1ae53326ef..9d1ca90c53f55e21a58ce9cd722e73ca7083bc04 100644 (file)
@@ -30,6 +30,7 @@
 #include "TextIndicator.h"
 #include "WebHitTestResult.h"
 #include <WebCore/FloatRect.h>
+#include <WebCore/PageOverlay.h>
 #include <wtf/text/WTFString.h>
 
 OBJC_CLASS DDActionContext;
@@ -54,6 +55,7 @@ struct ActionMenuHitTestResult {
     RetainPtr<DDActionContext> actionContext;
     WebCore::FloatRect detectedDataBoundingBox;
     RefPtr<TextIndicator> detectedDataTextIndicator;
+    WebCore::PageOverlay::PageOverlayID detectedDataOriginatingPageOverlay;
 };
 
 } // namespace WebKit
index 843617b90d7aad6fed4a585a5994ccc47b8ce9df..ea4620a9235fbffa487454b0b9ae5368e663c07f 100644 (file)
@@ -578,14 +578,15 @@ static NSString *pathToPhotoOnDisk(NSString *suggestedFilename)
 
     // Ref our WebPageProxy for use in the blocks below.
     RefPtr<WebPageProxy> page = _page;
+    PageOverlay::PageOverlayID overlayID = _hitTestResult.detectedDataOriginatingPageOverlay;
     _currentActionContext = [actionContext contextForView:_wkView altMode:YES interactionStartedHandler:^() {
-        page->send(Messages::WebPage::DataDetectorsDidPresentUI());
+        page->send(Messages::WebPage::DataDetectorsDidPresentUI(overlayID));
     } interactionChangedHandler:^() {
         [self _showTextIndicator];
-        page->send(Messages::WebPage::DataDetectorsDidChangeUI());
+        page->send(Messages::WebPage::DataDetectorsDidChangeUI(overlayID));
     } interactionStoppedHandler:^() {
         [self _hideTextIndicator];
-        page->send(Messages::WebPage::DataDetectorsDidHideUI());
+        page->send(Messages::WebPage::DataDetectorsDidHideUI(overlayID));
     }];
 
     [_currentActionContext setHighlightFrame:[_wkView.window convertRectToScreen:[_wkView convertRect:_hitTestResult.detectedDataBoundingBox toView:nil]]];
index 0c5aaa980f0b8d7b5be7ce7f68ceaf2bccaa69ea..5871fd62040dfd35f4ded8720118cef922242577 100644 (file)
@@ -53,6 +53,7 @@
 #include <WebCore/IntRect.h>
 #include <WebCore/IntSizeHash.h>
 #include <WebCore/Page.h>
+#include <WebCore/PageOverlay.h>
 #include <WebCore/PageVisibilityState.h>
 #include <WebCore/ScrollTypes.h>
 #include <WebCore/TextChecking.h>
@@ -1060,9 +1061,9 @@ private:
     void selectLastActionMenuRange();
     void focusAndSelectLastActionMenuHitTestResult();
 
-    void dataDetectorsDidPresentUI();
-    void dataDetectorsDidChangeUI();
-    void dataDetectorsDidHideUI();
+    void dataDetectorsDidPresentUI(WebCore::PageOverlay::PageOverlayID);
+    void dataDetectorsDidChangeUI(WebCore::PageOverlay::PageOverlayID);
+    void dataDetectorsDidHideUI(WebCore::PageOverlay::PageOverlayID);
 #endif
 
     uint64_t m_pageID;
index 34f8e52388e4be2ae4c6ea5526558016f962fc16..aead09232f6043cb02cb77b047eebf10da310f1b 100644 (file)
@@ -401,8 +401,8 @@ messages -> WebPage LegacyReceiver {
     PerformActionMenuHitTestAtLocation(WebCore::FloatPoint location)
     SelectLastActionMenuRange()
     FocusAndSelectLastActionMenuHitTestResult()
-    DataDetectorsDidPresentUI()
-    DataDetectorsDidChangeUI()
-    DataDetectorsDidHideUI()
+    DataDetectorsDidPresentUI(WebCore::PageOverlay::PageOverlayID pageOverlay)
+    DataDetectorsDidChangeUI(WebCore::PageOverlay::PageOverlayID pageOverlay)
+    DataDetectorsDidHideUI(WebCore::PageOverlay::PageOverlayID pageOverlay)
 #endif
 }
index bca5d9ce6fae7a16a6e5ed0cbf7bace36873b1e8..59785c3f4a3ac23ccc5f601ec6405231e72ef041 100644 (file)
@@ -974,8 +974,6 @@ String WebPage::platformUserAgent(const URL&) const
 
 void WebPage::performActionMenuHitTestAtLocation(WebCore::FloatPoint locationInViewCooordinates)
 {
-    m_lastActionMenuHitPageOverlay = nullptr;
-
     layoutIfNeeded();
 
     MainFrame& mainFrame = corePage()->mainFrame();
@@ -1039,8 +1037,8 @@ void WebPage::performActionMenuHitTestAtLocation(WebCore::FloatPoint locationInV
 
         actionMenuResult.detectedDataBoundingBox = detectedDataBoundingBox;
         actionMenuResult.detectedDataTextIndicator = TextIndicator::createWithRange(*mainResultRange);
+        actionMenuResult.detectedDataOriginatingPageOverlay = overlay->pageOverlayID();
         m_lastActionMenuRangeForSelection = mainResultRange;
-        m_lastActionMenuHitPageOverlay = webOverlay;
 
         break;
     }
@@ -1100,23 +1098,40 @@ void WebPage::focusAndSelectLastActionMenuHitTestResult()
     element->document().frame()->selection().setSelection(position);
 }
 
-void WebPage::dataDetectorsDidPresentUI()
+void WebPage::dataDetectorsDidPresentUI(PageOverlay::PageOverlayID overlayID)
 {
-    if (m_lastActionMenuHitPageOverlay)
-        m_lastActionMenuHitPageOverlay->dataDetectorsDidPresentUI();
+    MainFrame& mainFrame = corePage()->mainFrame();
+    for (const auto& overlay : mainFrame.pageOverlayController().pageOverlays()) {
+        if (overlay->pageOverlayID() == overlayID) {
+            if (WebPageOverlay* webOverlay = WebPageOverlay::fromCoreOverlay(*overlay))
+                webOverlay->dataDetectorsDidPresentUI();
+            return;
+        }
+    }
 }
 
-void WebPage::dataDetectorsDidChangeUI()
+void WebPage::dataDetectorsDidChangeUI(PageOverlay::PageOverlayID overlayID)
 {
-    if (m_lastActionMenuHitPageOverlay)
-        m_lastActionMenuHitPageOverlay->dataDetectorsDidChangeUI();
+    MainFrame& mainFrame = corePage()->mainFrame();
+    for (const auto& overlay : mainFrame.pageOverlayController().pageOverlays()) {
+        if (overlay->pageOverlayID() == overlayID) {
+            if (WebPageOverlay* webOverlay = WebPageOverlay::fromCoreOverlay(*overlay))
+                webOverlay->dataDetectorsDidChangeUI();
+            return;
+        }
+    }
 }
 
-void WebPage::dataDetectorsDidHideUI()
+void WebPage::dataDetectorsDidHideUI(PageOverlay::PageOverlayID overlayID)
 {
-    if (m_lastActionMenuHitPageOverlay)
-        m_lastActionMenuHitPageOverlay->dataDetectorsDidHideUI();
-    m_lastActionMenuHitPageOverlay = nil;
+    MainFrame& mainFrame = corePage()->mainFrame();
+    for (const auto& overlay : mainFrame.pageOverlayController().pageOverlays()) {
+        if (overlay->pageOverlayID() == overlayID) {
+            if (WebPageOverlay* webOverlay = WebPageOverlay::fromCoreOverlay(*overlay))
+                webOverlay->dataDetectorsDidHideUI();
+            return;
+        }
+    }
 }
 
 } // namespace WebKit