Closed Bug 1231964 Opened 4 years ago Closed 4 years ago
Move CC participant code that touches JS out of mozglue
For some reason, smaug's patches in bug 1120016 cause the various JS Heap API calls in nsCycleCollectionParticipant.cpp to trigger linking errors. It makes sense that those linking errors would occur, because nsCCParticipant.cpp is part of mozglue, which is used outside of xul.dll, but I have no idea why these errors are happening now. Copying these methods outside of that file seems to help. Things outside of xul should not be keeping alive JS anyways, so this should be ok. Ideally I'd split nsCycleCollectionParticipant.h into a separate one for things that use JS, but that would require changing too many includes so I'll create a new .cpp file in xpcom/base/.
My simplest patch basically just moves as much as possible out of xpcom/glue. You can't move too much out, or you get other link errors. Here's a version that builds on Windows: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c6039e75c41e Here's a full platform try run to see if that works on all platforms: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d45283f0d20d Here's a more invasive version that tries to move all of the stuff from nsCycleCollectionParticipant.h related to JS tracing into a new header: https://treeherder.mozilla.org/#/jobs?repo=try&revision=437369d94fc1 That seems cleaner from a conceptual level. The other version only works because it carefully relies on people not using the JS tracing stuff, I think. But in practice, the new header is included in nsWrapperCache.h, so it may be getting included into most files anyways. I'm not sure which is better.
I also did a full L64 debug try run with the second version, so it looks like the event wrapper stuff at least kind of works: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4351195d5eeb I'll upload a patch shortly after I confirm that it still builds.
The JS engine does not export symbols outside of XUL, so having these defined inside mozglue apparently causes linking errors on some platforms with the patches in bug 1120016. This patch moves enough methods outside of mozglue that the patch in that other bug will still link on all platforms, without moving so much out that there are other linking errors.
Attachment #8699550 - Flags: review?(bugs)
Comment on attachment 8699550 [details] [diff] [review] Move CC participant code that touches JS out of mozglue. Thanks a ton for fixing this mess, well at least making it to link :)
Attachment #8699550 - Flags: review?(bugs) → review+
You need to log in before you can comment on or make changes to this bug.