WKAccessibilityWebPageObject should use Accessibility::retrieveValueFromMainThread.
authorandresg_22@apple.com <andresg_22@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 9 Jan 2020 21:39:42 +0000 (21:39 +0000)
committerandresg_22@apple.com <andresg_22@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 9 Jan 2020 21:39:42 +0000 (21:39 +0000)
https://bugs.webkit.org/show_bug.cgi?id=206009

Reviewed by Chris Fleizach.

- WKAccessibilityWebPageObject now uses Accessibility::retrieveValueFromMainThread,
which is consistent with WebAccessibilityObjectWrapper.
- It also uses a captured protectedSelf to ensure the object is alive
when the lambda is invoked on the main thread.
- Added nullity check for m_page in accessibilityAttributeSizeValue,
which is a potential crasher in the multithreaded mode.

* WebProcess/WebPage/mac/WKAccessibilityWebPageObjectMac.mm:
(-[WKAccessibilityWebPageObject ALLOW_DEPRECATED_IMPLEMENTATIONS_END]):
(-[WKAccessibilityWebPageObject convertScreenPointToRootView:]):
(-[WKAccessibilityWebPageObject accessibilityAttributeSizeValue]):
(-[WKAccessibilityWebPageObject accessibilityAttributePositionValue]):
(-[WKAccessibilityWebPageObject accessibilityDataDetectorValue:point:]):
(-[WKAccessibilityWebPageObject accessibilityHitTest:]):
(retrieveAccessibilityValueFromMainThread): Not needed since it uses now the one in Accessibility nasespace.

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

Source/WebKit/ChangeLog
Source/WebKit/WebProcess/WebPage/mac/WKAccessibilityWebPageObjectMac.mm

index da89b5e..c0b0586 100644 (file)
@@ -1,3 +1,26 @@
+2020-01-09  Andres Gonzalez  <andresg_22@apple.com>
+
+        WKAccessibilityWebPageObject should use Accessibility::retrieveValueFromMainThread.
+        https://bugs.webkit.org/show_bug.cgi?id=206009
+
+        Reviewed by Chris Fleizach.
+
+        - WKAccessibilityWebPageObject now uses Accessibility::retrieveValueFromMainThread,
+        which is consistent with WebAccessibilityObjectWrapper.
+        - It also uses a captured protectedSelf to ensure the object is alive
+        when the lambda is invoked on the main thread.
+        - Added nullity check for m_page in accessibilityAttributeSizeValue,
+        which is a potential crasher in the multithreaded mode.
+
+        * WebProcess/WebPage/mac/WKAccessibilityWebPageObjectMac.mm:
+        (-[WKAccessibilityWebPageObject ALLOW_DEPRECATED_IMPLEMENTATIONS_END]):
+        (-[WKAccessibilityWebPageObject convertScreenPointToRootView:]):
+        (-[WKAccessibilityWebPageObject accessibilityAttributeSizeValue]):
+        (-[WKAccessibilityWebPageObject accessibilityAttributePositionValue]):
+        (-[WKAccessibilityWebPageObject accessibilityDataDetectorValue:point:]):
+        (-[WKAccessibilityWebPageObject accessibilityHitTest:]):
+        (retrieveAccessibilityValueFromMainThread): Not needed since it uses now the one in Accessibility nasespace.
+
 2020-01-09  John Wilander  <wilander@apple.com>
 
         Resource Load Statistics: Flip experimental website data removal setting from an enable to a disable
index 8d121bc..5914ec4 100644 (file)
 #import <pal/spi/cocoa/NSAccessibilitySPI.h>
 #import <wtf/ObjCRuntimeExtras.h>
 
+namespace ax = WebCore::Accessibility;
 
 @implementation WKAccessibilityWebPageObject
 
+#define PROTECTED_SELF protectedSelf = RetainPtr<WKAccessibilityWebPageObject>(self)
+
 - (void)dealloc
 {
     NSAccessibilityUnregisterUniqueIdForUIElement(self);
@@ -79,28 +82,16 @@ ALLOW_DEPRECATED_IMPLEMENTATIONS_END
     return m_attributeNames.get();
 }
 
-template<typename T, typename U> inline T retrieveAccessibilityValueFromMainThread(U&& lambda)
-{
-    if (isMainThread())
-        return lambda();
-
-    T value;
-    callOnMainThreadAndWait([&value, &lambda] {
-        value = lambda();
-    });
-    return value;
-}
-
 ALLOW_DEPRECATED_IMPLEMENTATIONS_BEGIN
 - (NSArray *)accessibilityParameterizedAttributeNames
 ALLOW_DEPRECATED_IMPLEMENTATIONS_END
 {
-    return retrieveAccessibilityValueFromMainThread<id>([&self] () -> id {
+    return ax::retrieveValueFromMainThread<id>([PROTECTED_SELF] () -> id {
         NSMutableArray *names = [NSMutableArray array];
-        if (!m_page)
+        if (!protectedSelf->m_page)
             return names;
         
-        if (auto corePage = m_page->corePage()) {
+        if (auto corePage = protectedSelf->m_page->corePage()) {
             for (auto& name : corePage->pageOverlayController().copyAccessibilityAttributesNames(true))
                 [names addObject:(NSString *)name];
         }
@@ -123,8 +114,10 @@ ALLOW_DEPRECATED_IMPLEMENTATIONS_END
 
 - (NSPoint)convertScreenPointToRootView:(NSPoint)point
 {
-    return retrieveAccessibilityValueFromMainThread<NSPoint>([&self, &point] () -> NSPoint {
-        return m_page->screenToRootView(WebCore::IntPoint(point.x, point.y));
+    return ax::retrieveValueFromMainThread<NSPoint>([&point, PROTECTED_SELF] () -> NSPoint {
+        if (!protectedSelf->m_page)
+            return point;
+        return protectedSelf->m_page->screenToRootView(WebCore::IntPoint(point.x, point.y));
     });
 }
 
@@ -189,30 +182,36 @@ ALLOW_DEPRECATED_IMPLEMENTATIONS_END
 
 - (NSValue *)accessibilityAttributeSizeValue
 {
-    return retrieveAccessibilityValueFromMainThread<id>([&self] () -> id {
-        return [NSValue valueWithSize:(NSSize)m_page->size()];
+    return ax::retrieveValueFromMainThread<id>([PROTECTED_SELF] () -> id {
+        if (!protectedSelf->m_page)
+            return nil;
+        return [NSValue valueWithSize:(NSSize)protectedSelf->m_page->size()];
     });
 }
 
 - (NSValue *)accessibilityAttributePositionValue
 {
-    return retrieveAccessibilityValueFromMainThread<id>([&self] () -> id {
-        return [NSValue valueWithPoint:(NSPoint)m_page->accessibilityPosition()];
+    return ax::retrieveValueFromMainThread<id>([PROTECTED_SELF] () -> id {
+        if (!protectedSelf->m_page)
+            return nil;
+        return [NSValue valueWithPoint:(NSPoint)protectedSelf->m_page->accessibilityPosition()];
     });
 }
 
 - (id)accessibilityDataDetectorValue:(NSString *)attribute point:(WebCore::FloatPoint&)point
 {
-    return retrieveAccessibilityValueFromMainThread<id>([&self, &attribute, &point] () -> id {
+    return ax::retrieveValueFromMainThread<id>([&attribute, &point, PROTECTED_SELF] () -> id {
+        if (!protectedSelf->m_page)
+            return nil;
         id value = nil;
         if ([attribute isEqualToString:@"AXDataDetectorExistsAtPoint"] || [attribute isEqualToString:@"AXDidShowDataDetectorMenuAtPoint"]) {
             bool boolValue;
-            if (m_page->corePage()->pageOverlayController().copyAccessibilityAttributeBoolValueForPoint(attribute, point, boolValue))
+            if (protectedSelf->m_page->corePage()->pageOverlayController().copyAccessibilityAttributeBoolValueForPoint(attribute, point, boolValue))
                 value = [NSNumber numberWithBool:boolValue];
         }
         if ([attribute isEqualToString:@"AXDataDetectorTypeAtPoint"]) {
             String stringValue;
-            if (m_page->corePage()->pageOverlayController().copyAccessibilityAttributeStringValueForPoint(attribute, point, stringValue))
+            if (protectedSelf->m_page->corePage()->pageOverlayController().copyAccessibilityAttributeStringValueForPoint(attribute, point, stringValue))
                 value = [NSString stringWithString:stringValue];
         }
         return value;
@@ -243,12 +242,12 @@ ALLOW_DEPRECATED_IMPLEMENTATIONS_END
 ALLOW_DEPRECATED_DECLARATIONS_BEGIN
 - (id)accessibilityHitTest:(NSPoint)point
 {
-    auto convertedPoint = retrieveAccessibilityValueFromMainThread<WebCore::IntPoint>([&self, &point] () -> WebCore::IntPoint {
-        if (!m_page)
+    auto convertedPoint = ax::retrieveValueFromMainThread<WebCore::IntPoint>([&point, PROTECTED_SELF] () -> WebCore::IntPoint {
+        if (!protectedSelf->m_page)
             return WebCore::IntPoint(point);
-        
-        auto convertedPoint = m_page->screenToRootView(WebCore::IntPoint(point));
-        
+
+        auto convertedPoint = protectedSelf->m_page->screenToRootView(WebCore::IntPoint(point));
+
         // Some plugins may be able to figure out the scroll position and inset on their own.
         bool applyContentOffset = true;
 
@@ -257,15 +256,15 @@ ALLOW_DEPRECATED_DECLARATIONS_BEGIN
         bool queryingIsolatedTree = WebCore::AXObjectCache::clientSupportsIsolatedTree() && _AXUIElementRequestServicedBySecondaryAXThread();
         applyContentOffset = !queryingIsolatedTree;
 #endif
-        if (auto pluginView = WebKit::WebPage::pluginViewForFrame(m_page->mainFrame()))
+        if (auto pluginView = WebKit::WebPage::pluginViewForFrame(protectedSelf->m_page->mainFrame()))
             applyContentOffset = !pluginView->plugin()->pluginHandlesContentOffsetForAccessibilityHitTest();
         
         if (!applyContentOffset)
             return convertedPoint;
 
-        if (WebCore::FrameView* frameView = m_page->mainFrameView())
+        if (auto* frameView = protectedSelf->m_page->mainFrameView())
             convertedPoint.moveBy(frameView->scrollPosition());
-        if (WebCore::Page* page = m_page->corePage())
+        if (auto* page = protectedSelf->m_page->corePage())
             convertedPoint.move(0, -page->topContentInset());
         return convertedPoint;
     });