728071 - application.ini data should be in libmozglue.so, not libxul.so

Status: RESOLVED FIXED

(Core :: mozglue, defect)

Comment 1

[Mike Hommey [:glandium]]

It's actually a feature, because nsAndroidStartup.cpp is where the buildid is hardcoded. Rebuilding changes the buildid, thus changing nsAndroidStartup.cpp. The actual problem is that the buildid is in nsAndroidStartup, or more precisely, that it is in libxul.so. It should be in libmozglue.so.

Comment 2

[Mike Hommey [:glandium]]

Attachment: part 1 - Move nsXREAppData definition in a separate header, and use it from application.ini.h

This is required for the upcoming second part, which moves the definition of the nsXREAppData for mobile from within libxul.so to libmozglue.so. But since the latter is built almost first, a lot of the includes pulled from nsXULAppAPI.h are not available in dist/include (which would make the required -I list awfully long), or not even generated (prcpucfg.h ; this one is overridable, but it's imho pretty awful).

Comment 3

[Mike Hommey [:glandium]]

Attachment: part 2 - Include application.ini.h in APKOpen.cpp instead of nsAndroidStartup.cpp

Comment 4

[Mike Hommey [:glandium]]

(In reply to Mike Hommey [:glandium] from comment #2)
> Created attachment 598825 [details] [diff] [review]
> part 1 - Move nsXREAppData definition in a separate header, and use it from
> application.ini.h
> 
> This is required for the upcoming second part, which moves the definition of
> the nsXREAppData for mobile from within libxul.so to libmozglue.so. But
> since the latter is built almost first, a lot of the includes pulled from
> nsXULAppAPI.h are not available in dist/include (which would make the
> required -I list awfully long), or not even generated (prcpucfg.h ; this one
> is overridable, but it's imho pretty awful).

Also note the patch replaces "PRUint32" with "unsigned int" in the struct definition, because we can't rely on prtypes.h, which includes prcpucfg.h.

Comment 5

[Benjamin Smedberg]

Comment on attachment 598825 [details] [diff] [review]
part 1 - Move nsXREAppData definition in a separate header, and use it from application.ini.h

1) please fix my email address to [e-mail] in the license header while you're touching this

2) let's use uint32_t and mozilla/StdInt.h to get known-size values

3) please "#ifndef nsXREAppData_h" without the leading/trailing underscores, which are technically reserved

r=me with those changes. Note that I only want the compiled nsXREAppData struct in libmozglue.so on Android, since we should really put it in the Firefox binary on other platforms.

Comment 6

[Mike Hommey [:glandium]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0e2ee581bf93
https://hg.mozilla.org/integration/mozilla-inbound/rev/9eaacb130b4c

Comment 7

[Mike Hommey [:glandium]]

It looks like this is not enough :(

Comment 8

[Mike Hommey [:glandium]]

Attachment: part 3 - Don't rebuild nsAndroidStartup.o when buildid changes

This is the missing part. Since nsAndroidStartup.cpp doesn't contain the buildid-dependent data anymore, no need for this dependency.

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/0e2ee581bf93
https://hg.mozilla.org/mozilla-central/rev/9eaacb130b4c

Comment 10

[Mike Hommey [:glandium]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8fa3f5864c20

Comment 11

[Richard Newman [:rnewman]]

https://hg.mozilla.org/mozilla-central/rev/8fa3f5864c20