Bug 211790 – AXIsolatedTree::updateChildren needs to apply pending changes before updating the given node.

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Andres Gonzalez]

AXIsolatedTree::updateChildren needs to apply pending changes before updating the given node.

[Andres Gonzalez]

Created attachment 399151 [details]
Patch

[chris fleizach]

Comment on attachment 399151 [details]
Patch

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

> Source/WebCore/ChangeLog:10
> +        AXIsolatedTree::updateChildren may be fired for a and isolated object

"for a and isolated"

> Source/WebCore/ChangeLog:12
> +        fail, causing that the isolated tree is not updated. This patch calls

rewrite:

"causing the isolated tree to not be updated."

[Darin Adler]

Comment on attachment 399151 [details]
Patch

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

> Source/WebCore/accessibility/AXLogger.cpp:63
> -void AXLogger::log(const AXCoreObject& object)
> +void AXLogger::log(const RefPtr<AXCoreObject>& object)

This goes from const to non-const (not sure why), from non-null to allowing null (makes sense), and from raw pointer to a smart pointer (no good reason I can think of for that).

[Andres Gonzalez]

Created attachment 399259 [details]
Patch

[Andres Gonzalez]

(In reply to chris fleizach from comment #2)
> Comment on attachment 399151 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=399151&action=review
> 
> > Source/WebCore/ChangeLog:10
> > +        AXIsolatedTree::updateChildren may be fired for a and isolated object
> 
> "for a and isolated"

Fixe.
> 
> > Source/WebCore/ChangeLog:12
> > +        fail, causing that the isolated tree is not updated. This patch calls
> 
> rewrite:
> 
> "causing the isolated tree to not be updated."

Fixed.

[Andres Gonzalez]

(In reply to Darin Adler from comment #3)
> Comment on attachment 399151 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=399151&action=review
> 
> > Source/WebCore/accessibility/AXLogger.cpp:63
> > -void AXLogger::log(const AXCoreObject& object)
> > +void AXLogger::log(const RefPtr<AXCoreObject>& object)
> 
> This goes from const to non-const (not sure why), from non-null to allowing
> null (makes sense), and from raw pointer to a smart pointer (no good reason
> I can think of for that).

There are some non-const properties in AXCoreObject that we want to log. The smart pointer is an attempt to hold the object while logging it on the secondary thread when it may be released on the main thread.

[EWS]

Committed r261694: <https://trac.webkit.org/changeset/261694>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 399259 [details].

[Radar WebKit Bug Importer]

<rdar://problem/63229053>

[Darin Adler]

Comment on attachment 399151 [details]
Patch

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

>>> Source/WebCore/accessibility/AXLogger.cpp:63
>>> +void AXLogger::log(const RefPtr<AXCoreObject>& object)
>> 
>> This goes from const to non-const (not sure why), from non-null to allowing null (makes sense), and from raw pointer to a smart pointer (no good reason I can think of for that).
> 
> There are some non-const properties in AXCoreObject that we want to log. The smart pointer is an attempt to hold the object while logging it on the secondary thread when it may be released on the main thread.

Using RefPtr is fine for the reason you cite above, but making the argument type be const RefPtr& doesn’t add value, as far as I can tell.