1370632 - Move LookupRemoveIf to nsBaseHashtable?

Status: RESOLVED FIXED

(Core :: XPCOM, enhancement)

Description

[Mats Palmgren (inactive)]

I wonder if we should move LookupRemoveIf() up to nsBaseHashtable
instead so that it can be used on other types of hashtables too?

I've made an example for nsDOMAttributeMap::DropAttribute, WIP patches
coming up...

Comment 1

[Mats Palmgren (inactive)]

Attachment: WIP part 1, this just moves the method up to the base class

Comment 2

[Mats Palmgren (inactive)]

Attachment: WIP part 2, use LookupRemoveIf() in nsDOMAttributeMap::DropAttribute to save one hashtable lookup

WDYT?

Comment 3

[Mats Palmgren (inactive)]

Looks like it would be easy to save a hashtable lookup in nsDOMAttributeMap::GetAttribute too
if we had a LookupForAdd... or perhaps GetOrInsert() can handle this case?
http://searchfox.org/mozilla-central/rev/d441cb24482c2e5448accaf07379445059937080/dom/base/nsDOMAttributeMap.cpp#138,143

Comment 4

[Nathan Froyd [:froydnj]]

Moving LookupRemoveIf to nsBaseHashtable makes sense to me.

We have LookupForAdd/LookupOrAdd in nsClassHashtable; we should see about porting those over to nsRefPtrHashtable and/or nsBaseHashtable, as appropriate.

Comment 5

[Mats Palmgren (inactive)]

Attachment: WIP 3, use-GetOrInsert-in-GetAttribute

This should work if mAttributeCache never contain null values.
Otherwise, we need something like LookupForAdd().

Comment 6

[Boris Zbarsky [:bzbarsky]]

mAttributeCache can't contain null values.

Comment 7

[Mats Palmgren (inactive)]

> mAttributeCache can't contain null values.

Great.  I'll do the DOM changes separately in bug 1370700.

Comment 8

[Mats Palmgren (inactive)]

Attachment: Move LookupRemoveIf() to nsBaseHashtable instead so that it can be used on more hashtables types

This is a verbatim copy, with "base_type::" removed.

Comment 9

[Nathan Froyd [:froydnj]]

Comment on attachment 8875034 [details] [diff] [review]
Move LookupRemoveIf() to nsBaseHashtable instead so that it can be used on more hashtables types

Review of attachment 8875034 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5c7be210a5c7
Move LookupRemoveIf() to nsBaseHashtable instead so that it can be used on more hashtables types.  r=froydnj

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/5c7be210a5c7