1231964 - Move CC participant code that touches JS out of mozglue

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Andrew McCreight [:mccr8]]

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/.

Comment 1

[Andrew McCreight [:mccr8]]

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.

Comment 2

[Andrew McCreight [:mccr8]]

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.

Comment 3

[Andrew McCreight [:mccr8]]

Attachment: Move CC participant code that touches JS out of mozglue.

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.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

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 :)

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/93179299ab59

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/93179299ab59