1761828 - Consider propagating isHandlingUserInput flag from ProxyAPIImplementation.callAsyncFunction and to make it available to the WebExtensions API methods implementation in the parent process

Status: RESOLVED FIXED

(WebExtensions :: Frontend, task, P2)

Description

[Luca Greco [:rpl] [:luca] [:lgreco]]

This task is part of an approach to fix Bug 1759231 currently being considered, and it is meant to allow us to change the behavior of an API method (in particular browser.downloads.download) when called with or without handling user input (instead of strictly requiring it, as already supported by the WebExtensions API schemas through the "requireUserInput" schema property).

See Bug 1759231 comment 9 for more details about the proposed approach and how this change fits into the proposed solution.

Comment 1

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: Bug 1761828 - Propagate isHandlingUserInput from ProxyAPIImplementation.callAsyncFunction. r=robwu!,zombie

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4a1543e5d245
Propagate isHandlingUserInput from ProxyAPIImplementation.callAsyncFunction. r=robwu

Comment 3

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/4a1543e5d245

Comment 4

[Shane Hughes [:aminomancer]]

Hey Luca, can you look into this error? One of my extensions is doing what basically amounts to this:

browser.windows.onFocusChanged.addListener(
  async (updateTabInfo) => await browser.tabs.getCurrent()
);

And the extension is working perfectly fine but it's logging errors when I focus a window:

NS_ERROR_XPC_SECURITY_MANAGER_VETO: ExtensionChild.jsm:629
    callAsyncFunction resource://gre/modules/ExtensionChild.jsm:629
    stub resource://gre/modules/Schemas.jsm:2847
    loadLatestData moz-extension://e158f52d-b5db-4594-9757-1e526a5df811/newtab.js:187
    apply self-hosted:2719
    applySafeWithoutClone resource://gre/modules/ExtensionCommon.jsm:693
    fire resource://gre/modules/ExtensionChild.jsm:838
    recvRunListener resource://gre/modules/ExtensionChild.jsm:842
    recvRunListener self-hosted:1165
    _recv resource://gre/modules/ConduitsChild.jsm:82
    receiveMessage resource://gre/modules/ConduitsChild.jsm:188
    (Async: JSActor query)
    _send resource://gre/modules/ConduitsChild.jsm:65
    _send resource://gre/modules/ConduitsParent.jsm:290
    _send self-hosted:1269
    listener resource://gre/modules/ExtensionParent.jsm:1081
    apply self-hosted:2719
    applySafeWithoutClone resource://gre/modules/ExtensionCommon.jsm:693
    applySafe resource://gre/modules/ExtensionParent.jsm:553
    async resource://gre/modules/ExtensionCommon.jsm:2577
    (Async: promise callback)
    async resource://gre/modules/ExtensionCommon.jsm:2574
    listener chrome://browser/content/parent/ext-windows.js:66
    (Async: promise callback)
    listener chrome://browser/content/parent/ext-windows.js:59
    (Async: EventListener.handleEvent)
    _addWindowListener chrome://extensions/content/parent/ext-tabs-base.js:1853
    addListener chrome://extensions/content/parent/ext-tabs-base.js:1788
    onFocusChanged chrome://browser/content/parent/ext-windows.js:72
    onFocusChanged self-hosted:1165
    registerEventListener resource://gre/modules/ExtensionCommon.jsm:413
    register resource://gre/modules/ExtensionCommon.jsm:2253
    addListener resource://gre/modules/ExtensionCommon.jsm:2653
    addListener resource://gre/modules/ExtensionCommon.jsm:2717
    recvAddListener resource://gre/modules/ExtensionParent.jsm:1127
    InterpretGeneratorResume self-hosted:1472
    AsyncFunctionNext self-hosted:682
    (Async: async)
    recvAddListener self-hosted:1165
    _recv resource://gre/modules/ConduitsChild.jsm:82
    receiveMessage resource://gre/modules/ConduitsParent.jsm:450

which I guess the exception is sanitized and reported further downstream:

InvalidStateError: An exception was thrown newtab.js:187

Anyway, that ExtensionChild.jsm:629 is this line:

    const isHandlingUserInput =
      context.contentWindow?.windowUtils?.isHandlingUserInput;

Such a strange coincidence that I would just happen to notice this myself haha.

Here's the addon if that helps.

Comment 5

[Luca Greco [:rpl] [:luca] [:lgreco]]

hi Shane,
thanks for letting me know about this error logged from that line changed in the patch linked to this issue,
do you know which sequence of steps would trigger that consistently?
(I gave it a quick try, but installing the addon that you linked in comment 4, opened two windows with the extension newtab page loaded in both and switched the focus between the two windows, but I didn't trigger that error, and so I'm wondering what I may be missing from the STR I tried).

That seems to be an exception that may be raised from the xrays wrappers, but I'd like to trigger it locally and dig a bit more in which are the conditions that are triggering it.

I'd like to pin point the issue asap, given that the change is currently in beta and we would still have time to request an uplift for a fix.

Comment 6

[Shane Hughes [:aminomancer]]

Yeah I can't reproduce it anymore either. I tried a regression mozregression --addon "~\\[e-mail]" --pref devtools.chrome.enabled:true devtools.debugger.remote-enabled:true --bad=2022-05-07 but I haven't been able to find it again. I guess it was just a glitch in the matrix haha. I'll try to document it better in the event I do see it again though

Comment 7

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to Shane Hughes [:aminomancer] from comment #6)

Yeah I can't reproduce it anymore either. I tried a regression mozregression --addon "~\\[e-mail]" --pref devtools.chrome.enabled:true devtools.debugger.remote-enabled:true --bad=2022-05-07 but I haven't been able to find it again. I guess it was just a glitch in the matrix haha. I'll try to document it better in the event I do see it again though

In any case thanks for having reported right away and for trying again to get to an STR that reproduce it consistently, it was very much appreciated!

If you manage to reproduce it again, feel free to open a new issue (or add a comment to this one, especially if you think that there aren't yet enough details to file a new one) and needinfo me on it.

Comment 8

[Shane Hughes [:aminomancer]]

(In reply to Luca Greco [:rpl] [:luca] [:lgreco] from comment #7)

In any case thanks for having reported right away and for trying again to get to an STR that reproduce it consistently, it was very much appreciated!

If you manage to reproduce it again, feel free to open a new issue (or add a comment to this one, especially if you think that there aren't yet enough details to file a new one) and needinfo me on it.

Thanks, will do!

Comment 9

[Rob Wu [:robwu]]

Could it be that you've navigated elsewhere when that extension API was called? That could be a way to get the specific error under these circumstances.

Comment 10

[Shane Hughes [:aminomancer]]

(In reply to Rob Wu [:robwu] from comment #9)

Could it be that you've navigated elsewhere when that extension API was called? That could be a way to get the specific error under these circumstances.

Bingo! Dang you're good haha. Okay Luca here's some better STR

1. Install that extension
2. Open new tab
3. Type some web address in the urlbar
4. Hit enter
5. Hit Accel+T to open another new tab
6. The exception should be logged

Comment 11

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to Shane Hughes [:aminomancer] from comment #10)

(In reply to Rob Wu [:robwu] from comment #9)

Could it be that you've navigated elsewhere when that extension API was called? That could be a way to get the specific error under these circumstances.

Bingo! Dang you're good haha. Okay Luca here's some better STR

1. Install that extension
2. Open new tab
3. Type some web address in the urlbar
4. Hit enter
5. Hit Accel+T to open another new tab
6. The exception should be logged

Thank you so much!

Turned out that the extension page that is triggering the unexpected error is the one navigated to a webpage and not the new one opened at part of step 5 of the STR.

That seems to be only happening when fission is enabled, and it is actually unexpected and the change introduced by this bug is mainly making it more easily triggerable and more visible as a side effect of that.

It also explain why as you stated in comment 4, the extension still seems to be working fine even if that unexpected exception would have prevented the API method call to be executed successfully (which was another detail from comment 4 that I was wondering about).

I filed Bug 1768522 to follow up about that (but it is not technically a regression introduced by this bug, it may be consider it more as a WebExtensions-related regression introduced as a side-effect of enabling fission on all channels).

Comment 12

[Shane Hughes [:aminomancer]]

Yeah awesome, that and your WIP patch make perfect sense. Very interesting bug