[webextension] "response is undefined" exceptions being thrown

[kzar]

Environment

Firefox 58, Adblock Plus devenv built from ​8399e34fff05, Debian Buster

How to reproduce

1. Create a new tab and open the console for it
2. Browse to ​https://reddit.com

Observed behaviour

Lots of exceptions showing in the console:

response is undefined  include.preload.js:682

Expected behaviour

Exceptions now being shown.

Notes

This is a regression caused by ​8399e34fff05.

When initialising element hiding include.preload.js sends an "elemhide.getSelectors" message and then attempts to access the trace property of the response. However response is sometimes undefined. This happens when updateFrameStyles gets undefined back when attempting to get the frame using ext.getFrame. That doesn't seem to be a bug, framesOfTabs really doesn't have the frame in question, but I'm not sure why that is.

[mjethani]

I dug a little deeper into this. The background page gets onHeadersReceived after it gets the elemhide.getSelectors message from the content script, so it doesn't have the frame at this point. The right thing to do here would be to give up adding of the style sheet at this point entirely (rather than adding it to the DOM) and then add it when we get the onHeadersReceived event. This would involve sending a message to the content script when we've added the style sheet, so it can do its tracing/logging/whatever.

At the very least we could rerun the code for elemhide.getSelectors when we get onHeadersReceived.

[abpbot]

A commit referencing this issue has landed:
​Issue 6385 - Don't attempt to inject styles for frames we can't find

[kzar]

(I found this workaround was also necessary to get ad blocking to work in Edge at all!)

[sebastian]

I still keep getting response is undefined include.preload.js:700, right after the extension loads. This might be unrelated though, as it doesn't seem to be a regression. I even get the same with Adblock Plus 3.0. My suspicion is that the content scripts send elemhide.getSelectors messages from the existing tabs before the background page is ready to respond. IIRC, Firefox injects content scripts in existing tabs when loading the extension. For reference, I'm using Firefox 58.

[Ross]

Fixed. The message still occurs after the extension loads as noted above. Opened as #6498.

ABP 3.0.2.1983
Firefox 53 / 58 / Windows 7
Chrome 49 / 65 / Windows 7
Opera 36 / 51 / Windows 7

[mjethani]

Replying to mjethani:

I dug a little deeper into this. The background page gets onHeadersReceived after it gets the elemhide.getSelectors message from the content script, so it doesn't have the frame at this point. The right thing to do here would be to give up adding of the style sheet at this point entirely (rather than adding it to the DOM) and then add it when we get the onHeadersReceived event. This would involve sending a message to the content script when we've added the style sheet, so it can do its tracing/logging/whatever.

At the very least we could rerun the code for elemhide.getSelectors when we get onHeadersReceived.

Just a thought, but I wonder if #6507 also fixes this properly, because I think the fix we implemented was still kind of a short-term hack and did not address the underlying issue.

[Himanshu0709]

spam

[goklopp]

spam