Skip to content
This repository has been archived by the owner on Jul 22, 2024. It is now read-only.

Crash service support #560

Merged
merged 11 commits into from
Oct 2, 2018
Merged

Crash service support #560

merged 11 commits into from
Oct 2, 2018

Conversation

keianhzo
Copy link
Contributor

Crash handler support.

}

if (!otherProcessesFound) {
Intent restartIntent = intent.setClass(CrashReporterService.this, VRBrowserActivity.class);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand this part. From the docs intent.setClass() returns it self. So I don't know why you need to reset all the values it already has.

intent.setClass(CrashReporterService.this, VRBrowserActivity.class).addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
startActivity(intent);

Should be the same?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original intent it targeted to the CrashReporterService so we need to retarget it for the VRBActivity but you are right we don't need to reset the rest of the values. Changed that.

@bluemarvin
Copy link
Contributor

Unfortunately we can't land this until we figure out a work around for the Go crash on first run issue.

@keianhzo keianhzo force-pushed the crash-reporter branch 3 times, most recently from 7482269 to 899a603 Compare September 21, 2018 18:14
@bluemarvin
Copy link
Contributor

@keianhzo I pushed some fixes for Android P to this PR and made the crash reporter setting only grayed out on Oculus. Please take a look.

@keianhzo
Copy link
Contributor Author

keianhzo commented Sep 24, 2018

@bluemarvin what's the issue with Android P? There is an infinite while loop in the latest commit that keeps on restarting the app whenever it's is closed after the first crash (crashparent). We should always let the crash service finish after handling the crash intent.

@bluemarvin
Copy link
Contributor

I posted a fix for gecko: https://bugzilla.mozilla.org/show_bug.cgi?id=1493227
We should probably wait to land this until the fix is in Gecko or we have an alternative.

@keianhzo
Copy link
Contributor Author

@bluemarvin added a fix for Android P. I still can see issues but only in API level 26.

@bluemarvin
Copy link
Contributor

@bluemarvin added a fix for Android P. I still can see issues but only in API level 26.

What issues are you seeing with 26? My S8 is running O right now.

@keianhzo
Copy link
Contributor Author

@bluemarvin it seems that system applies quite aggressive restrictions when the battery saver is on and the crash reporter service is killed upon second execution.

@keianhzo
Copy link
Contributor Author

@bluemarvin If it's working fine I guess we could land this and file a follow-up for testing in battery saving scenarios as I think this will only affect to Daydream "low end" clip-ins (S8).

@MortimerGoro
Copy link
Contributor

We need to wait for https://phabricator.services.mozilla.com/D7282 to land this

@MortimerGoro MortimerGoro merged commit 63b7d71 into master Oct 2, 2018
@bluemarvin bluemarvin deleted the crash-reporter branch October 9, 2018 19:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants