From 659ac681e9c7e5a41aae2d089712398a550c34f5 Mon Sep 17 00:00:00 2001 From: Luca Greco Date: Tue, 10 Oct 2023 18:43:01 +0000 Subject: [PATCH] Bug 1855977 - Set ExtensionProcessCrashObserver appInForeground flag to false if GeckoView is in background when Gecko main process is being started. r=geckoview-reviewers,willdurand,zmckenney Differential Revision: https://phabricator.services.mozilla.com/D189680 --- .../geckoview/GeckoViewStartup.sys.mjs | 11 +++++++++ .../org/mozilla/geckoview/GeckoRuntime.java | 6 +++++ .../components/extensions/Extension.sys.mjs | 22 ++++++++++++++++- .../test_ext_background_early_shutdown.js | 24 +++++++++++++++++++ .../xpcshell/test_process_crash_telemetry.js | 24 +++++++++++++++++++ 5 files changed, 86 insertions(+), 1 deletion(-) diff --git a/mobile/android/components/geckoview/GeckoViewStartup.sys.mjs b/mobile/android/components/geckoview/GeckoViewStartup.sys.mjs index 41a2c4e6c5c6..f37fd8fbccc6 100644 --- a/mobile/android/components/geckoview/GeckoViewStartup.sys.mjs +++ b/mobile/android/components/geckoview/GeckoViewStartup.sys.mjs @@ -249,6 +249,7 @@ export class GeckoViewStartup { "GeckoView:ResetUserPrefs", "GeckoView:SetDefaultPrefs", "GeckoView:SetLocale", + "GeckoView:InitialForeground", ]); Services.obs.addObserver(this, "browser-idle-startup-tasks-finished"); @@ -287,6 +288,16 @@ export class GeckoViewStartup { debug`onEvent ${aEvent}`; switch (aEvent) { + case "GeckoView:InitialForeground": { + // ExtensionProcessCrashObserver observes this topic to determine when + // the app goes into the foreground for the first time. This could be useful + // when the app is initially created in the background because, in this case, + // the "application-foreground" topic isn't notified when the application is + // moved into the foreground later. That is because "application-foreground" + // is only going to be notified when the application was first paused. + Services.obs.notifyObservers(null, "geckoview-initial-foreground"); + break; + } case "GeckoView:ResetUserPrefs": { for (const name of aData.names) { Services.prefs.clearUserPref(name); diff --git a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntime.java b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntime.java index a0241be6ab1a..e7d44c77fb09 100644 --- a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntime.java +++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntime.java @@ -166,6 +166,12 @@ public final class GeckoRuntime implements Parcelable { // Do not trigger the first onResume event because it breaks nsAppShell::sPauseCount counter // thresholds. GeckoThread.onResume(); + } else { + // Notify Gecko when the application has been moved in the foreground for the first time + // after being created and started (used by the ExtensionProcessCrashObserver on the Gecko + // side to adjust the appIsForeground property when the application-foreground or + // application-background topics are not notified). + EventDispatcher.getInstance().dispatch("GeckoView:InitialForeground", null); } mPaused = false; // Can resume location services, checks if was in use before going to background diff --git a/toolkit/components/extensions/Extension.sys.mjs b/toolkit/components/extensions/Extension.sys.mjs index 8c34c972ad23..57030b328479 100644 --- a/toolkit/components/extensions/Extension.sys.mjs +++ b/toolkit/components/extensions/Extension.sys.mjs @@ -663,7 +663,14 @@ ExtensionAddonObserver.init(); export var ExtensionProcessCrashObserver = { initialized: false, - _appInForeground: true, + // For Android apps we initially consider the app as always starting + // in the background, then we expect to be setting it to foreground + // when GeckoView LifecycleListener onResume method is called on the + // Android app first startup. After the application has got on the + // foreground for the first time then onPause/onResumed LifecycleListener + // are called, the application-foreground/-background topics will be + // notified to Gecko and this flag will be updated accordingly. + _appInForeground: AppConstants.platform !== "android", _isAndroid: AppConstants.platform === "android", _processSpawningDisabled: false, @@ -687,6 +694,7 @@ export var ExtensionProcessCrashObserver = { "addons.process-crash-observer" ); if (this._isAndroid) { + Services.obs.addObserver(this, "geckoview-initial-foreground"); Services.obs.addObserver(this, "application-foreground"); Services.obs.addObserver(this, "application-background"); } @@ -701,6 +709,7 @@ export var ExtensionProcessCrashObserver = { Services.obs.removeObserver(this, "process-type-set"); Services.obs.removeObserver(this, "ipc:content-shutdown"); if (this._isAndroid) { + Services.obs.removeObserver(this, "geckoview-initial-foreground"); Services.obs.removeObserver(this, "application-foreground"); Services.obs.removeObserver(this, "application-background"); } @@ -717,10 +726,21 @@ export var ExtensionProcessCrashObserver = { observe(subject, topic, data) { let childID = data; switch (topic) { + case "geckoview-initial-foreground": + this._appInForeground = true; + this.logger.debug( + `Detected Android application moved in the foreground (geckoview-initial-foreground)` + ); + break; case "application-foreground": // Intentional fall-through case "application-background": this._appInForeground = topic === "application-foreground"; + this.logger.debug( + `Detected Android application moved in the ${ + this._appInForeground ? "foreground" : "background" + }` + ); if (this._appInForeground) { Management.emit("application-foreground", { appInForeground: this._appInForeground, diff --git a/toolkit/components/extensions/test/xpcshell/test_ext_background_early_shutdown.js b/toolkit/components/extensions/test/xpcshell/test_ext_background_early_shutdown.js index ba401a037902..f0cdf16de912 100644 --- a/toolkit/components/extensions/test/xpcshell/test_ext_background_early_shutdown.js +++ b/toolkit/components/extensions/test/xpcshell/test_ext_background_early_shutdown.js @@ -64,6 +64,30 @@ add_setup(function () { // Need a profile to init Glean. do_get_profile(); Services.fog.initializeFOG(); + + Assert.equal( + ExtensionProcessCrashObserver.appInForeground, + AppConstants.platform !== "android", + "Expect appInForeground to be initially true on desktop and false on android builds" + ); + + // For Android build we mock the app moving in the foreground for the first time + // (which, in a real Fenix instance, happens when the application receives the first + // call to the onPause lifecycle callback and the geckoview-initial-foreground + // topic is being notified to Gecko as a side-effect of that). + // + // We have to mock the app moving in the foreground before any of the test extension + // startup, so that both Desktop and Mobile builds are in the same initial foreground + // state for the rest of the test file. + if (AppConstants.platform === "android") { + info("Mock geckoview-initial-foreground observer service topic"); + ExtensionProcessCrashObserver.observe(null, "geckoview-initial-foreground"); + Assert.equal( + ExtensionProcessCrashObserver.appInForeground, + true, + "Expect appInForeground to be true after geckoview-initial-foreground topic" + ); + } }); add_setup( diff --git a/toolkit/components/extensions/test/xpcshell/test_process_crash_telemetry.js b/toolkit/components/extensions/test/xpcshell/test_process_crash_telemetry.js index 69e9c2dcd3f7..151fce579e3f 100644 --- a/toolkit/components/extensions/test/xpcshell/test_process_crash_telemetry.js +++ b/toolkit/components/extensions/test/xpcshell/test_process_crash_telemetry.js @@ -24,6 +24,30 @@ add_setup(() => { // the call to testGetValue. Services.fog.initializeFOG(); Services.fog.testResetFOG(); + + Assert.equal( + ExtensionProcessCrashObserver.appInForeground, + AppConstants.platform !== "android", + "Expect appInForeground to be initially true on desktop and false on android builds" + ); + + // For Android build we mock the app moving in the foreground for the first time + // (which, in a real Fenix instance, happens when the application receives the first + // call to the onPause lifecycle callback and the geckoview-initial-foreground + // topic is being notified to Gecko as a side-effect of that). + // + // We have to mock the app moving in the foreground before any of the test extension + // startup, so that both Desktop and Mobile builds are in the same initial foreground + // state for the rest of the test file. + if (AppConstants.platform === "android") { + info("Mock geckoview-initial-foreground observer service topic"); + ExtensionProcessCrashObserver.observe(null, "geckoview-initial-foreground"); + Assert.equal( + ExtensionProcessCrashObserver.appInForeground, + true, + "Expect appInForeground to be true after geckoview-initial-foreground topic" + ); + } }); add_task(async function test_process_crash_telemetry() {