Title: [262966] trunk/Source/WebCore
- Revision
- 262966
- Author
- andresg...@apple.com
- Date
- 2020-06-12 12:24:56 -0700 (Fri, 12 Jun 2020)
Log Message
In isolated tree mode 2, AXIsolatedObject::setChildrenIDs should be called only on secondary thread.
https://bugs.webkit.org/show_bug.cgi?id=213124
Reviewed by Chris Fleizach.
Covered by existing tests.
- AXIsolatedTree::createSubtree was calling AXIsolatedObject::setChildrenIDs
which should be called only on the secondary thread. Now it is queueing
the children update under lock.
- The unsigned int range for object IDs was being overrun for large
number of objects like in the case of the sample page in <rdar://problem/59331146>.
Increased the size of AXID to size_t.
- Better handle the case of invalid object IDs, although this needs
more work, since we should never encounter this case.
* accessibility/AccessibilityObjectInterface.h: AXID are now size_t instead of unsigned ints.
* accessibility/isolatedtree/AXIsolatedObject.cpp:
(WebCore::AXIsolatedObject::AXIsolatedObject):
(WebCore::AXIsolatedObject::setChildrenIDs): Inlined in header.
* accessibility/isolatedtree/AXIsolatedObject.h:
* accessibility/isolatedtree/AXIsolatedTree.cpp:
(WebCore::AXIsolatedTree::createSubtree):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (262965 => 262966)
--- trunk/Source/WebCore/ChangeLog 2020-06-12 19:14:29 UTC (rev 262965)
+++ trunk/Source/WebCore/ChangeLog 2020-06-12 19:24:56 UTC (rev 262966)
@@ -1,3 +1,29 @@
+2020-06-12 Andres Gonzalez <andresg...@apple.com>
+
+ In isolated tree mode 2, AXIsolatedObject::setChildrenIDs should be called only on secondary thread.
+ https://bugs.webkit.org/show_bug.cgi?id=213124
+
+ Reviewed by Chris Fleizach.
+
+ Covered by existing tests.
+
+ - AXIsolatedTree::createSubtree was calling AXIsolatedObject::setChildrenIDs
+ which should be called only on the secondary thread. Now it is queueing
+ the children update under lock.
+ - The unsigned int range for object IDs was being overrun for large
+ number of objects like in the case of the sample page in <rdar://problem/59331146>.
+ Increased the size of AXID to size_t.
+ - Better handle the case of invalid object IDs, although this needs
+ more work, since we should never encounter this case.
+
+ * accessibility/AccessibilityObjectInterface.h: AXID are now size_t instead of unsigned ints.
+ * accessibility/isolatedtree/AXIsolatedObject.cpp:
+ (WebCore::AXIsolatedObject::AXIsolatedObject):
+ (WebCore::AXIsolatedObject::setChildrenIDs): Inlined in header.
+ * accessibility/isolatedtree/AXIsolatedObject.h:
+ * accessibility/isolatedtree/AXIsolatedTree.cpp:
+ (WebCore::AXIsolatedTree::createSubtree):
+
2020-06-12 Andy Estes <aes...@apple.com>
FileInputType should use WeakPtr for FileListCreator lambdas
Modified: trunk/Source/WebCore/accessibility/AccessibilityObjectInterface.h (262965 => 262966)
--- trunk/Source/WebCore/accessibility/AccessibilityObjectInterface.h 2020-06-12 19:14:29 UTC (rev 262965)
+++ trunk/Source/WebCore/accessibility/AccessibilityObjectInterface.h 2020-06-12 19:24:56 UTC (rev 262966)
@@ -72,7 +72,7 @@
struct AccessibilityText;
struct ScrollRectToVisibleOptions;
-typedef unsigned AXID;
+typedef size_t AXID;
extern const AXID InvalidAXID;
enum class AccessibilityRole {
Modified: trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp (262965 => 262966)
--- trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp 2020-06-12 19:14:29 UTC (rev 262965)
+++ trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp 2020-06-12 19:24:56 UTC (rev 262966)
@@ -44,7 +44,12 @@
ASSERT(isMainThread());
if (auto tree = AXIsolatedTree::treeForID(m_treeID))
m_cachedTree = tree;
- initializeAttributeData(object, parentID == InvalidAXID);
+ if (m_id != InvalidAXID)
+ initializeAttributeData(object, parentID == InvalidAXID);
+ else {
+ // Should never happen under normal circumstances.
+ ASSERT_NOT_REACHED();
+ }
}
Ref<AXIsolatedObject> AXIsolatedObject::create(AXCoreObject& object, AXIsolatedTreeID treeID, AXID parentID)
@@ -456,12 +461,6 @@
m_attributeMap.set(propertyName, value);
}
-void AXIsolatedObject::setChildrenIDs(Vector<AXID>&& childrenIDs)
-{
- ASSERT(isMainThread());
- m_childrenIDs = childrenIDs;
-}
-
void AXIsolatedObject::setParent(AXID parent)
{
ASSERT(isMainThread());
Modified: trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h (262965 => 262966)
--- trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h 2020-06-12 19:14:29 UTC (rev 262965)
+++ trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h 2020-06-12 19:24:56 UTC (rev 262966)
@@ -58,7 +58,14 @@
bool isDetached() const override;
void setParent(AXID);
- void setChildrenIDs(Vector<AXID>&&);
+ void setChildrenIDs(Vector<AXID>&& childrenIDs)
+ {
+ // FIXME: The following ASSERT should be met but it is commented out at the
+ // moment because of <rdar://problem/63985646> After calling _AXUIElementUseSecondaryAXThread(true),
+ // still receives client request on main thread.
+ // ASSERT(axObjectCache()->canUseSecondaryAXThread() ? !isMainThread() : isMainThread());
+ m_childrenIDs = childrenIDs;
+ }
private:
void detachRemoteParts(AccessibilityDetachmentType) override;
Modified: trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp (262965 => 262966)
--- trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp 2020-06-12 19:14:29 UTC (rev 262965)
+++ trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp 2020-06-12 19:24:56 UTC (rev 262966)
@@ -181,6 +181,12 @@
ASSERT(isMainThread());
auto object = AXIsolatedObject::create(axObject, m_treeID, parentID);
+ if (object->objectID() == InvalidAXID) {
+ // Either the axObject has an invalid ID or something else went terribly wrong. Don't bother doing anything else.
+ ASSERT_NOT_REACHED();
+ return object;
+ }
+
if (attachWrapper) {
object->attachPlatformWrapper(axObject.wrapper());
// Since this object has already an attached wrapper, set the wrapper
@@ -197,7 +203,10 @@
childrenIDs.append(child->objectID());
}
m_nodeMap.set(object->objectID(), childrenIDs);
- object->setChildrenIDs(WTFMove(childrenIDs));
+ {
+ LockHolder locker { m_changeLogLock };
+ m_pendingChildrenUpdates.append(std::make_pair(object->objectID(), childrenIDs));
+ }
return object;
}
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
