"browserAction.setIcon: No tab with id" error after using block element in Chrome

[Ross]

Environment

ABP 3.0.2.1983
Chrome 49 / 62 / Windows 7

Could not reproduce in Firefox or Opera.

How to reproduce

1. Navigate to a website (any).
2. Select [ABP Icon] > [Block element].
3. Select an element (any) and OK the dialog.

Observed behaviour

The element is hidden as expected however the following error appears in the console (tab id varies):

    extensions::lastError:133 Unchecked runtime.lastError while running browserAction.setIcon: No tab with id: 34.
    reportIfUnchecked @ extensions::lastError:133
    handleResponse @ extensions::sendRequest:78

Expected behaviour

Error to not occur/be handled.

[kzar]

I can reproduce this, seems it's only when the filter generated is a request blocking rather than element hiding one.

[sebastian]

Reproduced, here, as well. This seems to be a regression since the last release for Chrome (1.13.5). However, if this is the only known regression, and since it doesn't seem to have any visible effects to the user, it seems ok if we fix this with Adblock Plus 3.0.4, rather than blocking the 3.0.3 release.

[sebastian]

It appears to be a race condition: When any filter is added (or removed) we update the icon for each tab and window (including the "Block element" dialog). By the time borwser.tabs.query({}) is called the "Block element" dialog still exists, but by the time we update the icon (a couple milliseconds later) it has been removed causing this error.

[jsonesen]

Replying to sebastian:

By the time borwser.tabs.query({}) is called the "Block element" dialog still exists, but by the time we update the icon (a couple milliseconds later) it has been removed causing this error.

In this case should we first check the dialogue exists? or do you think it is better to catch the error and continue?

[sebastian]

Though less likely, there can be other windows/tabs (besides the "Block element" dialog) getting removed in between detecting filter changes and updating the icon. So I don't think that simply ignoring the "Block element" dialog would be an appropriate solution.

I wonder if we can mitigate the race condition if call browser.borwserAction.set*() imediatelly rather than first ​checking if the tab exists first.

Otherwise, (while not great) silencing the error seems to be the best option left.

[jsonesen]

It looks like commenting out the tab.get block stops the error.

[sebastian]

We still need to handle the scenario of pre-rendered tabs, but perhaps we can just handle browserAction.set*() failing first to not defer the action unnecessarily in the common case.

[jsonesen]

It seems we already catch errors in setIcon() for Edge ​here perhaps I can check the error message at the start of the catch and silence the error? Should avoid common case handling too.

EDIT:
That's exception handling whoops, but adding a callback to the setIcon call which reads browser.runtime.lastError works and avoids running the check on all set* but doesn't avoid it for common setIcon case.

[jsonesen]

A commit referencing this issue has landed:
​Issue 6940 - Wrap browser.browserAction.set* with promise

[Ross]

Fixed. Error no longer occurs when using Block element.

ABP 3.4.3.2253
Firefox 65.0.1 / 51 / Windows 10
Firefox Mobile 65.0.1 / Android 7.1.1
Chrome 72.0.3626.109 / 49.0.2623.75 / Windows 10
Opera 58.0.3135.65 / 36.0.2130.80 / Windows 10
Edge 44.17763.1.0 / Windows 10