commit 852300392400e30c4a32676d4a23b4e00246d715
parent 39485d5ed9ce93dbd8c1ea86ea69ca986531e9f1
Author: Luca Greco <lgreco@mozilla.com>
Date: Thu, 2 Oct 2025 19:07:23 +0000
Bug 1876317 - Replace TestActivityRunner DebuggerDelegate with AddonManagerDelegate to selectively add/remove ExtensionWrappers. r=geckoview-reviewers,robwu,ohall
This patch applies changes to TestActivityRunner to replace its use of
DebuggerDelegate for refresh the ExtensionWrapper instances associated
to the extensions installed on the Gecko side with a AddonManagerDelegate
instance that does create and rmeove ExtensionWrapper instances
selectively instead of dropping all of them and replace them with new
instances as it was currently being done by the refreshExtensionList
TestActivityRunner method.
The previous behavior has been already a source of unexpected test
failures in the past (see the other bugs linked to this bugzilla
issue for a few examples) and it did was also part of the reasons
for the perma-failure hit by test_ext_runtime_getContext.html
more recently (see Bug 1988445).
In Bug 1988445 the reason for the test failure was not due to the fact
that refreshExtensionList was clearing the ExtensionWrapper instances
on the TestRunnerActivity but a race between the time needed for
refreshExtensionList to get the update list of extensions metadata
(which was async on both GeckoView and Gecko sides) vs. the call
to the onBrowserAction method from the TestActivityRunner
ActionDelegate (originated from the browserAction onManifestEntry
lifecycle method), which was hitting a crash due to onBrowserAction
method trying to access the browserActions property on a non existing
entry of the mExtensions HashMap.
This patch prevents the crash hit by test_ext_runtime_getContext.html
by creating the ExtensionWrapper instances from the AddonManagerDelegate
onInstalling method (onInstalled ends up being too late and hitting the
same race and conseguent crash).
NOTE: this patch is also technically taking care of the TestRunnerActivity
part of Bug 1876329, using the 3rd of the possible approaches described in
Bug 1876329 comment 0 (but it doesn't fully cover it yet, separately
from this fix we would need to apply similar kind of changes to
TestRunner for the xpcshell test harness and Android-Components to avoid
the similar issues that installing extensions temporarily on Fenix may
also be able to hit).
Differential Revision: https://phabricator.services.mozilla.com/D265430
Diffstat:
1 file changed, 83 insertions(+), 41 deletions(-)
diff --git a/mobile/android/test_runner/src/main/java/org/mozilla/geckoview/test_runner/TestRunnerActivity.java b/mobile/android/test_runner/src/main/java/org/mozilla/geckoview/test_runner/TestRunnerActivity.java
@@ -14,6 +14,7 @@ import android.content.pm.ActivityInfo;
import android.graphics.SurfaceTexture;
import android.net.Uri;
import android.os.Bundle;
+import android.util.Log;
import android.view.Surface;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
@@ -46,6 +47,7 @@ public class TestRunnerActivity extends Activity {
private static final String LOGTAG = "TestRunnerActivity";
private static final String ERROR_PAGE =
"<!DOCTYPE html><head><title>Error</title></head><body>Error!</body></html>";
+ private static final String TEST_RUNNER_EXT_ID = "test-runner-support@tests.mozilla.org";
static GeckoRuntime sRuntime;
@@ -281,6 +283,9 @@ public class TestRunnerActivity extends Activity {
final WebExtension extension,
final GeckoSession session,
final WebExtension.Action action) {
+ if (!mExtensions.containsKey(extension.id)) {
+ throw new RuntimeException("Extension not found: " + extension.id);
+ }
mExtensions.get(extension.id).browserActions.put(session, action);
}
@@ -289,6 +294,9 @@ public class TestRunnerActivity extends Activity {
final WebExtension extension,
final GeckoSession session,
final WebExtension.Action action) {
+ if (!mExtensions.containsKey(extension.id)) {
+ throw new RuntimeException("Extension not found: " + extension.id);
+ }
mExtensions.get(extension.id).pageActions.put(session, action);
}
};
@@ -469,11 +477,34 @@ public class TestRunnerActivity extends Activity {
sRuntime = GeckoRuntime.create(this, runtimeSettingsBuilder.build());
webExtensionController()
- .setDebuggerDelegate(
- new WebExtensionController.DebuggerDelegate() {
+ .setAddonManagerDelegate(
+ new WebExtensionController.AddonManagerDelegate() {
+ @Override
+ public void onInstalling(final @NonNull WebExtension extension) {
+ // NOTE: doing this from onInstalled seems to not be early enough
+ // and some mochitests (e.g. test_ext_runtime_getContexts.html)
+ // end up triggering a race with the onBrowserAction method
+ // being called due to the GeckoView request sent from
+ // brawserAction API onManifestEntry lifecycle method.
+ initExtensionWrapper(extension);
+ }
+
+ @Override
+ public void onEnabling(final @NonNull WebExtension extension) {
+ // NOTE: similarly to onInstalled, onEnabled may still hit
+ // a race with the call to onBrowserAction method originated
+ // from the browserAction API onManifestEntry lifecycle method.
+ initExtensionWrapper(extension);
+ }
+
+ @Override
+ public void onDisabled(final @NonNull WebExtension extension) {
+ mExtensions.remove(extension.id);
+ }
+
@Override
- public void onExtensionListUpdated() {
- refreshExtensionList();
+ public void onUninstalled(final @NonNull WebExtension extension) {
+ mExtensions.remove(extension.id);
}
});
@@ -485,9 +516,6 @@ public class TestRunnerActivity extends Activity {
extension.setTabDelegate(mTabDelegate);
});
- webExtensionController()
- .setAddonManagerDelegate(new WebExtensionController.AddonManagerDelegate() {});
-
sRuntime.setDelegate(
() -> {
mKillProcessOnDestroy = true;
@@ -633,45 +661,59 @@ public class TestRunnerActivity extends Activity {
// Random start timestamp for the BrowsingDataDelegate API.
private static final int CLEAR_DATA_START_TIMESTAMP = 1234;
- private void refreshExtensionList() {
- webExtensionController()
- .list()
- .accept(
- extensions -> {
- mExtensions.clear();
- for (final WebExtension extension : extensions) {
- mExtensions.put(extension.id, new ExtensionWrapper(extension));
- extension.setActionDelegate(mActionDelegate);
- extension.setTabDelegate(mTabDelegate);
+ private void initExtensionWrapper(@NonNull WebExtension extension) {
+ // Trigger an explicit TestRunnerActivity crash if a new ExtensionWrapper
+ // would be overwriting an existing ExtensionWrapper for the same
+ // extension id (so that we may be hitting a failure in the same
+ // test case that is hitting a race between the onUninstalled/onDisabled
+ // AddonManagerDelegate methods removing an older ExtensionWrapper
+ // instance and the onInstalling/onEnabling AddonManagerDelegate methods
+ // creating a new one).
+ if (mExtensions.containsKey(extension.id)) {
+ throw new RuntimeException(
+ "Unexpected existing ExtensionWrapper found for add-on id: " + extension.id);
+ }
+ mExtensions.put(extension.id, new ExtensionWrapper(extension));
+ extension.setActionDelegate(mActionDelegate);
+ extension.setTabDelegate(mTabDelegate);
- extension.setBrowsingDataDelegate(
- new WebExtension.BrowsingDataDelegate() {
- @Nullable
- @Override
- public GeckoResult<Settings> onGetSettings() {
- final long types =
- Type.CACHE
- | Type.COOKIES
- | Type.HISTORY
- | Type.FORM_DATA
- | Type.DOWNLOADS;
- return GeckoResult.fromValue(
- new Settings(CLEAR_DATA_START_TIMESTAMP, types, types));
- }
- });
-
- for (final GeckoSession session : mOwnedSessions) {
- final WebExtension.SessionController controller =
- session.getWebExtensionController();
- controller.setActionDelegate(extension, mActionDelegate);
- controller.setTabDelegate(extension, mSessionTabDelegate);
- }
- }
- });
+ extension.setBrowsingDataDelegate(
+ new WebExtension.BrowsingDataDelegate() {
+ @Nullable
+ @Override
+ public GeckoResult<Settings> onGetSettings() {
+ final long types =
+ Type.CACHE | Type.COOKIES | Type.HISTORY | Type.FORM_DATA | Type.DOWNLOADS;
+ return GeckoResult.fromValue(new Settings(CLEAR_DATA_START_TIMESTAMP, types, types));
+ }
+ });
+
+ for (final GeckoSession session : mOwnedSessions) {
+ final WebExtension.SessionController controller = session.getWebExtensionController();
+ controller.setActionDelegate(extension, mActionDelegate);
+ controller.setTabDelegate(extension, mSessionTabDelegate);
+ }
}
@Override
protected void onDestroy() {
+ // Raise an explicit RuntimeException if some test extension has leaked an ExtensionWrapper
+ // instance in the TestRunnerActivity and trigger a more visible failure (in addition to
+ // the similar shutdown check done by the Gecko side of the test harness).
+ if (!mExtensions.isEmpty()) {
+ Boolean hasLeakedExtensionWrappers = false;
+ for (final String extensionId : mExtensions.keySet()) {
+ if (TEST_RUNNER_EXT_ID.equals(extensionId)) {
+ continue;
+ }
+ hasLeakedExtensionWrappers = true;
+ Log.e(LOGTAG, "ExtensionWrapper leaked for add-on id: " + extensionId);
+ }
+ if (hasLeakedExtensionWrappers) {
+ throw new RuntimeException(
+ "ExtensionWrapper instances leaked detected at TestRunnerActivity shutdown");
+ }
+ }
mSession.close();
super.onDestroy();
