bug 1567097 comment 0 (a)

FWIW, all */unit_ipc/xpcshell.ini disable android.

Junior, could you work on this when possible (P2)?

The crash stack is:

mozilla::jni::GetEnvForThread() 
mozilla::ipc::AndroidProcessLauncher::DoLaunch()
mozilla::ipc::BaseProcessLauncher::PerformAsyncLaunch()

which is triggered by BaseProcessLauncher::Launch
and initial from the stack:

GeckoChildProcessHost::AsyncLaunch
mozilla::ipc::GeckoChildProcessHost::LaunchAndWaitForProcessHandle
mozilla::dom::ContentParent::LaunchSubprocessInternal
mozilla::dom::ContentParent::LaunchSubprocessSync
mozilla::dom::ContentParent::GetNewOrUsedBrowserProcess
GetOrCreateTestShellParent
XRE_SendTestShellCommand

whose root is javascript sendCommand in test https://searchfox.org/mozilla-central/rev/e7c61f4a68b974d5fecd216dc7407b631a24eb8f/testing/xpcshell/head.js#1320

That is, we need to set the JNIEnv if we want to launch a child process.
Currently we set the environment only for native launch through GeckoStart for parent process or XRE_SetAndroidChildFds for child process.
https://searchfox.org/mozilla-central/rev/e7c61f4a68b974d5fecd216dc7407b631a24eb8f/mozglue/android/APKOpen.cpp#370-386

But I'm not familiar with xpcshell-test setup in android.

Hello :snorp,
Do you (or someone else) know where might be the best place to set the JNIEnv for xpcshell-test?
Thanks!

xpcshell doesn't have a Dalvik VM at all, so we'd need to change that in order to fix this bug. My suggestion would be to host the xpcshell in an Android Service. Probably a pretty decent amount of work.

Thanks :snorp.
Looks like it won't happen in the near future.

In bug 1567342 I'll probably need to run nodeJS in a separate Android service, so maybe we could merge these efforts.

Only giving this a P3 priority to get it off the Fennec Project Board.

Raising the priority because this now blocks a P2 bug to re-enable some networking tests for Fenix (a Tier-1 platform).

The way forward here is to not run a bare executable on Android, but instead launch Gecko from a Service as we do for the JUnit tests. The steps are roughly:

1. Create a new Service in org.mozilla.geckoview.test, XPCShellService perhaps?
2. Make the service instantiate a GeckoRuntime that will somehow end up calling XRE_XPCShellMain(). We don't want xpcshell garbage in the public API, so one way to do this would be a hack that detects a special process name (like :xpcshell) and calls the different entry point. It's also possible that we don't even need a different entry point on Android, since we're already kinda headless.
3. Fix up the mach to launch this new service instead of running the bare executables. Any arguments we would normally pass can be sent in the Intent to start the service.

James (David Bolter doesn't accept ni?), I'm looking for an assignee here, as this blocks a (P2) work to re-enable now Tier-1/Fenix networking xpcshell tests. Can you advice please?

Other option is to give more detailed pointers where to start (a code to duplicate or use as an example perhaps; for the purpose stated in comment 9 by James) and someone from our team could work on this.

Thanks.

It's going to be pretty hairy. I think we'll need to try to prioritize this on the GV team. We probably need Emily for that.

proposed for GV76 sprint

I changed the summary to reflect how we're really treating this bug -- we need to run the xpcshell tests in an Android service in order for e10s to work.

We don't need to initialize SafeBrowsing immediately at startup and we can
wait until the InitLater stage.

This has the added benefit of not crashing xpcshell-test which doesn't have
SafeBrowsing support.

Sending arguments in one string breaks arguments that contain a space, e.g. in
xpcshell-test.

nsBaseAppShell already listens to xpcom-shutdown and calls |Exit| when it
receives it so we don't need to also listen for it in nsAppShell (which causes
two calls to xpcom-shutdown instead of one).

This is consistent to geckoview-junit and mochitest.

This commit adds a new command line option |-xpcshell| that, when passed, will
run an xpcshell instead of launching a full Gecko instance.

This command line option is restricted to org.mozilla.geckoview.test for now,
as it's really hard to use and not really a usecase outside mozilla. We can
revisit this if there's interest.

This service, based off of TestRunnerActivity, can be used to run an
xpcshell-test instance.

It supports up to 10 concurrent instances, although we can add more if we need
to.

Local testing indicates that with more than 4 concurrent instances there's not
gain in test running performance.

This also adds a new intent action
|org.mozilla.geckoview.test.XPCSHELL_TEST_MAIN| that can be used by the test
harness to start the main application and gain foreground priority without
starting Gecko (which would interfere with the xpcshell runner services).

This is done so that xpcshell-test runs with a Dalvik VM and has access to all
the java-implemented bits of GeckoView.

When getting the *-extension-installed messages we should always notify
GeckoViewWebExtension.

This currently works because we always install the test support extension which
causes us to initialize GeckoViewWebExtension. On xpcshel tests, however, there
is no support extension so we need to account for that in GeckoViewStartup.

There is a little Woodoo in this patch but here it is:

• According to the Android docs, services that implement onStartService are
  not considered bound services and need to stop themselves. Since we do want
  our child processes to die after every client unbounds, we shouldn't implement
  it.
• Sometimes processes don't die after |onDestroy| is called, this causes
  processes being reused on some esoteric edge case (multiple runtimes, mostly).
  To avoid these corner cases, we call System.exit in onDestroy. Note: chromium
  does this too.
• We now don't need to kill the process on onBind anymore (this is mostly to
  align with chromium).
• We make sure we don't get created multiple times, as that could cause really
  weird behavior which is hard to debug (especially on users' devices).

For tests, we want to be able to run multiple runtimes at the same time (each
runtime gets its own process).

This mostly work, except our process allocator does not currently check that it
owns a certain process and it will happily try to take control of a different
runtime's process (and fail).

To make this work we tag each content process with the parent process' UUID so
that we can check whether the process that we try to create is free or not.

This strategy is pretty rudimental and we can iteratively improve on it if we
decide to use multiple runtimes on actual apps (and not just tests).

This error is sent to the native layer so we never log it (at least locally).

This also adds a "cause" exception to have the full stacktrace.

Comment on attachment 9208422 [details]
Bug 1567341 - Ensure GeckoServiceChildProcess is treated as a bound service.

Revision D108036 was moved to bug 1696460. Setting attachment 9208422 [details] to obsolete.

Comment on attachment 9208423 [details]
Bug 1567341 - Allow multiple runtimes to run at the same time.

Revision D108037 was moved to bug 1696460. Setting attachment 9208423 [details] to obsolete.

Comment on attachment 9208424 [details]
Bug 1567341 - Improve bind error logging.

Revision D108038 was moved to bug 1696460. Setting attachment 9208424 [details] to obsolete.

On Android we kill the content process when receiving |MarkAsDead|. This means
that the shutdown sequence races with killing the content process, as sometimes
the content process is killed before it has a chance to complete the shutdown
sequence and notify the parent process.

To avoid test timeouts we remove the shutdown blockers instead.

The code as written does not remove the connection from both
mContentConnections and mNonStartedContentConnections since && short circuits.

Since the connection is always present in mContentConnections, we just check
that.

Backed out 24 changesets (bug 1567341) for causing xpcshell failures in test_telemetry.js
Backout link: https://hg.mozilla.org/integration/autoland/rev/c83c94d1986637661ff52d0beb24b4c94a6cd887
Push with failures, failure log.

Thanks

https://hg.mozilla.org/mozilla-central/rev/efb6cf38264e
https://hg.mozilla.org/mozilla-central/rev/dbc4f44ee10a
https://hg.mozilla.org/mozilla-central/rev/e755ef39a0ee
https://hg.mozilla.org/mozilla-central/rev/e50020363a1c
https://hg.mozilla.org/mozilla-central/rev/b2b2eaa9a75f
https://hg.mozilla.org/mozilla-central/rev/072597b667ec
https://hg.mozilla.org/mozilla-central/rev/303fda21f7c0
https://hg.mozilla.org/mozilla-central/rev/2e1e1ec91eb8
https://hg.mozilla.org/mozilla-central/rev/8980d58216bc
https://hg.mozilla.org/mozilla-central/rev/0e4b4e8cdb69
https://hg.mozilla.org/mozilla-central/rev/c2a0b0be6c7d
https://hg.mozilla.org/mozilla-central/rev/9b55527ca451
https://hg.mozilla.org/mozilla-central/rev/c347bab90951
https://hg.mozilla.org/mozilla-central/rev/848ba2ab270c
https://hg.mozilla.org/mozilla-central/rev/0db615460866
https://hg.mozilla.org/mozilla-central/rev/f2e031c40335
https://hg.mozilla.org/mozilla-central/rev/1c8892fce160
https://hg.mozilla.org/mozilla-central/rev/fc76a1768da4
https://hg.mozilla.org/mozilla-central/rev/c4aa3900ca76
https://hg.mozilla.org/mozilla-central/rev/de0dc03a7073
https://hg.mozilla.org/mozilla-central/rev/edb1af92004d
https://hg.mozilla.org/mozilla-central/rev/2bd1a86b219b
https://hg.mozilla.org/mozilla-central/rev/2bb57a8dd29f
https://hg.mozilla.org/mozilla-central/rev/1b0d3c1d43c7