With the removal of the XPCOM glue, the external string API is dead. We need to remove it. I intend to: * remove nsStringAPI.h and nsStringAPI.cpp * remove nsStringGlue.h and change current includers to nsString.h * remove nsEmbedString.h and change current includers to nsString.h * remove nsProfileStringTypes.h and fixup callsites This is currently blocked on some webrtc refactoring in bug 1332622

Benjamin, you removed #include "nsStringGlue.h" from browser/app/nsBrowserApp.cpp. When I do the same in Thunderbird's "main" program, which is 99% the same, I get a compile error on Windwos: c:\mozilla-source\comm-central\mozilla\xpcom\build\BinaryPath.h(185): error C3861: 'nsDependentString': identifier not found coming from #ifdef XP_WIN rv = NS_NewLocalFile(nsDependentString(exePath), true, Have you compiled your patches on Windows? BTW, if I change the include to nsString.h I get: nsStringFwd.h(15): fatal error C1189: #error: Internal string headers are not available from external-linkage code.

Forwarded the RDF bits from me to glandium, as that change is really build config, IMHO.

I have a windows try run going, but I did include a set of #ifdefs in BinaryPath.h that should cover this case.

I can't see your try run. Are you saying the BinaryPath.h in the try doesn't match your patch here?

The only NS_NewLocalFile in BinaryPath.h is now in a MOZILLA_INTERNAL_API block. So I guess you're missing something from the patch stack?

Comment on attachment 8841672 [details] Bug 1332639 - Remove unneeded #include so that this file builds, https://reviewboard.mozilla.org/r/115830/#review117258 LGTM, I've double-checked the commit that introduced that inclusion (e96c297c8723) and it seems that it was already unused back then.

(In reply to Benjamin Smedberg [:bsmedberg] from comment #11) > The only NS_NewLocalFile in BinaryPath.h is now in a MOZILLA_INTERNAL_API > block. So I guess you're missing something from the patch stack? Thanks. I haven't applied your patches. I'm just preparing C-C by changing nsStringGlue.h to nsString.h. I hadn't noticed the newly added MOZILLA_INTERNAL_API in BinaryPath.h. So I guess I'll complete the TB work when this has landed.

Comment on attachment 8841670 [details] Bug 1332639 - Remove the external unicharutil library which isn't used and is now unlinkable, https://reviewboard.mozilla.org/r/115826/#review117286 Bug 1114997 sucks, but OK.

Comment on attachment 8841669 [details] Bug 1332639 - Stop building the media/mtransport/testlib library which isn't used, https://reviewboard.mozilla.org/r/115824/#review117386 lgtm

Comment on attachment 8841667 [details] Bug 1332639 - Remove the external string API: nsStringAPI.h/cpp and nsEmbedString.h, https://reviewboard.mozilla.org/r/115820/#review117678 Note, it seems to me this should be last (or close to last) in the patch queue, not first. ::: netwerk/base/nsNetUtil.h:59 (Diff revision 1) > namespace mozilla { class OriginAttributes; } > > template <class> class nsCOMPtr; > template <typename> struct already_AddRefed; > > #ifdef MOZILLA_INTERNAL_API Without the external string API, the file obviously doesn't build without MOZILLA_INTERNAL_API anymore. Seems like the non MOZILLA_INTERNAL_API branch should be removed in a followup. ::: xpcom/glue/nsStringGlue.h:18 (Diff revision 1) > > #ifndef nsStringGlue_h__ > #define nsStringGlue_h__ > > -#ifdef MOZILLA_INTERNAL_API > +#ifndef MOZILLA_INTERNAL_API > +#error "Everything using XPCOM strings must be compiled with MOZILLA_INTERNAL_API." Maybe "Using XPCOM strings is limited to libxul" would be better than kind of implying that one can work around the limitation with a dubious define.

Comment on attachment 8841668 [details] Bug 1332639 - Remove the RDF "standalone" library which isn't used, https://reviewboard.mozilla.org/r/115822/#review117684

Comment on attachment 8841671 [details] Bug 1332639 - Fix nsBrowserApp.cpp and related headers to compile without the external string API, https://reviewboard.mozilla.org/r/115828/#review117692

Pushed by bsmedberg@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/1bcada51f54b Remove unneeded #include so that this file builds, r=gsvelto https://hg.mozilla.org/integration/mozilla-inbound/rev/9825f34adc3d Remove the RDF "standalone" library which isn't used, r=glandium https://hg.mozilla.org/integration/mozilla-inbound/rev/ab3684fe0488 Stop building the media/mtransport/testlib library which isn't used, r=dminor https://hg.mozilla.org/integration/mozilla-inbound/rev/e761b88de9de Remove the external unicharutil library which isn't used and is now unlinkable, r=emk https://hg.mozilla.org/integration/mozilla-inbound/rev/cae4a255be3f Fix nsBrowserApp.cpp and related headers to compile without the external string API, r=glandium https://hg.mozilla.org/integration/mozilla-inbound/rev/a8f45b2afbf3 Remove the external string API: nsStringAPI.h/cpp and nsEmbedString.h, r=glandium

(In reply to Mike Hommey [:glandium] from comment #17) > Comment on attachment 8841668 [details] > Bug 1332639 - Remove the RDF "standalone" library which isn't used, > > https://reviewboard.mozilla.org/r/115822/#review117684 Well, it's used in comm-central...

I don't know how it could be; it doesn't actually link without the string API which hasn't been built since we removed the XPCOM glue.

Right, the standalone API. I misinterpreted it as nsRDFResource was going away.

No it just lives in libxul.

(In reply to Magnus Melin from comment #20) > (In reply to Mike Hommey [:glandium] from comment #17) > > Comment on attachment 8841668 [details] > > Bug 1332639 - Remove the RDF "standalone" library which isn't used, > > > > https://reviewboard.mozilla.org/r/115822/#review117684 > > Well, it's used in comm-central... was. past tense. Removed in bug 1318966

https://hg.mozilla.org/mozilla-central/rev/1bcada51f54b https://hg.mozilla.org/mozilla-central/rev/9825f34adc3d https://hg.mozilla.org/mozilla-central/rev/ab3684fe0488 https://hg.mozilla.org/mozilla-central/rev/e761b88de9de https://hg.mozilla.org/mozilla-central/rev/cae4a255be3f https://hg.mozilla.org/mozilla-central/rev/a8f45b2afbf3

Is it deliberate that nsStringAPI.* are still present in the repo?

I noticed that nsString.h includes nsReadableUtils.h, so it's not required in nsReadableUtils.h.

dmajor no! I thought I had hg rm'ed it but apparently not, I'll do that.

Pushed by bsmedberg@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/d60dce18350a followup. I apparently didn't actually remove the nsStringAPI.* files, but that was the intention, r=oops