Closed Bug 1165768 Opened 4 years ago Closed 4 years ago

Convert all static |PLDHashTable| variables to |PLDHashTable2|

Categories

(Core :: General, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed

People

(Reporter: njn, Assigned: njn)

References

Details

Attachments

(7 files)

This bug is about redoing part 2.5 of bug 1161377, though in a slightly different way. In particular, avoiding the regression in plug-in handling that showed up as bug 1165155.
Depends on: 1165770
This involves removing the last uses of PLDHashTable::SetOps(), which is a
nasty encapsulation-breaking hack. Hooray!
Attachment #8606884 - Flags: review?(nfroyd)
Comment on attachment 8606884 [details] [diff] [review]
(part 1) - Convert |sNPObjWrappers| to |PLDHashTable2*|

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

::: dom/plugins/base/nsJSNPRuntime.cpp
@@ +1970,5 @@
>    if (entry->mNpp == nppcx->npp) {
> +    // HACK: temporarily hide the hash we're enumerating so that invalidate()
> +    // and deallocate() don't touch it.
> +    PLDHashTable2 *tmp = static_cast<PLDHashTable2*>(table);
> +    sNPObjWrappers = nullptr;

Kinda tempted to say we should use mozilla::AutoRestore here.  Followup?

@@ +1998,5 @@
>      }
>  
>      ::JS_SetPrivate(entry->mJSObj, nullptr);
>  
> +    sNPObjWrappers = tmp;

Should we MOZ_ASSERT(!sNPObjWrappers) here?
Attachment #8606884 - Flags: review?(nfroyd) → review+
Attachment #8606886 - Flags: review?(nfroyd) → review+
Attachment #8606887 - Flags: review?(nfroyd) → review+
Attachment #8606888 - Flags: review?(nfroyd) → review+
Attachment #8606889 - Flags: review?(nfroyd) → review+
Attachment #8606890 - Flags: review?(nfroyd) → review+
Comment on attachment 8606891 [details] [diff] [review]
(part 7) - Convert |gHashTable| to |PLDHashTable2*|

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

I am essentially rubber-stamping these, as I believe them to be quite similar to patches that I have already r+'d.  If that's not the case, please speak up now. :)
Attachment #8606891 - Flags: review?(nfroyd) → review+
> I am essentially rubber-stamping these, as I believe them to be quite
> similar to patches that I have already r+'d.  If that's not the case, please
> speak up now. :)

The only difference is that the variable declarations and the |new| statements use PLDHashTable2 instead of PLDHashTable.
You need to log in before you can comment on or make changes to this bug.