Closed Bug 1368701 Opened 2 years ago Closed 2 years ago

Autophone - Quitter extension fails to terminate Fennec on Nytimes on Nexus 4 after Bug 1359653

Categories

(Firefox for Android :: General, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 56
Tracking Status
firefox54 --- wontfix
firefox55 --- fixed
firefox56 --- fixed

People

(Reporter: bc, Assigned: esawin)

References

(Blocks 1 open bug)

Details

(Keywords: regression)

Attachments

(4 files, 5 obsolete files)

89.87 KB, image/png
Details
106.61 KB, image/png
Details
7.09 KB, patch
snorp
: review+
Details | Diff | Splinter Review
1.89 KB, patch
snorp
: review+
Details | Diff | Splinter Review
Autophone uses a version of the Quitter extension
<http://searchfox.org/mozilla-central/source/tools/quitter>
in order to cause Fennec to exit cleanly after a page completes loading.

This stopped working for the Nexus 4 devices only for the nytimes test page only after Bug 1359653 landed. Testing locally shows this is also the case for Samsung GS3 devices. This implies the issue is for Android 4.2 and below (SDK 15 and SDK 17).

When fennec fails to exit, Autophone will manually kill the fennec process. This has the side effect of invalidating the cache which can be detected in the performance graphs by the first and second visits to the page resulting in identical measurements. You can see this in phonedash:

http://phonedash.mozilla.org/#/2017-04-30/2017-05-30/binning=repo-phonetype-phoneid-test_name-cached_label-metric&rejected=norejected&errorbars=noerrorbars&errorbartype=standarderror&valuetype=median&remote-nytimes=on&throbberstart=on&throbberstop=on&first=on&second=on&autoland=on&mozilla-inbound=on&nexus-4=on&nexus-4-7=on&nexus-4-8=on&nexus-4-9=on

You can see the initial landing, the backout and the relanding.

https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=8f4637881ddc42a948c894e62c8486fe8677a938&filter-searchStr=autophone&selectedJob=97070212

logcat:
https://autophone.s3.amazonaws.com/v1/task/HfCg8ppMSx-z0NJPdrvlpA/runs/0/artifacts/public/build/autophone-s1s2-s1s2-nytimes-remote.ini-1-nexus-4-9-61ef3751-28cf-4fe7-868a-cad1a8f098b1-logcat.log

There is nothing in the log which stands out to me.

This does not occur for the blank page test nor the twitter page test. I attempted to remove any JavaScript errors from my local copy of the nytimes page but that had no effect.

The signed version of quitter used in Autophone is available at https://github.com/mozilla/autophone/blob/master/xpi/quitter.xpi

I'm not sure how we want to handle this going forward. The old style extensions are no longer going to be supported in Gecko 57 and later even in testing? In that case Quitter will have to be replaced or we will have to give up on the measurement of cached results once it is no longer supported. If we can't resolve this particular issue, I think it would be best to stop testing nytimes on Nexus 4 devices.

kmag: Do you have any idea why Android 4.2 and below wouldn't like your patches?

The nytimes page isn't available publicly due to copyright issues but I can provide it to you if you wish to test this but I presume it will require a Nexus 4 device or at least a device with Android 4.0 or Android 4.2.

Autophone S1S2 tests are available on try via:
try: -b o -p android-api-15 -u autophone-s1s2 -t none
Flags: needinfo?(kmaglione+bmo)
@snorp: Is this what you had in mind for the shutdown intent?
If we need to make sure the process is killed, we could do that after finishing.

A similar approach with GeckoView should work once content process shutdown is fixed.
Assignee: nobody → esawin
Attachment #8874933 - Flags: feedback?(snorp)
Attachment #8874933 - Flags: feedback?(snorp) → feedback+
Can a malicious app / site cause Fennec to exit through this?
Comment on attachment 8874933 [details] [diff] [review]
0001-Bug-1368701-1.0-Add-shutdown-intent-action-to-shutdo.patch

Bob, can you check if this helps you with the shutdown issue on Nexus 4?
To shutdown Fennec, you need to send an intent with the shutdown action, e.g.:
adb shell am start -a "org.mozilla.gecko.SHUTDOWN" -n "org.mozilla.fennec_esawin/.App"

You still have to wait for it to shutdown though. We might also need to move the task to background first and force-kill the process.
If possible, it would be great to test what works best on autophone before we land anything.
Attachment #8874933 - Flags: feedback+ → feedback?(bob)
Ok. I'll check this laster this morning.
I ran a number of tests and the shutdown intent works pretty well. I did notice on some occasions where it did not shut down the browser but these were limited to initial runs loading the initialize profile page and were not common. This mostly occurred on nexus 4 and to some extent on the pixel which appears to have general issues initializing its profile.

I didn't notice much of a distinction between the first and second runs which caused me some concern that the cache again was being invalidated but it appears that there was a small persistent difference. The difficulty arises from a change 

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=c78b979c5c5965f97f66733caa77e1ae0510262f&tochange=96ea13bb00c5a058a8d571e30794e8d0385182ee

from maglione.k@gmail.com which improved the first visit throbber stop while leaving the second visit throbber stop unchanged. There is very little benefit coming from the cache after this. I believe there is improvement with the cached visits to be made here and would like someone to validate my impressions of the improvement of first visit throbber stop with unchanged second visit throbber stop.

I tweaked several of the wait parameters to keep the device from idling unnecessarily and came to the conclusion that if we are planning on using this to work around the loss of the quitter extension, I should test that as well. I removed the calls to quitter in the nytimes page and initialize_profile and ran several tests. I found the results much noisier than I am used to and began looking into that. I realized part of the issue was profile initialization which at this point still included the installation of quitter even though it wasn't being used. I then did more runs with quitter no longer installed and I believe it improved the situation somewhat. I have a feeling that initializing the profile has been having other side effects which may have been causing some problems.

Therefore I give a + to the change to allow the shutdown intent. I would like to recommend that I go ahead and make the changes necessary to support it while also removing quitter altogether now rather than later since it will be going away soon enough and possibly causes unknown issues in general. If we do this now, we will have the shutdown intent available on both trunk and beta after next week.

snorp, jmaher: Ok with you?
Flags: needinfo?(snorp)
Flags: needinfo?(jmaher)
thanks for the detailed explanation and I think the shutdown intent seems harmless in general.
Flags: needinfo?(jmaher)
esawin: Just to confirm, this will also work in geckoview_example?
Flags: needinfo?(esawin)
Blocks: 1371291
Sounds good to me
Flags: needinfo?(snorp)
(In reply to Bob Clary [:bc:] from comment #7)
> esawin: Just to confirm, this will also work in geckoview_example?

I think we still have to write that patch, but it would be similar to the Fennec one.
I would definitely like them both.
Attachment #8874933 - Flags: feedback?(bob) → feedback+
This version additionally kills the (Fennec) process on shutdown.

@bc: can you test this and report back on whether that improves or breaks things on autophone?
Attachment #8875859 - Flags: feedback?(bob)
This is the geckoview_example patch. It will also kill the process on shutdown, like patch 1.1 for Fennec.

Shutdown via:

adb shell am start -a "org.mozilla.geckoview_example.SHUTDOWN" -n "org.mozilla.geckoview_example/.GeckoViewActivity"
Flags: needinfo?(esawin)
Attachment #8875861 - Flags: feedback?(bob)
I thought things were going ok during testing but I am having second thoughts about whether this is working properly or even if Autophone is screwed up. I'll need more time to check things out and figure out what is going on. I'll get an answer as soon as I can.
Depends on: 1371248
Comment on attachment 8875859 [details] [diff] [review]
0001-Bug-1368701-1.1-Add-shutdown-intent-action-to-shutdo.patch

Review of attachment 8875859 [details] [diff] [review]:
-----------------------------------------------------------------

This doesn't actually work for me. I'll attach a screenshot of a local phonedash instance with my test results to illustrate.

::: mobile/android/base/java/org/mozilla/gecko/GeckoApp.java
@@ +2495,5 @@
>          }
> +
> +        if (mKillProcessOnDestroy) {
> +            android.os.Process.killProcess(android.os.Process.myPid());
> +        }

Does this really do anything? finishAndShutdown(false) will set mRestartOnShutdown to false and mShutdownOnDestroy to true. This means GeckoApplication.shutdown(null) would have already been called which would have already called Process.killProcess(Process.myPid()), right?
Attachment #8875859 - Flags: feedback?(bob) → feedback-
Comment on attachment 8875861 [details] [diff] [review]
0002-Bug-1368701-2.0-Add-shutdown-intent-action-to-shutdo.patch

Review of attachment 8875861 [details] [diff] [review]:
-----------------------------------------------------------------

This doesn't actually work for me. I'll attach a screenshot to show the lack of change.
Attachment #8875861 - Flags: feedback?(bob) → feedback-
This screenshot shows 4 different tests.

The first data point is from an inbound tree without these patches https://treeherder.allizom.org/#/jobs?repo=try&revision=dc6a0d39fc19648f563663e66081d800870e07a6 and uses the current test pages along with quitter with Autophone from the master branch corresponding to what is running in production now. You can see the difference in first and second runs for both throbberstart and throbberstop which I think indicates that the cache is working.

The second data point is from another inbound tree with no real changes from the first https://treeherder.allizom.org/#/jobs?repo=try&revision=79c11f9908969e306556dd5ad7248c0528cac142 with the new test pages without references to quitter and without the quitter extension together with a development version of Autophone which does not have support for quitter but does have support for the SHUTDOWN intents. This version of Autophone issues the shutdown intents and waits up to 10 seconds before issuing am force-stop, am kill or just plain kill in an escalating attempt to kill the process. Since this build does not support the shutdown intents, it effectively waits 10 seconds then kills fennec or geckoview_example with an am force-stop. You can still see the difference in first and second runs for both throbberstart and throbberstop which I think indicates that the cache is working. Note also the slight increase in the throbber times.

The third data point is the same inbound tree with the 0001 1.0 version of the patch applied https://treeherder.allizom.org/#/jobs?repo=try&revision=c3f2c287bc6efe4d8752935ceae3c886aafa0bac using with the new version of Autophone just as the second data point. This build does issue the shutdown intents and the the build supports at least the 1.0 version for fennec. As you can see we have lost the difference between the first and second visits as well as increased the throbber times significantly.

The fourth data point is the same inbound tree with the 0001 1.1 and 0002 2.0 versions of the patches applied https://treeherder.allizom.org/#/jobs?repo=try&revision=f184ec301124f423adb91affebcf8589b1bb7a4f using the new version of Autophone just as we used in the second and third data points. This version shows the same behavior with regard to the throbber times and first and second visits with an even larger increase in the throbber times.
This screenshot shows the same four builds using geckoview_example e10s. Apart from the descrease in the throbber start time from the first to second builds, the values are basically the same. Again we see that the cache is not working in these tests either before or after the patches.
Can we use the same method that Quitter uses to shutdown Gecko?
http://searchfox.org/mozilla-central/source/tools/quitter/QuitterObserver.js#60

        let appStartup = Cc["@mozilla.org/toolkit/app-startup;1"].getService(Ci.nsIAppStartup);
        appStartup.quit(Ci.nsIAppStartup.eForceQuit);

If we explicitly use the toolkit to shutdown Gecko rather than relying on an extension we would be insulated from the loss of oldschool extensions and still have the benefit of shutting down cleanly.
Comment on attachment 8875859 [details] [diff] [review]
0001-Bug-1368701-1.1-Add-shutdown-intent-action-to-shutdo.patch

(In reply to Bob Clary [:bc:] from comment #14)
> Does this really do anything? finishAndShutdown(false) will set
> mRestartOnShutdown to false and mShutdownOnDestroy to true. This means
> GeckoApplication.shutdown(null) would have already been called which would
> have already called Process.killProcess(Process.myPid()), right?

You're right, that's redundant for Fennec. We only have to keep it in the geckoview_example patch.
Attachment #8875859 - Attachment is obsolete: true
Thanks for the detailed results.

I don't know why the caches would be presumably invalidated. With purgecaches disabled, the caches remain valid for me locally after the intent-based shutdown.

There are also noticeable differences in throbberstart and throbbertime between the first and second data point for the geckoview tests, which are unexpected and unrelated to this patches.

On the Nexus 4, I've noticed that starting up too early after a shutdown would crash Fennec (no useful log, not reproducing with debugger attached), so we might need to increase the post-shutdown delay.

Btw., are there any downsides to force-stopping the process compared to using the quitter add-on (or this intent-based shutdown) that we have experienced on autophone?
The downside to force stopping is the invalidation of the cached measurements.
Can we use the toolkit method to quit in Fennec and GeckoView?

I'm not sure how much of this cache fall out is due to kmag's regression. kmag?
I can put up the full verbose logs for all of the runs if you are interested. They are pretty big, so I'll wait for confirmation from you before uploading.
(In reply to Bob Clary [:bc:] from comment #22)
> The downside to force stopping is the invalidation of the cached
> measurements.

Looking at the second data point, force-stopping does seem to preserve startup caches, or what do you mean by cached measurements?

> Can we use the toolkit method to quit in Fennec and GeckoView?

We could, I'll post the Fennec patch for you to test.
This patch uses a similar approach to the Quitter add-on to force-quit Fennec.
Attachment #8876857 - Flags: feedback?(bob)
bc: Can you please apply this for the autophone runs and post back the logcats, it should make it easier to parse?
(In reply to Eugen Sawin [:esawin] from comment #24)
>
> Looking at the second data point, force-stopping does seem to preserve
> startup caches, or what do you mean by cached measurements?
> 

cached web pages.

If we wait 10 seconds before issuing the intent, it will preserve the cached web pages but it doesn't force the cache web pages to be saved the same way a normal shutdown does.

(In reply to Eugen Sawin [:esawin] from comment #25)
> Created attachment 8876857 [details] [diff] [review]
> 0001-Bug-1368701-1.2-Add-shutdown-intent-action-to-shutdo.patch
> 
> This patch uses a similar approach to the Quitter add-on to force-quit
> Fennec.

Great.

(In reply to Eugen Sawin [:esawin] from comment #26)
> Created attachment 8876858 [details] [diff] [review]
> 0003-Bug-1368701-A.0-Add-startup-cache-logs.patch
> 
> bc: Can you please apply this for the autophone runs and post back the
> logcats, it should make it easier to parse?

Do you want the original patches tested with this or the toolkit quitter-lite patch?
(In reply to Bob Clary [:bc:] from comment #27)
> cached web pages.
> 
> If we wait 10 seconds before issuing the intent, it will preserve the cached
> web pages but it doesn't force the cache web pages to be saved the same way
> a normal shutdown does.

But don't your linked results show that force-stop does preserve web cache? The difference between first and second run seems to be preserved for the second data point. 

> Do you want the original patches tested with this or the toolkit
> quitter-lite patch?

Just the latest Fennec path version, although this will only log startup cache info.
Since the patch affected throbberstart, I've assumed the startup cache may be invalidated, too.
From comment 16

The second data point is from another inbound tree with no real changes from the first https://treeherder.allizom.org/#/jobs?repo=try&revision=79c11f9908969e306556dd5ad7248c0528cac142 

*** This version of the tree doesn't have the shutdown intent patches at all. ***

with the new test pages without references to quitter and without the quitter extension together with a development version of Autophone which does not have support for quitter but does have support for the SHUTDOWN intents. This version of Autophone issues the shutdown intents and waits up to 10 seconds before issuing am force-stop, am kill or just plain kill in an escalating attempt to kill the process.

*** Since this build does not support the shutdown intents ***

, it effectively waits 10 seconds then kills fennec or geckoview_example with an am force-stop. You can still see the difference in first and second runs for both throbberstart and throbberstop which I think indicates that the cache is working. Note also the slight increase in the throbber times.

The web page cache is only being saved because I wait an inordinately long time for Fennec to save it before I kill Fennec.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=bdfb70b7ece5bdaf616a508ddc17e9195f8ea01d

application crashed [@ mozilla::jni::NativeStub<mozilla::java::GeckoAppShell::ReportJavaCrash_t, GeckoAppShellSupport, mozilla::jni::Args<const mozilla::jni::Ref<mozilla::jni::TypedObject<_jthrowable*>, _jthrowable*>&, const mozilla::jni::StringParam&> >::Wrap<GeckoAppShellSupport::ReportJavaCrash>]

PROCESS-CRASH | autophone-s1s2 | java.lang.UnsatisfiedLinkError: No implementation found for void org.mozilla.gecko.GeckoThread.forceQuit() (tried Java_org_mozilla_gecko_GeckoThread_forceQuit and Java_org_mozilla_gecko_GeckoThread_forceQuit__) at org.mozilla.gecko.GeckoThread.forceQuit(Native Method)

PROCESS-CRASH | autophone-s1s2 | application crashed [@ mozilla::jni::NativeStub<mozilla::java::GeckoAppShell::ReportJavaCrash_t, GeckoAppShellSupport, mozilla::jni::Args<const mozilla::jni::Ref<mozilla::jni::TypedObject<_jthrowable*>, _jthrowable*>&, const mozilla::jni::StringParam&> >::Wrap<GeckoAppShellSupport::ReportJavaCrash>]
Attachment #8876857 - Flags: feedback?(bob)
(In reply to Bob Clary [:bc:] (pto 06/13) from comment #30)
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=bdfb70b7ece5bdaf616a508ddc17e9195f8ea01d
> 
> application crashed [@
> mozilla::jni::NativeStub<mozilla::java::GeckoAppShell::ReportJavaCrash_t,
> GeckoAppShellSupport, mozilla::jni::Args<const
> mozilla::jni::Ref<mozilla::jni::TypedObject<_jthrowable*>, _jthrowable*>&,
> const mozilla::jni::StringParam&>
> >::Wrap<GeckoAppShellSupport::ReportJavaCrash>]
> 
> PROCESS-CRASH | autophone-s1s2 | java.lang.UnsatisfiedLinkError: No
> implementation found for void org.mozilla.gecko.GeckoThread.forceQuit()
> (tried Java_org_mozilla_gecko_GeckoThread_forceQuit and
> Java_org_mozilla_gecko_GeckoThread_forceQuit__) at
> org.mozilla.gecko.GeckoThread.forceQuit(Native Method)
> 
> PROCESS-CRASH | autophone-s1s2 | application crashed [@
> mozilla::jni::NativeStub<mozilla::java::GeckoAppShell::ReportJavaCrash_t,
> GeckoAppShellSupport, mozilla::jni::Args<const
> mozilla::jni::Ref<mozilla::jni::TypedObject<_jthrowable*>, _jthrowable*>&,
> const mozilla::jni::StringParam&>
> >::Wrap<GeckoAppShellSupport::ReportJavaCrash>]

Artifact builds won't work for this patch.
Thanks. I had trouble with my local build yesterday and didn't think of the artifact nature of the build on try. I'm building now and will check as soon as I can.
The results look ok to me. The logs can be found at https://bclary.com/1368701/
This is the geckoview_example patch based on the forced quitting.
Does this help with preserving the cache?
Attachment #8877591 - Flags: feedback?(bob)
Testing geckoview_example:

try : https://treeherder.mozilla.org/#/jobs?repo=try&revision=04ad0e642e0038952d62a3a02da8b6e088a48cc3
logs: https://bclary.com/1368701-02/

The shutdown doesn't happen consistently. You can see that in the logs when it waits more than once and then uses a am force-stop to kill it. It appears that the second visit performs worse than the first visit. You can see the text summary by searching for RESULTS:. cachesOK=1 never occurs. It is always 0.
Comment on attachment 8877591 [details] [diff] [review]
0002-Bug-1368701-2.1-Add-shutdown-intent-action-to-shutdo.patch

I would say there is still something not quite right here. Could it be due to the fact we still don't have the ability to specify the profile?
Attachment #8877591 - Flags: feedback?(bob) → feedback-
(In reply to Bob Clary [:bc:] (pto 2017-06-16) from comment #36)
> Comment on attachment 8877591 [details] [diff] [review]
> 0002-Bug-1368701-2.1-Add-shutdown-intent-action-to-shutdo.patch
> 
> I would say there is still something not quite right here. Could it be due
> to the fact we still don't have the ability to specify the profile?

CheckCompatibility in nsAppRunner.cpp needs a valid profile, which could explain why cachesOK is always false.
Also with BuildConfig.DEBUG set, startup caches will be purged, maybe we should log that to make sure we're setting that correctly for optimal builds.

I'm not sure yet what's happening there, I can't reproduce the cache invalidation locally with this patch.
My autophone code is still specifying the profile via -profile /storage/sdcard0/tests/autophone/profile when starting the app. Perhaps that is the problem. I can run a test without it and see how it does.
I did a test run over the weekend and it wasn't a clear win or loss for geckoview_example. I'm going to try the patch in bug 1365636 and see how that looks. Even if we haven't got geckoview completely resolved, I think we should go ahead and get Fennec's fixes landed and move on and fix geckoview as we can. More news in a bit.
(In reply to Bob Clary [:bc:] from comment #39)
> I did a test run over the weekend and it wasn't a clear win or loss for
> geckoview_example. I'm going to try the patch in bug 1365636 and see how
> that looks. Even if we haven't got geckoview completely resolved, I think we
> should go ahead and get Fennec's fixes landed and move on and fix geckoview
> as we can. More news in a bit.

Sounds good, I'll file a new bug for geckoview_example. We won't be able to land this before next week though, since all potential reviewers are on PTO and the security issue is still open.
Comment on attachment 8876857 [details] [diff] [review]
0001-Bug-1368701-1.2-Add-shutdown-intent-action-to-shutdo.patch

Review of attachment 8876857 [details] [diff] [review]:
-----------------------------------------------------------------

r+: This works for me in fennec.

I ran one last test:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a0f54867a0f5e0f555e0ab855312e4b3f1d74581

It didn't report to Treeherder but the graphs are available at phonedash-dev:
http://phonedash-dev.allizom.org/#/2017-06-19/2017-06-20/binning=repo-phonetype-phoneid-test_name-cached_label-metric&rejected=norejected&errorbars=noerrorbars&errorbartype=standarderror&valuetype=median&remote-nytimes=on&throbberstart=on&throbberstop=on&first=on&second=on&try-bclary=on&nexus-4=on&nexus-4-13=on

and the log are available at:
https://bclary.com/1368701-06/

Again geckoview is problematic even with the patch from bug 1365636 but we'll have to revisit that someday.

Do we have the bug for restricting access to the shutdown intent filed yet?
Attachment #8876857 - Flags: review?(bob) → review+
Clearing out the needinfo because it looks like you've got lots of responses by now.
Flags: needinfo?(kmaglione+bmo)
Adds "app.shutdownintent.enabled" pref to enable the shutdown intent action.
Attachment #8883103 - Flags: review?(snorp)
Attachment #8883103 - Flags: review?(bob)
Attachment #8876857 - Flags: review+ → review?(snorp)
Attachment #8874933 - Attachment is obsolete: true
Attachment #8876858 - Attachment is obsolete: true
Attachment #8875861 - Attachment is obsolete: true
Attachment #8883103 - Flags: review?(snorp) → review+
Attachment #8876857 - Flags: review?(snorp) → review+
Comment on attachment 8883103 [details] [diff] [review]
0003-Bug-1368701-3.0-Guard-shutdown-intent-action-via-Gec.patch

r+
Attachment #8883103 - Flags: review?(bob) → review+
Comment on attachment 8876857 [details] [diff] [review]
0001-Bug-1368701-1.2-Add-shutdown-intent-action-to-shutdo.patch

Review of attachment 8876857 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/java/org/mozilla/gecko/GeckoApp.java
@@ +2120,5 @@
>          final String action = intent.getAction();
>  
> +        if (ACTION_SHUTDOWN.equals(action)) {
> +            mShutdownOnDestroy = true;
> +            GeckoThread.forceQuit();

BTW, I think calling `finishAndShutdown(/* restart */ false);` should be enough here. No need to go through native code. Same for GVE; calling `finish` then `killProcess` should work.
(In reply to Jim Chen [:jchen] [:darchons] from comment #45)
> BTW, I think calling `finishAndShutdown(/* restart */ false);` should be
> enough here. No need to go through native code. Same for GVE; calling
> `finish` then `killProcess` should work.

That was the initial approach and it was causing issues on Autophone (see previous comments).
Pushed by esawin@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2ab6ace91380
[1.2] Add shutdown intent action to shutdown Fennec. r=snorp,bc
https://hg.mozilla.org/integration/mozilla-inbound/rev/41d92e5db112
[3.0] Guard shutdown intent action via Gecko pref on Fennec. r=snorp,bc
https://hg.mozilla.org/mozilla-central/rev/2ab6ace91380
https://hg.mozilla.org/mozilla-central/rev/41d92e5db112
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Bob says we want this on Beta too. Can you please nominate it?
Flags: needinfo?(esawin)
Attachment #8877591 - Attachment is obsolete: true
Flags: needinfo?(esawin)
Comment on attachment 8876857 [details] [diff] [review]
0001-Bug-1368701-1.2-Add-shutdown-intent-action-to-shutdo.patch

Approval Request Comment
[Feature/Bug causing the regression]: Fennec shutdown for Autophone.
[User impact if declined]: No user impact, only used for Autophone.
[Is this code covered by automated tests?]: No.
[Has the fix been verified in Nightly?]: Yes.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: It's disabled by default via pref.
[String changes made/needed]: None.
Attachment #8876857 - Flags: approval-mozilla-beta?
Attachment #8883103 - Flags: approval-mozilla-beta?
Comment on attachment 8876857 [details] [diff] [review]
0001-Bug-1368701-1.2-Add-shutdown-intent-action-to-shutdo.patch

provide a way for autophone to shutdown fennec, beta55+
Attachment #8876857 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #8883103 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
See Also: → 1422405
You need to log in before you can comment on or make changes to this bug.