Skip to content

Commit

Permalink
Bug 1859585 - Add a new onReady delegated method to the GeckoView Add…
Browse files Browse the repository at this point in the history
…onManagerDelegate. r=amejiamarmol,willdurand,owlish

This patch includes the following changes:

- A new AddonManagerDelegate.onReady delegated method: this new delegated method is expected to be called when an extension
  has been started.
  On the firefox-android's android-components side (in particular the components providing the building blocks used by the
  Fenix AddonManager UI) are meant to be using this new delegated method to make sure that the resolved optionsPageUrl will
  be stored in the installedExtension map kept on the android-components side (by the WebExtensionSupport class to be precise),
  as a side-effect of that the settings button in the addon detail view is expected to become visible after having
  installed a new extension from AMO (as it was already the case for addons installed from the recommended set directly from
  the Fenix Addon Manager).

- Removes the implicit await on the extension to be ready from the underlying logic handling on the Gecko side the calls to
  the WebExtensionController.install method (which before these changes was the hack meant to make sure the settings button
  would be visible after installing an addon from the Fenix AddonManager UI).

- Replaces the previous test case asserting that the optionsPageUrl was going to be available right after having installed
  the extension (which was working only thanks to the implicit await described right above) with a test case covering the
  new expected behavior.

NOTE: To fully fix the issue tracked by this bugzilla ticket, this change has to be complemented with some more changes on the
firefox-android github repo side, in particular the once included in this draft patch:
- https://gist.githubusercontent.com/rpl/b87634df091214b14263216680b098e3/raw/630e305adba4f4867586729f01efe55e03ad367d/firefox-android--01-androidcomponents-changes.patch

Differential Revision: https://phabricator.services.mozilla.com/D193085
  • Loading branch information
rpl committed Nov 10, 2023
1 parent cf492d3 commit e50cebc
Show file tree
Hide file tree
Showing 7 changed files with 122 additions and 36 deletions.
1 change: 1 addition & 0 deletions mobile/android/geckoview/api.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2699,6 +2699,7 @@ package org.mozilla.geckoview {
method @UiThread default public void onInstallationFailed(@Nullable WebExtension, @NonNull WebExtension.InstallException);
method @UiThread default public void onInstalled(@NonNull WebExtension);
method @UiThread default public void onInstalling(@NonNull WebExtension);
method @UiThread default public void onReady(@NonNull WebExtension);
method @UiThread default public void onUninstalled(@NonNull WebExtension);
method @UiThread default public void onUninstalling(@NonNull WebExtension);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,8 @@
"matches": ["*://*.example.com/*"],
"js": ["borderify.js"]
}
]
],
"options_ui": {
"page": "dummy.html"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2566,17 +2566,37 @@ class NavigationDelegateTest : BaseSessionTest() {
}
})

val onReadyResult = GeckoResult<String>()
var extBaseUrl = ""
sessionRule.addExternalDelegateUntilTestEnd(
WebExtensionController.AddonManagerDelegate::class,
{ delegate -> controller.setAddonManagerDelegate(delegate) },
{ controller.setAddonManagerDelegate(null) },
object : WebExtensionController.AddonManagerDelegate {
@AssertCalled(count = 1)
override fun onReady(extension: WebExtension) {
extBaseUrl = extension.metaData.baseUrl
onReadyResult.complete(null)
super.onReady(extension)
}
},
)

val extension = sessionRule.waitForResult(
controller.install("https://example.org/tests/junit/page-history.xpi"),
)

// Wait for the extension to have been started before trying to navigate
// to the test extension page.
sessionRule.waitForResult(onReadyResult)

assertThat(
"baseUrl should be a valid extension URL",
extension.metaData.baseUrl,
extBaseUrl,
startsWith("moz-extension://"),
)

val url = extension.metaData.baseUrl + "page.html"
val url = extBaseUrl + "page.html"
processSwitchingTest(url)

sessionRule.waitForResult(controller.uninstall(extension))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,26 @@ class WebExtensionTest : BaseSessionTest() {
),
)

// Wait for the onReady AddonManagerDelegate method to be called, and assert
// that the baseUrl and optionsPageUrl are both available as expected.
val onReadyResult = GeckoResult<Void>()
sessionRule.addExternalDelegateUntilTestEnd(
WebExtensionController.AddonManagerDelegate::class,
{ delegate -> controller.setAddonManagerDelegate(delegate) },
{ controller.setAddonManagerDelegate(null) },
object : WebExtensionController.AddonManagerDelegate {
@AssertCalled(count = 1)
override fun onReady(extension: WebExtension) {
assertNotNull(extension.metaData.baseUrl)
assertTrue(extension.metaData.baseUrl.matches("^moz-extension://[0-9a-f\\-]*/$".toRegex()))
assertNotNull(extension.metaData.optionsPageUrl)
assertTrue((extension.metaData.optionsPageUrl ?: "").matches("^moz-extension://[0-9a-f\\-]*/options.html$".toRegex()))
onReadyResult.complete(null)
super.onReady(extension)
}
},
)

sessionRule.delegateDuringNextWait(object : WebExtensionController.PromptDelegate {
@AssertCalled(count = 1)
override fun onInstallPrompt(extension: WebExtension): GeckoResult<AllowOrDeny> {
Expand All @@ -436,11 +456,11 @@ class WebExtensionTest : BaseSessionTest() {
controller.install("resource://android/assets/web_extensions/dummy.xpi"),
)

val metadata = dummy.metaData
assertTrue((metadata.optionsPageUrl ?: "").matches("^moz-extension://[0-9a-f\\-]*/options.html$".toRegex()))
assertEquals(metadata.openOptionsPageInTab, true)
assertTrue(metadata.baseUrl.matches("^moz-extension://[0-9a-f\\-]*/$".toRegex()))
// In the onReady AddonManagerDelegate optionsPageUrl metadata is asserted again
// and expected to not be empty anymore.
assertNull(dummy.metaData.optionsPageUrl)

sessionRule.waitForResult(onReadyResult)
sessionRule.waitForResult(controller.uninstall(dummy))
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,16 @@ default void onInstalled(final @NonNull WebExtension extension) {}
@UiThread
default void onInstallationFailed(
final @Nullable WebExtension extension, final @NonNull InstallException installException) {}

/**
* Called whenever an extension startup has been completed (and relative urls to assets packaged
* with the extension can be resolved into a full moz-extension url, e.g. optionsPageUrl is
* going to be empty until the extension has reached this callback).
*
* @param extension The {@link WebExtension} that has been fully started.
*/
@UiThread
default void onReady(final @NonNull WebExtension extension) {}
}

/** This delegate is used to notify of extension process state changes. */
Expand Down Expand Up @@ -486,7 +496,8 @@ public void setAddonManagerDelegate(final @Nullable AddonManagerDelegate delegat
"GeckoView:WebExtension:OnUninstalled",
"GeckoView:WebExtension:OnInstalling",
"GeckoView:WebExtension:OnInstallationFailed",
"GeckoView:WebExtension:OnInstalled");
"GeckoView:WebExtension:OnInstalled",
"GeckoView:WebExtension:OnReady");
} else if (delegate != null && mAddonManagerDelegate == null) {
EventDispatcher.getInstance()
.registerUiThreadListener(
Expand All @@ -499,7 +510,8 @@ public void setAddonManagerDelegate(final @Nullable AddonManagerDelegate delegat
"GeckoView:WebExtension:OnUninstalled",
"GeckoView:WebExtension:OnInstalling",
"GeckoView:WebExtension:OnInstallationFailed",
"GeckoView:WebExtension:OnInstalled");
"GeckoView:WebExtension:OnInstalled",
"GeckoView:WebExtension:OnReady");
}

mAddonManagerDelegate = delegate;
Expand Down Expand Up @@ -934,6 +946,9 @@ public GeckoResult<WebExtension> update(final @NonNull WebExtension extension) {
} else if ("GeckoView:WebExtension:OnInstallationFailed".equals(event)) {
onInstallationFailed(bundle);
return;
} else if ("GeckoView:WebExtension:OnReady".equals(event)) {
onReady(bundle);
return;
}

extensionFromBundle(bundle)
Expand Down Expand Up @@ -1225,6 +1240,17 @@ private void onInstalled(final GeckoBundle bundle) {
mAddonManagerDelegate.onInstalled(extension);
}

private void onReady(final GeckoBundle bundle) {
if (mAddonManagerDelegate == null) {
Log.e(LOGTAG, "no AddonManager delegate registered");
return;
}

final GeckoBundle extensionBundle = bundle.getBundle("extension");
final WebExtension extension = new WebExtension(mDelegateControllerProvider, extensionBundle);
mAddonManagerDelegate.onReady(extension);
}

private void onDisabledProcessSpawning() {
if (mExtensionProcessDelegate == null) {
Log.e(LOGTAG, "no extension process delegate registered");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ exclude: true
- Added [`GeckoSession.sendImpressionAttributionEvent`][121.5] for sending impression attribution event for a given product recommendation.
- Added support for controlling `privacy.query_stripping.enabled` and `privacy.query_stripping.enabled.pbmode` via [`GeckoSession.ContentDelegate.queryParameterStrippingEnabled`][121.6] and [`GeckoSession.ContentDelegate.queryParameterStrippingPrivateBrowsingEnabled`][121.7].
- Added support for controlling `privacy.query_stripping.allow_list` and `privacy.query_stripping.strip_list` via [`GeckoSession.ContentDelegate.queryParameterStrippingAllowList`][121.8] and [`GeckoSession.ContentDelegate.queryParameterStrippingStripList`][121.9].
- Add [`WebExtensionController.AddonManagerDelegate.onReady`][121.10] ([bug 1859585]({{bugzilla}}1859585).

[121.1]: {{javadoc_uri}}/TranslationsController.RuntimeTranslation.html
[121.2]: {{javadoc_uri}}/ContentBlocking.Settings.Builder.html#cookieBannerGlobalRulesEnabled(boolean)
Expand All @@ -30,6 +31,7 @@ exclude: true
[121.7]: {{javadoc_uri}}/ContentBlocking.Settings.Builder.html#queryParameterStrippingPrivateBrowsingEnabled(boolean)
[121.8]: {{javadoc_uri}}/ContentBlocking.Settings.Builder.html#queryParameterStrippingAllowList(String)
[121.9]: {{javadoc_uri}}/ContentBlocking.Settings.Builder.html#queryParameterStrippingStripList(boolean)
[121.10]: {{javadoc_uri}}/WebExtensionController.AddonManagerDelegate.html#onReady

## v120
- Added [`disableExtensionProcessSpawning`][120.1] for disabling the extension process spawning. ([bug 1855405]({{bugzilla}}1855405))
Expand Down Expand Up @@ -1467,4 +1469,4 @@ to allow adding gecko profiler markers.
[65.24]: {{javadoc_uri}}/CrashReporter.html#sendCrashReport(android.content.Context,android.os.Bundle,java.lang.String)
[65.25]: {{javadoc_uri}}/GeckoResult.html

[api-version]: f650e289305768d5bfc8717d610d5c0348c140ea
[api-version]: 447654edd260c5b5341f0cadc98a4a8e8a68672a
66 changes: 40 additions & 26 deletions mobile/android/modules/geckoview/GeckoViewWebExtension.sys.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -487,32 +487,12 @@ class ExtensionInstallListener {

async onInstallEnded(aInstall, aAddon) {
debug`onInstallEnded addonId=${aAddon.id}`;
const addonId = aAddon.id;
const { sourceURI } = aInstall;

if (aAddon.userDisabled || aAddon.embedderDisabled) {
const extension = await exportExtension(
aAddon,
aAddon.userPermissions,
sourceURI
);
this.resolve({ extension });
return; // we don't want to wait until extension is enabled, so return early.
}

const onReady = async (name, { id }) => {
if (id != addonId) {
return;
}
lazy.Management.off("ready", onReady);
const extension = await exportExtension(
aAddon,
aAddon.userPermissions,
sourceURI
);
this.resolve({ extension });
};
lazy.Management.on("ready", onReady);
const extension = await exportExtension(
aAddon,
aAddon.userPermissions,
aInstall.sourceURI
);
this.resolve({ extension });
}
}

Expand Down Expand Up @@ -611,6 +591,40 @@ new AddonInstallObserver();
class AddonManagerListener {
constructor() {
lazy.AddonManager.addAddonListener(this);
// Some extension properties are not going to be available right away after the extension
// have been installed (e.g. in particular metaData.optionsPageURL), the GeckoView event
// dispatched from onExtensionReady listener will be providing updated extension metadata to
// the GeckoView side when it is actually going to be available.
this.onExtensionReady = this.onExtensionReady.bind(this);
lazy.Management.on("ready", this.onExtensionReady);
}

async onExtensionReady(name, extInstance) {
// In xpcshell tests there wil be test extensions that trigger this event while the
// AddonManager has not been started at all, on the contrary on a regular browser
// instance the AddonManager is expected to be already fully started for an extension
// for the extension to be able to reach the "ready" state, and so we just silently
// early exit here if the AddonManager is not ready.
if (!lazy.AddonManager.isReady) {
return;
}

debug`onExtensionReady ${extInstance.id}`;

const addonWrapper = await lazy.AddonManager.getAddonByID(extInstance.id);
if (!addonWrapper) {
return;
}

const extension = await exportExtension(
addonWrapper,
addonWrapper.userPermissions,
/* aSourceURI */ null
);
lazy.EventDispatcher.instance.sendRequest({
type: "GeckoView:WebExtension:OnReady",
extension,
});
}

async onDisabling(aAddon) {
Expand Down

0 comments on commit e50cebc

Please sign in to comment.