Closed Bug 899857 Opened 7 years ago Closed 6 years ago
DL provider lists shared plugin directory first
[Context] We are working on getting the xpcshell tests to run concurrently. In order to achieve this, we changed the xpcshell harness to give every test a plugins directory (bugs 892121 and 890098). The harness specifies this path to xpcshell using the "-p" cmdline argument. The problem that we encounter is that (at least sometimes) the shared (i.e. $appdir/plugins) folder ends up being the first one in the APluginsDL enumeration, which makes some tests use that folder instead of their own for plugin handling. The xpcshell (cpp land) adds the $appdir/plugins path on its own, regardless of the "-p" argument (if I'm not mistaken!). Now, I don't know if changing xpcshell to add $appdir/plugins *ONLY* when there is no -p argument, but I think at least we could make sure that the "-p" specified path ends up first in the enumeration.
To get the path specified with "-p" to appear first, you probably be need to swap the mGREDir and mPluginDir sections in . But as pointed out in bug 890098, this is still fragile - if possible let's add only one path to APluginsDL. I don't know about the situation where $appdir/plugins is needed, but couldn't we just use "-p" in that case too and drop the mGREDir part?  http://hg.mozilla.org/mozilla-central/annotate/b3fcd828cadc/js/xpconnect/shell/xpcshell.cpp#l1885
Okay, I think the appdir/plugins stuff is just historical cruft, and bug 817881 made it obsolete. We should be able to drop that path from APluginsDL without problems now that we're passing in -p with our correct plugin path. Incidentally, I think we could probably fix things so we didn't have to do all this file copying in xpcshell nowadays (bug 486570).
I m waiting for try to open and then I'll push a try run. After/if it turns green, I'll flag for r?.
Ok, so this fixes half of the problem. It seems that in automation, in runxpcshelltests.py's context, appdir != mgredir. I filed bug 900273 to have mozharness pass in the actual plugin path. After that lands, we can go on with removing the default location from xpcshell.cpp.
Will have to go ahead this way since it assures a smoother transition (with mozharness changes and all).
Attachment #783902 - Attachment is obsolete: true
Comment on attachment 785230 [details] [diff] [review] Make xpcshell default to a plugins directory only if -p is not specified https://tbpl.mozilla.org/?tree=Try&rev=9e6b4a5450c4
Attachment #785230 - Flags: review?(ted)
Attachment #785230 - Flags: review?(ted) → review+
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
https://hg.mozilla.org/releases/mozilla-beta/rev/4099ce90560f https://hg.mozilla.org/releases/mozilla-release/rev/231f0dbcfbef I gave lsblakk a heads-up about this and she was OK with me pushing to m-r. I guess we still need to figure out what to do about b2g18/esr17.
Adds a dummy argument for the b2g18/esr17 branches.
Attachment #786439 - Flags: review?(ted)
Comment on attachment 786439 [details] [diff] [review] b2g18/esr17 patch Review of attachment 786439 [details] [diff] [review]: ----------------------------------------------------------------- This depresses me, but it's the simplest way forward.
Attachment #786439 - Flags: review?(ted) → review+
You need to log in before you can comment on or make changes to this bug.