Once we don't need the glue for the Firefox stub, we should stop building it. This is currently blocked on bug 1306237.

Comment on attachment 8809094 [details] Bug 1306329 part A - nsUTF8Utils is used from outside libxul, and NS_WARNING now only works from within libxul, so make its usage conditional, https://reviewboard.mozilla.org/r/91742/#review91670 Minor preference to `#undef UTF8UTILS_WARNING` at the end of the header, but I suppose it doesn't matter too much.

Comment on attachment 8811001 [details] Bug 1306329 part B - Remove a few crashreporter tests which can no longer be implemented without linking to internal symbols. https://reviewboard.mozilla.org/r/93254/#review93238 ::: toolkit/crashreporter/test/nsTestCrasher.cpp (Diff revision 1) > PureVirtualCall(); > // not reached > break; > } > - case CRASH_RUNTIMEABORT: { > - NS_RUNTIMEABORT("Intentional crash"); We could probably replace this with just calling `nsIDebug2::abort` from JS, right? I'm not sure if that buys us much over the `MOZ_CRASH` case.

Comment on attachment 8811002 [details] Bug 1306329 part C - things that depend on xul should no longer link the XPCOM glue library, https://reviewboard.mozilla.org/r/93256/#review95156 Until comm-central and b2g are ready to not use the xpcom glue, this should be behind some flag (NO_XPCOM_GLUE?)

Comment on attachment 8811004 [details] Bug 1306329 part E - Don't build the dependent XPCOM glue. https://reviewboard.mozilla.org/r/93260/#review95158 Same comment as part D. xpcom/glue/staticruntime should also receive the same treatment.

Comment on attachment 8811005 [details] Bug 1306329 part F - Stop exporting XPCOM and XUL symbols, https://reviewboard.mozilla.org/r/93262/#review95160 Same comment as part D and E.

Pushed by bsmedberg@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/77f704be0a6d part A - nsUTF8Utils is used from outside libxul, and NS_WARNING now only works from within libxul, so make its usage conditional, r=froydnj https://hg.mozilla.org/integration/mozilla-inbound/rev/1ba830898e8c part B - Remove a few crashreporter tests which can no longer be implemented without linking to internal symbols. r=ted

I apologize for the spam; reviewboard totally didn't do what I expected when adding a part on the end.

Comment on attachment 8818292 [details] Bug 1306329 part G - Disable two webrtc tests because they link the XPCOM glue which no longer exists, https://reviewboard.mozilla.org/r/98406/#review98664

Should these patches update ALLOWED_XPCOM_GLUE in python/mozbuild/mozbuild/frontend/emitter.py?

(In reply to Nicholas Nethercote [:njn] from comment #28) > Should these patches update ALLOWED_XPCOM_GLUE in > python/mozbuild/mozbuild/frontend/emitter.py? Yes or this will cause a build unit test failure, sorry I missed that in the review.

https://hg.mozilla.org/mozilla-central/rev/77f704be0a6d https://hg.mozilla.org/mozilla-central/rev/1ba830898e8c

Comment on attachment 8818295 [details] Bug 1306329 part D - Remove unneeded nsXPCOMGlue includes, https://reviewboard.mozilla.org/r/98410/#review99050 This part is needed *before* bug 1306327, so this can't be in this bug.

(In reply to Mike Hommey [:glandium] from comment #31) > Comment on attachment 8818295 [details] > Bug 1306329 part D - Remove unneeded nsXPCOMGlue includes, > > https://reviewboard.mozilla.org/r/98410/#review99050 > > This part is needed *before* bug 1306327, so this can't be in this bug. Landed in bug 1329932.

Comment on attachment 8818294 [details] Bug 1306329 part C - things that depend on xul should no longer link the XPCOM glue library, https://reviewboard.mozilla.org/r/98408/#review104328 This patch requires the ALLOWED_XPCOM_GLUE stuff in emitter.py to be removed first, otherwise, the build fails during spline reticulation. ::: build/gecko_templates.mozbuild:29 (Diff revision 1) > - xpcomglue = 'xpcomglue' > + pass > elif msvcrt == 'static': > USE_STATIC_LIBS = True > - xpcomglue = 'xpcomglue_staticruntime' This should be kept. While we may not have binaries linked to the static CRT at the moment, that might happen at any time, and we need to keep that working.

Comment on attachment 8818296 [details] Bug 1306329 part E - Don't build the dependent XPCOM glue. https://reviewboard.mozilla.org/r/98412/#review104334 ::: xpcom/glue/moz.build:15 (Diff revision 1) > DIRS += ['standalone'] > > # On win we build two glue libs - glue linked to crt dlls here and in staticruntime we build > # a statically linked glue lib. > if CONFIG['OS_ARCH'] == 'WINNT': > DIRS += ['staticruntime'] We don't need the static runtime version of the dependent xpcom glue anymore, you can remove this line and the corresponding directory. ::: xpcom/glue/moz.build (Diff revision 1) > 'Mutex.h', > 'Observer.h', > 'ReentrantMonitor.h', > ] > > -include('objs.mozbuild') After this, nothing uses objs.mozbuild but xpcom/build/moz.build. We should move its contents there (and, in fact, probably move the files too). But that can be done in a followup. ::: xpcom/glue/moz.build:87 (Diff revision 1) > -] > - > -# Force to build a static library only > -NO_EXPAND_LIBS = True > - > DIST_INSTALL = True This line can be removed too,

Comment on attachment 8818297 [details] Bug 1306329 part F - Stop exporting XPCOM and XUL symbols, https://reviewboard.mozilla.org/r/98414/#review104344 Most changes under xpcom are in my patch queue in bug 1306327, although some of the cleanup in nsXPCOMPrivate.h isn't, but I'm going to add that there. That leaves the changes to nscore.h and xrecore.h, along sdp_file_parser.cpp and xptcall.h.

> This patch requires the ALLOWED_XPCOM_GLUE stuff in emitter.py to be removed > first, otherwise, the build fails during spline reticulation. Indeed, I have a backout of all that.

Comment on attachment 8826768 [details] Bug 1306329 - Backout 621aa115c3df (bug 1316450). https://reviewboard.mozilla.org/r/104640/#review105384

Comment on attachment 8826769 [details] Bug 1306329 - Things that depend on xul should no longer link the XPCOM glue library. https://reviewboard.mozilla.org/r/104642/#review105386

Comment on attachment 8826770 [details] Bug 1306329 - Don't build the dependent XPCOM glue. https://reviewboard.mozilla.org/r/104644/#review105388

Comment on attachment 8826771 [details] Bug 1306329 - Stop exporting XPCOM and XUL symbols. https://reviewboard.mozilla.org/r/104646/#review105390

Pushed by mh@glandium.org: https://hg.mozilla.org/integration/autoland/rev/ad4e531c7070 Backout 621aa115c3df (bug 1316450). r=glandium https://hg.mozilla.org/integration/autoland/rev/377ca1419f1a Things that depend on xul should no longer link the XPCOM glue library. r=glandium https://hg.mozilla.org/integration/autoland/rev/6bb17b9a62d8 Don't build the dependent XPCOM glue. r=glandium https://hg.mozilla.org/integration/autoland/rev/1c2f51ce3faf Stop exporting XPCOM and XUL symbols. r=glandium

I had to back those out in https://hg.mozilla.org/integration/autoland/rev/93ffc3c44ee31192f6080f37d82994ca348e4148 for hazard build failures: https://treeherder.mozilla.org/logviewer.html#?job_id=69737255&repo=autoland

How does removing code add hazards?

My guess here is that changing the XPCOM functions from C to C++ messed up these exceptions: http://searchfox.org/mozilla-central/rev/790b2cb423ea1ecb5746722d51633caec9bab95a/js/src/devtools/rootAnalysis/annotations.js#179 Unfortunately I don't see any docs on how to set up this build locally; https://developer.mozilla.org/en-US/docs/Mozilla/Continuous_integration links to a bug but without instructions. I might try naively changing these to the equivalent C++ name from c++filt and see if that works on try.

I pushed a version of the last patch that keeps the extern "C" (while still getting rid of the export). If that works, I'll push that and figure out the rest later. It isn't critical to getting rid of the exports. https://treeherder.mozilla.org/#/jobs?repo=try&revision=58c5425a3627cc6a576908efdfb73fa6a616b877

Or https://treeherder.mozilla.org/#/jobs?repo=try&revision=010d86f90a6fb9480ab2d31ae1778bf45f741f89

Pushed by mh@glandium.org: https://hg.mozilla.org/integration/autoland/rev/8f4893e390c3 Backout 621aa115c3df (bug 1316450). r=glandium https://hg.mozilla.org/integration/autoland/rev/15c053fffd35 Things that depend on xul should no longer link the XPCOM glue library. r=glandium https://hg.mozilla.org/integration/autoland/rev/65fcf5e91cf0 Don't build the dependent XPCOM glue. r=glandium https://hg.mozilla.org/integration/autoland/rev/a5aa102e8f0f Stop exporting XPCOM and XUL symbols. r=glandium

https://hg.mozilla.org/mozilla-central/rev/15c053fffd35 https://hg.mozilla.org/mozilla-central/rev/65fcf5e91cf0 https://hg.mozilla.org/mozilla-central/rev/a5aa102e8f0f

Pushed by bsmedberg@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/1d62adf74d89 Backout 621aa115c3df (bug 1316450). r=glandium https://hg.mozilla.org/integration/mozilla-inbound/rev/28ca2b49e4ca Things that depend on xul should no longer link the XPCOM glue library. r=glandium https://hg.mozilla.org/integration/mozilla-inbound/rev/2dc53f232f8b Don't build the dependent XPCOM glue. r=glandium https://hg.mozilla.org/integration/mozilla-inbound/rev/5abe47a8bfe0 Stop exporting XPCOM and XUL symbols. r=glandium

https://hg.mozilla.org/mozilla-central/rev/28ca2b49e4ca https://hg.mozilla.org/mozilla-central/rev/2dc53f232f8b https://hg.mozilla.org/mozilla-central/rev/5abe47a8bfe0

This fixing of this bug apparently causes Firefox 53 to crash whenever old extensions, which are not prevented from loading, invoke any of the removed symbols. Here is a typical crash report: https://crash-stats.mozilla.com/report/index/bp-9b566232-4dd8-448d-90a7-855cf2170407 The crash is very easy to understand. It occurs because the XPCOM dylib in my old extension calls _NS_GetServiceManager. Because there is no such symbol in Firefox 53, Firefox crashes. STEPS TO REPRODUCE: • Get on a Mac running macOS 10.12. • Download and install BookMacster version 2.2.18 from here: https://sheepsystems.com/bookmacster/BookMacster.2.2.18.zip (Do not get it from the link given on my website because I hope to soon have it replaced with a newer version 2.3 which contains a new WebExtension instead.) • Launch Firefox 53.0b9 (64-bit) Build ID 20170403072723. • Launch BookMacster. Close or cancel any windows that open. • Click in the menu: BookMacster > Manage Browser Extensions. A window will open. • In the top "Syncing" section, row for "Firefox", click button "Install", and follow the instructions given. • Relaunch Firefox 53.0b9. • Click in the menu Tools > Add-Ons and verify that you have Sheep Systems Sync Extension version 330 loaded. • Return to the Manage Browser Extensions window in BookMacster and click "Test" in the row for Firefox. This sends a message to the .dylib in my extension. RESULT: Firefox 53 will crash immediately. Now, of course the way it will happen in real life is that users who have been running version 2.2.18 of BookMacster or similar apps, and have installed my old extension, and been happily using it in Firefox 52 or earlier, and have not updated BookMacster before the week of April 18 because they have automatic updating turned off, will find that, after Firefox updates itself to version 53, Firefox will crash whenever they add, delete or change a bookmark in Firefox, or perform certain operations in my app. Similar crashes can be expected to occur with any other extensions still out in the wild which have XPCOM dylibs. Please note that there is apparently nothing in Firefox 53 which prevents my old extension from loading. The install.rdf in my old extension specifies the maxVersion for Firefox = 51.0, but I read somewhere that Firefox has never enforced this :(

Jerry, can you file a new bug for this issue (make it block this one). I would have expected the extension not to load at all. How come it still loads if there are missing symbols? Are they weakly linked?

OK, Mike. I have now filed Bug 1354395, and added a comment there regarding the weak linking question.