In isolated tree mode 2, AXIsolatedObject::setChildrenIDs should be called only on secondary thread.
Created attachment 401727 [details] Patch
Comment on attachment 401727 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=401727&action=review > Source/WebCore/ChangeLog:11 > + - 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. INcreased -> Increased > Source/WebCore/accessibility/AccessibilityObjectInterface.h:75 > +typedef size_t AXID; should we make this a uint64_t
Created attachment 401750 [details] Patch
Created attachment 401751 [details] Patch
(In reply to chris fleizach from comment #2) > Comment on attachment 401727 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=401727&action=review > > > Source/WebCore/ChangeLog:11 > > + - 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. > > INcreased -> Increased Fixed. > > > Source/WebCore/accessibility/AccessibilityObjectInterface.h:75 > > +typedef size_t AXID; > > should we make this a uint64_t size_t is guarantied the largest unsigned integer range the system can handle. But we can make it a uint64_t if we prefer to specify the size explicitly.
Committed r262966: <https://trac.webkit.org/changeset/262966> All reviewed patches have been landed. Closing bug and clearing flags on attachment 401751 [details].
<rdar://problem/64307457>
Comment on attachment 401751 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=401751&action=review > Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h:68 > + 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; > + } Style thought for the future: Generally seems like a poor pattern to move code from .cpp files to headers without a strong motivation; long term across many source files it likely makes our code slower to compile so we better be sure there is a benefit. Also, larger class definitions with function bodies can make it harder to see the design of the class. I see, thought, that *this* is the kind of simple setter that can be super efficient so it’s tempting to make it inlined. Even if doing that, I think it’s better style to do that outside the class definition so it’s easier to refactor (move to and from the .cpp file). And I think that global optimization may be effective at inlining even when we don’t move things to headers.