Bug 211790

Summary: AXIsolatedTree::updateChildren needs to apply pending changes before updating the given node.
Product: WebKit Reporter: Andres Gonzalez <andresg_22>
Component: New BugsAssignee: Andres Gonzalez <andresg_22>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, apinheiro, cfleizach, darin, dmazzoni, ews-watchlist, jcraig, jdiggs, samuel_white, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Andres Gonzalez
Reported 2020-05-12 11:31:34 PDT
AXIsolatedTree::updateChildren needs to apply pending changes before updating the given node.
Attachments
Patch (11.48 KB, patch)
2020-05-12 11:43 PDT, Andres Gonzalez 		no flags
DetailsFormatted DiffDiff
Patch (11.49 KB, patch)
2020-05-13 07:35 PDT, Andres Gonzalez 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Andres Gonzalez
Comment 1 2020-05-12 11:43:40 PDT
Created attachment 399151 [details] Patch
chris fleizach
Comment 2 2020-05-12 11:50:14 PDT
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 3 2020-05-12 12:12:53 PDT
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
Comment 4 2020-05-13 07:35:44 PDT
Created attachment 399259 [details] Patch
Andres Gonzalez
Comment 5 2020-05-13 07:39:13 PDT
(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
Comment 6 2020-05-13 07:44:47 PDT
(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
Comment 7 2020-05-14 08:35:50 PDT
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
Comment 8 2020-05-14 08:36:14 PDT
<rdar://problem/63229053>
Darin Adler
Comment 9 2020-05-18 10:46:41 PDT
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.
Note You need to log in before you can comment on or make changes to this bug.