Closed Bug 1655761 Opened 5 years ago Closed 5 years ago

Disabling extension with EnableSource.APP can fail with TypeError

Categories

(GeckoView :: Extensions, defect, P1)

Unspecified
All
defect

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: csadilek, Unassigned)

References

Details

We're seeing a few users running into this during migration:
https://github.com/mozilla-mobile/android-components/issues/7875

java.lang.Exception: Unexpected error: TypeError: extension.setEmbedderDisabled is not a function
    at org.mozilla.geckoview.WebExtensionController$WebExtensionResult.sendError(WebExtensionController.java:6)
    at org.mozilla.gecko.EventDispatcher$JavaCallbackDelegate$1.run(EventDispatcher.java:3)
    at android.os.Handler.handleCallback(Handler.java:761)
    at android.os.Handler.dispatchMessage(Handler.java:98)
    at android.os.Looper.loop(Looper.java:156)
    at android.app.ActivityThread.main(ActivityThread.java:6523)
    at java.lang.reflect.Method.invoke(Method.java)
    at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:942)
    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:832)
Flags: needinfo?(rob)

A way to reach this state is when registerWebExtension uses an add-on ID of extension that already exists in the add-on manager. When webExtensionController.list() is called, the add-ons from the AddonManager are returned. But registerWebExtension can shadow the actual add-on from the add-on manager, and return a non-AddonWrapper object:
https://searchfox.org/mozilla-central/rev/d9f92154813fbd4a528453c33886dc3a74f27abb/mobile/android/modules/geckoview/GeckoViewWebExtension.jsm#621,642

GeckoViewWebExtension.jsm contains lots of code that assumes the extensionById call to return an AddonWrapper. The registerWebExtension code path is not tested, unsupported and causing bugs. A-C should not use this, especially since GV already offers an alternative to registerWebExtension since 78 (bug 1601067). A-C seems not keen on migrating away from registerWebExtension: https://github.com/mozilla-mobile/android-components/pull/7245#issuecomment-639826805

This is the third unresolved bug that is caused by the use of registerWebExtension instead of the recommended way to use add-ons (through the AddonManager). Android-Components should ideally migrate to installBuiltin so that GV can remove this registerWebExtension method in bug 1634504. I believe that we will continue to have more bugs if A-C continues to use this untested deprecated method.

Flags: needinfo?(rob)
See Also: → 1655755

This is the third unresolved bug that is caused by the use of registerWebExtension instead of the recommended way to use add-ons (through the AddonManager). Android-Components should ideally migrate to installBuiltin so that GV can remove this registerWebExtension method in bug 1634504. I believe that we will continue to have more bugs if A-C continues to use this untested deprecated method.

AC migrated to registerWebExtension, it just hasn't gotten to release yet.

I think it's unlikely that this is because of registerWebExtension, since this code runs before the app registers any extension, but I can test that.

ni to test this on a migration build

Flags: needinfo?(agi)

Tested it locally and I can't reproduce. Although there's probably something that triggers this (or a timing issue). Our current lead is that the webcompat addon is the one causing this, since it's the only one that has an ID that could conflict.

Flags: needinfo?(agi)
Priority: -- → P1

We changed the webcompat addon ID to avoid a collision and that indeed fixed the problem. Since registerWebExtension is going away (and AC doesn't use it) I'll WONTFIX this.

Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.