Bug 180503 – Remove nodes from AXObjectCache when the associated subframe document is getting destroyed.

• Status: RESOLVED
• Resolution: FIXED
• Component: Accessibility

[zalan]

rdar://problem/35891328

[zalan]

Created attachment 328632 [details]
Patch

[chris fleizach]

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)

[zalan]

Created attachment 328634 [details]
Patch

[Nan Wang]

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?

[zalan]

(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 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?

[zalan]

(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]

(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 on attachment 328634 [details]
Patch

Clearing flags on attachment: 328634

Committed r225613: <https://trac.webkit.org/changeset/225613>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.