1336086 - don't export mozilla::services::_external_Get*Service*

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Nathan Froyd [:froydnj]]

More symbols that we don't need to export.

Comment 1

[Nathan Froyd [:froydnj]]

CC'ing TB people to let them know this is going to break things.

Comment 2

[Jorg K (CEST = GMT+2)]

Thanks for the heads-up. Can you be more specific? What is going to break?

Comment 3

[Nathan Froyd [:froydnj]]

I don't know what's going to break because I don't know TB specifics.  Bug 714967 covers the addition of these symbols, and it sounds like there was some build config where TB code linked to libxul rather than being linked inside of libxul (?).  That sort of configuration would break with these changes (if it hasn't already broken because of other export removals).

Comment 4

[Nathan Froyd [:froydnj]]

Attachment: remove mozilla::services::_external_Get*Service

Removing the exported symbols is straightforward enough.

The only wrinkle is that PeerConnectionCtx.cpp gets compiled with and
without MOZILLA_INTERNAL_API.  When compiling without
MOZILLA_INTERNAL_API, mozilla::services::Get*Service was redirected to
the _external_* symbol variants.  But as the _external_* symbols no
longer exist, PeerConnectionCtx.cpp's code no longer worked.

Fortunately, PeerConnectionCtx.cpp already contains a few #ifdef
MOZILLA_INTERNAL_API blocks to handle internal/external compilation;
fixing this newest issue was just a matter of extending existing blocks
and adding new ones.  The key observation is that we never added any
observers when compiling without MOZILLA_INTERNAL_API, so we can #ifdef
out the removals of observers, as those would have no effect, and simply
skip getting the observer service for the observer additions if we're
compiling without MOZILLA_INTERNAL_API.

Comment 5

[Nathan Froyd [:froydnj]]

[Tracking Requested - why for this release]:

We removed some symbols from export in 53:

https://groups.google.com/forum/#!msg/mozilla.dev.platform/qB-9gJl9Jgs/O8LhMg1dBwAJ

this bug would remove more of them.  It seems better to have one release where we remove as many as possible all at once, rather than removing some in one release, some in another, and more in another.

Comment 6

[Benjamin Smedberg]

We should probably uplift this to 53.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b7e4323f4e73
remove mozilla::services::_external_Get*Service; r=bsmedberg,jesup

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/b7e4323f4e73

Comment 9

[Nathan Froyd [:froydnj]]

Comment on attachment 8833063 [details] [diff] [review]
remove mozilla::services::_external_Get*Service

Approval Request Comment
[Feature/Bug causing the regression]: Not applicable.  (comment 5 has some context)
[User impact if declined]: More symbols exported from libxul, which might be used in all sorts of peculiar ways.
[Is this code covered by automated tests?]: No.
[Has the fix been verified in Nightly?]: Yes.  (Me, manually)
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: We are unexporting symbols which people shouldn't be using in the first place.
[String changes made/needed]: None.

Comment 10

[Gerry Chang [:gchang]]

Comment on attachment 8833063 [details] [diff] [review]
remove mozilla::services::_external_Get*Service

Remove symbols from 53. Aurora53+.

Comment 11

[Gerry Chang [:gchang]]

Track 53+ as the symbols are removed from 53.

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/3d0d4b016285