1208605 - Don't use static ANDROID_PACKAGE_NAME in toolkit/crashreporter

Status: RESOLVED FIXED

(Toolkit :: Crash Reporting, defect)

Description

[Nick Alexander :nalexander [he/him]]

Right now, we use a static ANDROID_PACKAGE_NAME reference at https://dxr.mozilla.org/mozilla-central/source/toolkit/crashreporter/nsExceptionHandler.cpp#1113.  This is a totally reasonable approach for the apps we build in automation.

Unfortunately, |mach artifact| based builds -- which are similar to the mooted "Go Faster" style builds -- use binaries built in automation (with package name 'org.mozilla.fennec') in locally produced binaries with package names like 'org.mozilla.fennec_$USER'.  We could re-use these binaries more easily if we extracted the appropriate package dynamically, at run-time.

This ticket tracks using the JNI to extract the package dynamically.

Comment 1

[Nick Alexander :nalexander [he/him]]

snorp: jchen: it's straightforward to use the JNI to extract the package name: something like:

static jstring getpkg(JNIEnv* env, jobject thiz, jobject activity) {
	jclass android_content_Context =env->GetObjectClass(activity);
	jmethodID midGetPackageName = env->GetMethodID(android_content_Context,"getPackageName", "()Ljava/lang/String;");
	jstring packageName= (jstring)env->CallObjectMethod(activity, midGetPackageName);
	return packageName;
}

(from https://gist.github.com/douzifly/11086408) should work.  Is there a reason not to do this?  Is it forbidden to use JNI in this location?

Comment 2

[Nick Alexander :nalexander [he/him]]

(In reply to Nick Alexander :nalexander from comment #1)
> snorp: jchen: it's straightforward to use the JNI to extract the package
> name: something like:

Hurgh, perhaps not so straight-forward: one needs an Android context somewhere, and it's not obvious where.  Presumably we can't get a GeckoAppShell or whatever at this point?

Comment 3

[Nick Alexander :nalexander [he/him]]

Would it be reasonable to fish the package name from an environment variable (set early in the process life, by the Java side) and perhaps fall back to the ANDROID_PACKAGE_NAME?

Comment 4

[Nick Alexander :nalexander [he/him]]

Attachment: MozReview Request: Bug 1208605 - Try MOZ_ANDROID_PACKAGE_NAME env var before static ANDROID_PACKAGE_NAME. r?snorp

Bug 1208605 - Try MOZ_ANDROID_PACKAGE_NAME env var before static ANDROID_PACKAGE_NAME. r?snorp

Comment 5

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8666236 [details]
MozReview Request: Bug 1208605 - Try MOZ_ANDROID_PACKAGE_NAME env var before static ANDROID_PACKAGE_NAME. r?snorp

https://reviewboard.mozilla.org/r/20473/#review18461

looks good

Comment 6

[James Willcox (:snorp) ([e-mail]) (he/him)]

Jim did this get committed?

Comment 7

[(inactive) Jim Chen [:jchen] [:darchons]]

I think you meant Nick?

Comment 8

[James Willcox (:snorp) ([e-mail]) (he/him)]

Indeed.

Comment 9

[Nick Alexander :nalexander [he/him]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=150bd0ae202f

Comment 10

[Nick Alexander :nalexander [he/him]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=bc1c7acfba18

Comment 11

[Nick Alexander :nalexander [he/him]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=279f247ab624

Comment 12

[Nick Alexander :nalexander [he/him]]

Try is green (the failures are a known, recently spiked, intermittent).  This is blocked on me verifying that the crash reporter still works, although it's hard to imagine it doesn't and we'd find out if it didn't quite quickly.

Comment 13

[Nick Alexander :nalexander [he/him]]

https://hg.mozilla.org/integration/fx-team/rev/8a3e98127ebc28fcc01d27bdedb4c959a456c47d
Bug 1208605 - Try MOZ_ANDROID_PACKAGE_NAME env var before static ANDROID_PACKAGE_NAME. r=snorp

Comment 14

[Carsten Book [:Tomcat]]

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