Bug 206596 - Implement AX isolated tree mode support for event notifications.
Summary: Implement AX isolated tree mode support for event notifications.
Status: RESOLVED DUPLICATE of bug 207581
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Andres Gonzalez
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-01-22 10:44 PST by Andres Gonzalez
Modified: 2022-02-10 16:26 PST (History)
8 users (show)

See Also:

Attachments
Patch (32.42 KB, patch)
2020-01-22 11:08 PST, Andres Gonzalez 		no flags Details | Formatted Diff | Diff
Patch (33.92 KB, patch)
2020-01-22 11:57 PST, Andres Gonzalez 		no flags Details | Formatted Diff | Diff
Patch (33.99 KB, patch)
2020-01-22 12:45 PST, Andres Gonzalez 		no flags Details | Formatted Diff | Diff
Patch (34.78 KB, patch)
2020-01-22 13:11 PST, Andres Gonzalez 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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 ***