Closed
Bug 942528
Opened 11 years ago
Closed 11 years ago
Make XPCWrappedNative into a more normal CCed class
Categories
(Core :: XPConnect, defect)
Core
XPConnect
Tracking
()
RESOLVED
FIXED
mozilla28
People
(Reporter: mccr8, Assigned: mccr8)
References
Details
(Whiteboard: [qa-])
Attachments
(1 file, 1 obsolete file)
5.15 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
PCWN and XPCWJS are the only two remaining CCed C++ things that don't use the purple buffer (and thus snow white). I'm abusing the purple buffer for ICC, so right now I have to reinvent these mechanisms, poorly, for XPCWN and XPCWJS. But maybe I can just make these classes into normal CCed classes, and then I wouldn't have to do anything more, while at the same time reducing overall weirdness. I have some patches that sort of work, but not entirely.
Assignee | ||
Comment 1•11 years ago
|
||
The XPCWN stuff is fairly green. I hit some crashiness that might be fixable by immediately removing the XPCWN from the global table when it hits a refcount of 0. There's also a problem that looks like my patch is tripping some kind of lock detection when we run about:memory. https://tbpl.mozilla.org/?tree=Try&rev=456ae933a061 XPCWJS is stable enough to run the browser for a bit, but it fails at least one test ( content/base/test/chrome/test_bug549682.xul ) which looks like it involves a weakly-held XPCWJS going away too quickly, which I could believe. There may be other problems, too. I don't understand why XPCWJS has to addref itself twice for weak references. That code is from 2001, so who knows if it is really needed. Though if you just remove it, bad things happen, I discovered.
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → continuation
Assignee | ||
Comment 2•11 years ago
|
||
If I'm reading this right, weak references to WPCJS are actually strong, due to the extra addref that only gets dropped when there are no weak references.
Assignee | ||
Comment 3•11 years ago
|
||
I have a patch that works for XPCWN, though it does NS_IMPL_CYCLE_COLLECTING_RELEASE_WITH_LAST_RELEASE(XPCWrappedNative, Destroy()) which is kind of cheating... https://tbpl.mozilla.org/?tree=Try&rev=546c6a77723a
Assignee | ||
Comment 4•11 years ago
|
||
Assignee | ||
Updated•11 years ago
|
Attachment #8337335 -
Attachment is obsolete: true
Assignee | ||
Comment 5•11 years ago
|
||
Try run looks green: https://tbpl.mozilla.org/?tree=Try&rev=23fbf78872d2 I'm going to split XPCWJS into bug 944492.
Summary: Investigate making XPCWrappedNative and nsXPCWrappedJS normal CCed classes → Make XPCWrappedNative into a more normal CCed classes
Assignee | ||
Updated•11 years ago
|
Attachment #8340086 -
Flags: review?(bugs)
Assignee | ||
Updated•11 years ago
|
Summary: Make XPCWrappedNative into a more normal CCed classes → Make XPCWrappedNative into a more normal CCed class
Comment 6•11 years ago
|
||
Comment on attachment 8340086 [details] [diff] [review] Make XPCWN more of a normal cycle collected class. r=smaug >+NS_IMPL_CYCLE_COLLECTING_RELEASE_WITH_LAST_RELEASE(XPCWrappedNative, Destroy()) Could you add a comment here why we want _LAST_RELEASE + Destroy()
Attachment #8340086 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 7•11 years ago
|
||
Comment added. https://hg.mozilla.org/integration/mozilla-inbound/rev/f7330220c26d
Comment 8•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f7330220c26d
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Updated•10 years ago
|
Whiteboard: [qa-]
You need to log in
before you can comment on or make changes to this bug.
Description
•