Closed Bug 1068555 Opened 10 years ago Closed 9 years ago

"adguard-adblocker" add-on does not work with e10s

Categories

(Firefox :: Extension Compatibility, defect)

x86_64
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
e10s + ---

People

(Reporter: artems777, Unassigned)

References

()

Details

(Keywords: addon-compat)

Version: 35 Branch → unspecified
Summary: adguard-adblocker add-on does not work with e10s → "adguard-adblocker" add-on does not work with e10s
Blocks: e10s-addons
The developer has been notified through AMO.
Hello! We've tested Adguard with e10s and the bug is rather weird. Looks like our implementation of nsIContentPolicy does not work for most of requests. Firefox does not call shouldLoad method for HTTP requests. But it does call it for requests like "about:newtab" or "chrome://global/skin/icons/close.png".
Looks like we should use nsICategoryManager from Compatibility shims to fix this on our side.
We've looked into it and it looks like there is a bug with bootstrapped add-ons. Here are two test add-ons: https://www.dropbox.com/sh/e8g3kurnqqegqh9/AAAX8eStAZ-anLluJ7cj0RNta?dl=0 They are very simple. Both implement nsIContentPolicy and write intercepted requests to log. TestContentPolicySdk: sdk add-on built with cfx TestContentPolicyBootstrap: bootstrapped add-on If browser.tabs.remote.autostart is disabled both add-ons works ok and nsIContentPolicy works for all requests. If browser.tabs.remote.autostart is enabled: 1. TestContentPolicySdk is ok 2. TestContentPolicyBootstrap is not. shouldLoad does not called for HTTP requests.
Ally: does Andrey's nsIContentPolicy test case look like a Jetpack bug?
Flags: needinfo?(ally)
It does.
Flags: needinfo?(tomica+amo)
Flags: needinfo?(ally)
(In reply to Chris Peterson (:cpeterson) from comment #5) > Ally: does Andrey's nsIContentPolicy test case look like a Jetpack bug? Andrey says it works in the SDK addon, but not in a plain bootstrap one: (In reply to Andrey Meshkov from comment #4) > If browser.tabs.remote.autostart is enabled: > 1. TestContentPolicySdk is ok > 2. TestContentPolicyBootstrap is not. shouldLoad does not called for HTTP > requests. which is weird. SDK doesn't have to do anything with nsIContentPolicy, as it's not an API we provide, so using it from an SDK addon should behave the same as using it from any other bootstrap addon (currently, SDK addons are nothing more than bootstrap addons with some modules. the only thing that comes to mind is that the shims are not loaded, or not _yet_ loaded by the time the bootstrap code runs? Andrey, can you please re-test with this in mind? maybe delay the running of your main addon code to see if we have a bug in when shims are loaded?
Flags: needinfo?(tomica+amo) → needinfo?(am)
Hi Tomislav! 1. Sorry, I've confused add-on types in my previous message. TestContentPolicyBootstrap is ok, but TestContentPolicySdk is not. 2. We've done some debugging and found out the place where content policy is added. RemoteAddonsParent.jsm: ------------------------------ CategoryManagerInterposition.methods.addCategoryEntry = function(addon, target, category, entry, value, persist, replace) { if (category == "content-policy") { ContentPolicyParent.addContentPolicy(entry); } target.addCategoryEntry(category, entry, value, persist, replace); }; In case of bootstrapped add-on this method is called and everything works ok. In case of SDK add-on this method is not called at all. We've tried to add 5 sec timeout but nothing has changed.
Flags: needinfo?(am)
Andrey, are you still blocked by the issue on comment #8?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(am)
Hi Jorge! We've just tested in the latest nightly and it seems like this bug is now fixed.
Flags: needinfo?(am)
That's great news. Let us know when the new version has been submitted to AMO for review and I'll try to give it priority. We'll close this bug when the fixed version is publicly available.
You are in time:) Just uploaded it, version 2.0.4
The latest version seems to work fine, poking Jorge.
Flags: needinfo?(jorge)
Yep, we've fixed everything in 2.0.13. Sorry, I should have left a comment here. Thank you Jerod!
The latest version is now up on AMO.
Status: NEW → RESOLVED
Closed: 9 years ago
Flags: needinfo?(jorge)
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.