Changeset 263285 in webkit

Timestamp:

Jun 19, 2020, 2:03:35 PM (6 years ago)

Author:

Andres Gonzalez

Message:

AX: web process crash in AXObjectCache::postNotification.
​https://bugs.webkit.org/show_bug.cgi?id=213398

Reviewed by Chris Fleizach.

AXObjectCache was being instantiated on the AX secondary thread.
Therefore the timers for the different delayed notifications where
initialized with the secondary thread. When postNotification was triggered
on the main thread as it should, and the timer was accessed, the timer
would assert/crash for being accessed in a thread different than where
it was created. This change guaranties that AXObjectCache is always
created on the main thread.

Source/WebCore:

• accessibility/AXObjectCache.cpp:

(WebCore::AXObjectCache::enableAccessibility):
(WebCore::AXObjectCache::AXObjectCache):
(WebCore::AXObjectCache::postNotification):

Source/WebKit:

• WebProcess/WebPage/mac/WKAccessibilityWebPageObjectBase.mm:

(-[WKAccessibilityWebPageObjectBase axObjectCache]):
(-[WKAccessibilityWebPageObjectBase accessibilityPluginObject]):
(-[WKAccessibilityWebPageObjectBase accessibilityRootObjectWrapper]):
(-[WKAccessibilityWebPageObjectBase setWebPage:]):
(-[WKAccessibilityWebPageObjectBase setHasMainFramePlugin:]):
(-[WKAccessibilityWebPageObjectBase setRemoteParent:]):

Location:

trunk/Source

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/accessibility/AXObjectCache.cpp (modified) (3 diffs)
• WebKit/ChangeLog (modified) (1 diff)
• WebKit/WebProcess/WebPage/mac/WKAccessibilityWebPageObjectBase.mm (modified) (6 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-06-19  Andres Gonzalez  <andresg_22@apple.com>
+
+        AX: web process crash in AXObjectCache::postNotification.
+        https://bugs.webkit.org/show_bug.cgi?id=213398
+
+        Reviewed by Chris Fleizach.
+
+        AXObjectCache was being instantiated on the AX secondary thread.
+        Therefore the timers for the different delayed notifications where
+        initialized with the secondary thread. When postNotification was triggered
+        on the main thread as it should, and the timer was accessed, the timer
+        would assert/crash for being accessed in a thread different than where
+        it was created. This change guaranties that AXObjectCache is always
+        created on the main thread.
+
+        * accessibility/AXObjectCache.cpp:
+        (WebCore::AXObjectCache::enableAccessibility):
+        (WebCore::AXObjectCache::AXObjectCache):
+        (WebCore::AXObjectCache::postNotification):
+
 2020-06-19  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WebCore/accessibility/AXObjectCache.cpp

@@ -196 +196 @@
 void AXObjectCache::enableAccessibility()
 {
+    ASSERT(isMainThread());
     gAccessibilityEnabled = true;
 }
@@ -223 +224 @@
     , m_performCacheUpdateTimer(*this, &AXObjectCache::performCacheUpdateTimerFired)
 {
+    ASSERT(isMainThread());
 }
 
@@ -1130 +1132 @@
     AXTRACE("AXObjectCache::postNotification");
     AXLOG(std::make_pair(object, notification));
+    ASSERT(isMainThread());
 
     stopCachingComputedObjectAttributes();

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2020-06-19  Andres Gonzalez  <andresg_22@apple.com>
+
+        AX: web process crash in AXObjectCache::postNotification.
+        https://bugs.webkit.org/show_bug.cgi?id=213398
+
+        Reviewed by Chris Fleizach.
+
+        AXObjectCache was being instantiated on the AX secondary thread.
+        Therefore the timers for the different delayed notifications where
+        initialized with the secondary thread. When postNotification was triggered
+        on the main thread as it should, and the timer was accessed, the timer
+        would assert/crash for being accessed in a thread different than where
+        it was created. This change guaranties that AXObjectCache is always
+        created on the main thread.
+
+        * WebProcess/WebPage/mac/WKAccessibilityWebPageObjectBase.mm:
+        (-[WKAccessibilityWebPageObjectBase axObjectCache]):
+        (-[WKAccessibilityWebPageObjectBase accessibilityPluginObject]):
+        (-[WKAccessibilityWebPageObjectBase accessibilityRootObjectWrapper]):
+        (-[WKAccessibilityWebPageObjectBase setWebPage:]):
+        (-[WKAccessibilityWebPageObjectBase setHasMainFramePlugin:]):
+        (-[WKAccessibilityWebPageObjectBase setRemoteParent:]):
+
 2020-06-19  Chris Fleizach  <cfleizach@apple.com>
 

trunk/Source/WebKit/WebProcess/WebPage/mac/WKAccessibilityWebPageObjectBase.mm

@@ -43 +43 @@
 #import <WebCore/Scrollbar.h>
 
+namespace ax = WebCore::Accessibility;
+
 @implementation WKAccessibilityWebPageObjectBase
 
 - (NakedPtr<WebCore::AXObjectCache>)axObjectCache
 {
+    ASSERT(isMainThread());
+
     if (!m_page)
         return nullptr;
@@ -63 +67 @@
 - (id)accessibilityPluginObject
 {
+    ASSERT(isMainThread());
     auto retrieveBlock = [&self]() -> id {
         id axPlugin = nil;
@@ -85 +90 @@
 - (id)accessibilityRootObjectWrapper
 {
-    if (!WebCore::AXObjectCache::accessibilityEnabled())
-        WebCore::AXObjectCache::enableAccessibility();
+    return ax::retrieveAutoreleasedValueFromMainThread<id>([protectedSelf = retainPtr(self)] () -> RetainPtr<id> {
+        if (!WebCore::AXObjectCache::accessibilityEnabled())
+            WebCore::AXObjectCache::enableAccessibility();
 
-    if (m_hasMainFramePlugin)
-        return self.accessibilityPluginObject;
+        if (protectedSelf.get()->m_hasMainFramePlugin)
+            return protectedSelf.get().accessibilityPluginObject;
 
-    if (auto cache = [self axObjectCache]) {
-        if (WebCore::AXCoreObject* root = cache->rootObject())
-            return root->wrapper();
-    }
+        if (auto cache = protectedSelf.get().axObjectCache) {
+            if (auto* root = cache->rootObject())
+                return root->wrapper();
+        }
 
-    return nil;
+        return nil;
+    });
 }
 
 - (void)setWebPage:(NakedPtr<WebKit::WebPage>)page
 {
+    ASSERT(isMainThread());
+
     m_page = page;
 
@@ -116 +125 @@
 - (void)setHasMainFramePlugin:(bool)hasPlugin
 {
+    ASSERT(isMainThread());
     m_hasMainFramePlugin = hasPlugin;
 }
@@ -121 +131 @@
 - (void)setRemoteParent:(id)parent
 {
+    ASSERT(isMainThread());
     if (parent != m_parent) {
         [m_parent release];
@@ -132 +143 @@
 }
 
-
 @end