Bug 180503

Summary: Remove nodes from AXObjectCache when the associated subframe document is getting destroyed.
Product: WebKit Reporter: alan <zalan>
Component: AccessibilityAssignee: alan <zalan>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, apinheiro, cdumez, cfleizach, commit-queue, dbates, dmazzoni, esprehn+autocc, ews-watchlist, jcraig, jdiggs, kangil.han, n_wang, samuel_white, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 173540    
Attachments:
Description Flags
Patch
none
Patch none

alan
Reported 2017-12-06 14:02:33 PST
rdar://problem/35891328
Attachments
Patch (9.15 KB, patch)
2017-12-06 14:37 PST, alan 		no flags
DetailsFormatted DiffDiff
Patch (9.14 KB, patch)
2017-12-06 14:49 PST, alan 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
alan
Comment 1 2017-12-06 14:37:52 PST
Created attachment 328632 [details] Patch
chris fleizach
Comment 2 2017-12-06 14:45:24 PST
Comment on attachment 328632 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=328632&action=review > Source/WebCore/ChangeLog:25 > + lists are tend to stay small. those lists tend (strikeout are)
alan
Comment 3 2017-12-06 14:49:19 PST
Created attachment 328634 [details] Patch
Nan Wang
Comment 4 2017-12-06 15:04:02 PST
Comment on attachment 328634 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=328634&action=review > Source/WebCore/accessibility/AXObjectCache.cpp:2729 > +static void filterForRemoval(const ListHashSet<T>& list, const Document& document, HashSet<Node*>& nodesToRemove) Why do we have a typename here for *list* but *nodesToRemove* is limited to <Node*>? This won't work if T is not a derived class of Node, right?
alan
Comment 5 2017-12-06 15:56:25 PST
(In reply to Nan Wang from comment #4) > Comment on attachment 328634 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=328634&action=review > > > Source/WebCore/accessibility/AXObjectCache.cpp:2729 > > +static void filterForRemoval(const ListHashSet<T>& list, const Document& document, HashSet<Node*>& nodesToRemove) > > Why do we have a typename here for *list* but *nodesToRemove* is limited to > <Node*>? > This won't work if T is not a derived class of Node, right? ::remove() takes a Node& so nodesToRemove can't have non-Node based objects.
Nan Wang
Comment 6 2017-12-06 16:04:35 PST
Comment on attachment 328634 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=328634&action=review >>> Source/WebCore/accessibility/AXObjectCache.cpp:2729 >>> +static void filterForRemoval(const ListHashSet<T>& list, const Document& document, HashSet<Node*>& nodesToRemove) >> >> Why do we have a typename here for *list* but *nodesToRemove* is limited to <Node*>? >> This won't work if T is not a derived class of Node, right? > > ::remove() takes a Node& so nodesToRemove can't have non-Node based objects. I see. But list has to contain only node based objects right? Why is the template necessary?
alan
Comment 7 2017-12-06 16:27:01 PST
(In reply to Nan Wang from comment #6) > Comment on attachment 328634 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=328634&action=review > > >>> Source/WebCore/accessibility/AXObjectCache.cpp:2729 > >>> +static void filterForRemoval(const ListHashSet<T>& list, const Document& document, HashSet<Node*>& nodesToRemove) > >> > >> Why do we have a typename here for *list* but *nodesToRemove* is limited to <Node*>? > >> This won't work if T is not a derived class of Node, right? > > > > ::remove() takes a Node& so nodesToRemove can't have non-Node based objects. > > I see. But list has to contain only node based objects right? Why is the > template necessary? Correct. All incoming lists have to contain Node based object. However, while you can pass in an Element* when the function expects Node*, you can't pass in ListHashSet<Element*> for const ListHashSet<Node*>.
Nan Wang
Comment 8 2017-12-06 16:31:50 PST
(In reply to zalan from comment #7) > (In reply to Nan Wang from comment #6) > > Comment on attachment 328634 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=328634&action=review > > > > >>> Source/WebCore/accessibility/AXObjectCache.cpp:2729 > > >>> +static void filterForRemoval(const ListHashSet<T>& list, const Document& document, HashSet<Node*>& nodesToRemove) > > >> > > >> Why do we have a typename here for *list* but *nodesToRemove* is limited to <Node*>? > > >> This won't work if T is not a derived class of Node, right? > > > > > > ::remove() takes a Node& so nodesToRemove can't have non-Node based objects. > > > > I see. But list has to contain only node based objects right? Why is the > > template necessary? > Correct. All incoming lists have to contain Node based object. However, > while you can pass in an Element* when the function expects Node*, you can't > pass in ListHashSet<Element*> for const ListHashSet<Node*>. I thought it's possible to pass in ListHashSet that holds the derived class objects. But ok then.
WebKit Commit Bot
Comment 9 2017-12-06 16:55:05 PST
Comment on attachment 328634 [details] Patch Clearing flags on attachment: 328634 Committed r225613: <https://trac.webkit.org/changeset/225613>
WebKit Commit Bot
Comment 10 2017-12-06 16:55:06 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.