[webkit-changes] [263285] trunk/Source

Fri, 19 Jun 2020 14:04:56 -0700

Title: [263285] trunk/Source
Revision
263285
Author
andresg...@apple.com
Date
2020-06-19 14:03:35 -0700 (Fri, 19 Jun 2020)

Log 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:]):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (263284 => 263285)


--- trunk/Source/WebCore/ChangeLog	2020-06-19 21:03:35 UTC (rev 263284)
+++ trunk/Source/WebCore/ChangeLog	2020-06-19 21:03:35 UTC (rev 263285)
@@ -1,3 +1,23 @@
+2020-06-19  Andres Gonzalez  <andresg...@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  <cdu...@apple.com>
 
         [iOS] RenderThemeIOS::cssValueToSystemColorMap() does an unnecessary linear search under systemColorFromCSSValueID()

Modified: trunk/Source/WebCore/accessibility/AXObjectCache.cpp (263284 => 263285)


--- trunk/Source/WebCore/accessibility/AXObjectCache.cpp	2020-06-19 21:03:35 UTC (rev 263284)
+++ trunk/Source/WebCore/accessibility/AXObjectCache.cpp	2020-06-19 21:03:35 UTC (rev 263285)
@@ -195,6 +195,7 @@
 
 void AXObjectCache::enableAccessibility()
 {
+    ASSERT(isMainThread());
     gAccessibilityEnabled = true;
 }
 
@@ -222,6 +223,7 @@
     , m_currentModalNode(nullptr)
     , m_performCacheUpdateTimer(*this, &AXObjectCache::performCacheUpdateTimerFired)
 {
+    ASSERT(isMainThread());
 }
 
 AXObjectCache::~AXObjectCache()
@@ -1129,6 +1131,7 @@
 {
     AXTRACE("AXObjectCache::postNotification");
     AXLOG(std::make_pair(object, notification));
+    ASSERT(isMainThread());
 
     stopCachingComputedObjectAttributes();

Modified: trunk/Source/WebKit/ChangeLog (263284 => 263285)


--- trunk/Source/WebKit/ChangeLog	2020-06-19 21:03:35 UTC (rev 263284)
+++ trunk/Source/WebKit/ChangeLog	2020-06-19 21:03:35 UTC (rev 263285)
@@ -1,3 +1,26 @@
+2020-06-19  Andres Gonzalez  <andresg...@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  <cfleiz...@apple.com>
 
         AX: Make isolated tree enablement status dependent on client preference

Modified: trunk/Source/WebKit/WebProcess/WebPage/mac/WKAccessibilityWebPageObjectBase.mm (263284 => 263285)


--- trunk/Source/WebKit/WebProcess/WebPage/mac/WKAccessibilityWebPageObjectBase.mm	2020-06-19 21:03:35 UTC (rev 263284)
+++ trunk/Source/WebKit/WebProcess/WebPage/mac/WKAccessibilityWebPageObjectBase.mm	2020-06-19 21:03:35 UTC (rev 263285)
@@ -42,10 +42,14 @@
 #import <WebCore/ScrollView.h>
 #import <WebCore/Scrollbar.h>
 
+namespace ax = WebCore::Accessibility;
+
 @implementation WKAccessibilityWebPageObjectBase
 
 - (NakedPtr<WebCore::AXObjectCache>)axObjectCache
 {
+    ASSERT(isMainThread());
+
     if (!m_page)
         return nullptr;
 
@@ -62,6 +66,7 @@
 
 - (id)accessibilityPluginObject
 {
+    ASSERT(isMainThread());
     auto retrieveBlock = [&self]() -> id {
         id axPlugin = nil;
         auto dispatchBlock = [&axPlugin, &self] {
@@ -84,22 +89,26 @@
 
 - (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;
 
     if (page) {
@@ -115,11 +124,13 @@
 
 - (void)setHasMainFramePlugin:(bool)hasPlugin
 {
+    ASSERT(isMainThread());
     m_hasMainFramePlugin = hasPlugin;
 }
 
 - (void)setRemoteParent:(id)parent
 {
+    ASSERT(isMainThread());
     if (parent != m_parent) {
         [m_parent release];
         m_parent = [parent retain];
@@ -131,5 +142,4 @@
     return [[self accessibilityRootObjectWrapper] accessibilityFocusedUIElement];
 }
 
-
 @end

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to