SpecialPowers wrapper should preserve identity

REOPENED
Unassigned

Status

REOPENED
7 years ago
3 years ago

People

(Reporter: bholley, Unassigned)

Tracking

Trunk
mozilla13
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

7 years ago
See bug 702353 comment 10. We can't do this reliably until we fix bug 673468.
(Reporter)

Comment 1

7 years ago
Created attachment 589073 [details] [diff] [review]
patch v1

Attaching the patch for posterity. Should be ready to land once weakmaps do the right thing in the wrapper case.
(Reporter)

Updated

7 years ago
Attachment #589073 - Attachment is patch: true
(Reporter)

Comment 2

7 years ago
Comment on attachment 589073 [details] [diff] [review]
patch v1

Looks like the dependent bug was fixed. Flagging mrbkap for review.
Attachment #589073 - Flags: review?(mrbkap)
Attachment #589073 - Flags: review?(mrbkap) → review+
(Reporter)

Comment 4

7 years ago
Looks good - pushed to m-i:

http://hg.mozilla.org/integration/mozilla-inbound/rev/f9145dab4be9
Assignee: nobody → bobbyholley+bmo
Flags: in-testsuite+
Target Milestone: --- → mozilla13

Comment 5

7 years ago
https://hg.mozilla.org/mozilla-central/rev/f9145dab4be9
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Depends on: 731442
Version: unspecified → Trunk
(Reporter)

Comment 6

7 years ago
Backed out due to bug 731442:

http://hg.mozilla.org/mozilla-central/rev/75fcd465d506
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
I don't know if this bug or the other bug is the place to discuss that, but basically the problem is that only native classes that are wrapper caches that preserve their wrapper can be used as weak map keys.

The warning in question is at this line:
http://mxr.mozilla.org/mozilla-central/source/js/src/jsweakmap.cpp#292

The actual callback function is here:
http://mxr.mozilla.org/mozilla-central/source/js/xpconnect/src/XPCJSRuntime.cpp#1996

Notice that we don't actually allow all wrapper cache classes as weak map keys.  This is because there are some wrapper caches that don't preserve their wrapper. Right now, we only allow nsINodes because that seemed like the most common thing people would use.

Updated

6 years ago
Blocks: 762528
(Reporter)

Updated

3 years ago
Assignee: bobbyholley → nobody
You need to log in before you can comment on or make changes to this bug.