Closed Bug 921148 Opened 6 years ago Closed 6 years ago
xpcshell has compiled-in MOZ
_APP _NAME and other data which shouldn't be
I discovered while moving xpcshell into libxul that xpcshell on windows actually has compiled-in app-specific data such as MOZ_APP_NAME. This is a result of patches in bug 307181, but I'm not quite sure why it was necessary to build this logic into xpcshell itself rather than install a scripted dirserviceprovider to mock whatever update directories were necessary: http://hg.mozilla.org/mozilla-central/rev/c20d415ef1b5#l9.131 Ehsan/rstrong, do you remember why this needed to be native code? XRE_UPDATE_ROOT_DIR is the key we're replying for here; looking through the tests it doesn't seem they in general have any app-specific behaviors that would require us to use a specific update directory.
Well, in order for our xpcshell based tests to work correctly, we need to make sure that xpcshell-initiated tests put the updates and look for them in the same directory as the rest of the update system does. The reason I did this in the xpcshell.cpp code is that the shlwapi was easier to access from C++. I guess you could use js-ctypes to call them from JS, in which case you should be able to remove this code and move it to a mock directory service provider. Things were just easier this way...
I've been cleaning up the tests so I'll take this. Benjamin, how important is this?
Assignee: nobody → robert.bugzilla
This is not super-important, it's just a reverse build system dependency of the sort that usually makes us cry later by accident. It will lock block the "hack Firefox frontend without a compile environment" bug, which I can't find right now.
(In reply to comment #3) > This is not super-important, it's just a reverse build system dependency of the > sort that usually makes us cry later by accident. It will lock block the "hack > Firefox frontend without a compile environment" bug, which I can't find right > now. Sorry! :-)
Benjamin, would you like me to remove XRE_EXECUTABLE_FILE as well?
That doesn't appear to require any app-specific defines; apart from those I don't really care.
Passes tests and makes a couple of tests that needed to run synchronously run in parallel. Just need to clean the patch up.
Try push of the patch in progress https://tbpl.mozilla.org/?tree=Try&rev=601cfb6693a3
6 years ago
Depends on: 951662
I'll land this at the same time as the patches in bug 951662.
6 years ago
OS: Linux → Windows 7
6 years ago
Status: NEW → ASSIGNED
Attachment #8355365 - Flags: review?(netzen) → review+
Pushed to mozilla-inbound https://hg.mozilla.org/integration/mozilla-inbound/rev/3f4af26d0535 There are tests in bug 951662
Target Milestone: --- → mozilla29
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.