Bug 206596

Summary: Implement AX isolated tree mode support for event notifications.
Product: WebKit Reporter: Andres Gonzalez <andresg_22>
Component: New BugsAssignee: Andres Gonzalez <andresg_22>
Status: RESOLVED DUPLICATE    
Severity: Normal CC: aboxhall, apinheiro, cfleizach, dmazzoni, ews-watchlist, jcraig, jdiggs, samuel_white
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Description Andres Gonzalez 2020-01-22 10:44:16 PST
Implement AX isolated tree mode support for event notifications.
Comment 1 Andres Gonzalez 2020-01-22 11:08:59 PST
Created attachment 388442 [details]
Patch
Comment 2 Andres Gonzalez 2020-01-22 11:57:57 PST
Created attachment 388455 [details]
Patch
Comment 3 Andres Gonzalez 2020-01-22 11:59:21 PST
(In reply to Andres Gonzalez from comment #2)
> Created attachment 388455 [details]
> Patch

Fix for GTK and WIN builds.
Comment 4 chris fleizach 2020-01-22 12:12:02 PST
Comment on attachment 388455 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=388455&action=review

> Source/WebCore/accessibility/AXObjectCache.cpp:606
> +void AXObjectCache::cacheAndWrap(AccessibilityObject* newObject, RenderObject* renderer, Node* node, Widget* widget)

maybe rename to cacheAndInitializeWrapperObject()

> Source/WebCore/accessibility/AXObjectCache.cpp:806
> +    RefPtr<AccessibilityObject> obj;

how come you want to remove the initialization?
Comment 5 Andres Gonzalez 2020-01-22 12:22:15 PST
(In reply to chris fleizach from comment #4)
> Comment on attachment 388455 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=388455&action=review
> 
> > Source/WebCore/accessibility/AXObjectCache.cpp:606
> > +void AXObjectCache::cacheAndWrap(AccessibilityObject* newObject, RenderObject* renderer, Node* node, Widget* widget)
> 
> maybe rename to cacheAndInitializeWrapperObject()

Done.
> 
> > Source/WebCore/accessibility/AXObjectCache.cpp:806
> > +    RefPtr<AccessibilityObject> obj;
> 
> how come you want to remove the initialization?

The default constructor of most smart pointers typically initialize the internal pointer to nullptr, like in the case of RefPtr:

    ALWAYS_INLINE constexpr RefPtr() : m_ptr(nullptr) { }

In this case it is also followed by a switch that will initialize the pointer to something, including:

    default:
        obj = nullptr;
Comment 6 Andres Gonzalez 2020-01-22 12:45:40 PST
Created attachment 388461 [details]
Patch
Comment 7 Andres Gonzalez 2020-01-22 13:11:18 PST
Created attachment 388462 [details]
Patch
Comment 8 Andres Gonzalez 2020-01-22 13:13:06 PST
(In reply to Andres Gonzalez from comment #7)
> Created attachment 388462 [details]
> Patch

Another fix for GTK.
Comment 9 chris fleizach 2020-01-22 15:33:49 PST
Comment on attachment 388462 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=388462&action=review

> Source/WebCore/accessibility/AXObjectCache.cpp:1032
> +        AccessibilityObject* obj = note.first.get();

seems like an auto here would be an improvement

> Source/WebCore/accessibility/AXObjectCache.cpp:1152
> +        return isolatedObjectForAXObject(*axObject).get();

this worries me here that we're accessing the tree and objects on that tree on the main thread now
Comment 10 chris fleizach 2020-03-07 00:13:50 PST
Do we still need this one? or is it obsolete
Comment 11 Andres Gonzalez 2020-03-07 07:57:56 PST

*** This bug has been marked as a duplicate of bug 207581 ***