bugzilla.mozilla.org has resumed normal operation. Attachments prior to 2014 will be unavailable for a few days. This is tracked in Bug 1475801.
Please report any other irregularities here.

Don't use static ANDROID_PACKAGE_NAME in toolkit/crashreporter

RESOLVED FIXED in Firefox 45

Status

()

Toolkit
Crash Reporting
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: nalexander, Unassigned)

Tracking

unspecified
mozilla45
Points:
---

Firefox Tracking Flags

(firefox45 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
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.
(Reporter)

Comment 1

3 years ago
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?
Flags: needinfo?(snorp)
Flags: needinfo?(nchen)
(Reporter)

Comment 2

3 years ago
(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?
(Reporter)

Comment 3

3 years ago
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?
(Reporter)

Comment 4

3 years ago
Created attachment 8666236 [details]
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
Attachment #8666236 - Flags: review?(snorp)
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
Attachment #8666236 - Flags: review?(snorp) → review+
Flags: needinfo?(nchen)
Flags: needinfo?(snorp)
Jim did this get committed?
Flags: needinfo?(nchen)
I think you meant Nick?
Flags: needinfo?(nchen) → needinfo?(nalexander)
(Reporter)

Comment 12

3 years ago
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.
Flags: needinfo?(nalexander)
(Reporter)

Comment 13

3 years ago
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

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8a3e98127ebc
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in before you can comment on or make changes to this bug.