Open Bug 718543 Opened 9 years ago Updated 5 years ago

SpecialPowers wrapper should preserve identity

Categories

(Testing :: Mochitest, defect)

defect
Not set
normal

Tracking

(Not tracked)

REOPENED
mozilla13

People

(Reporter: bholley, Unassigned)

References

Details

Attachments

(1 file)

See bug 702353 comment 10. We can't do this reliably until we fix bug 673468.
Attached patch patch v1Splinter Review
Attaching the patch for posterity. Should be ready to land once weakmaps do the right thing in the wrapper case.
Attachment #589073 - Attachment is patch: true
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+
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
https://hg.mozilla.org/mozilla-central/rev/f9145dab4be9
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Depends on: 731442
Version: unspecified → Trunk
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.
Blocks: 762528
Assignee: bobbyholley → nobody
You need to log in before you can comment on or make changes to this bug.