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

Status: RESOLVED FIXED

(Firefox :: Extension Compatibility, defect)

Description

[artems777]

https://addons.mozilla.org/ru/firefox/addon/adguard-adblocker/

Comment 1

[Jorge Villalobos [:jorgev] (he/him)]

The developer has been notified through AMO.

Comment 2

[Andrey Meshkov]

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".

Comment 3

[Andrey Meshkov]

Looks like we should use nsICategoryManager from Compatibility shims to fix this on our side.

Comment 4

[Andrey Meshkov]

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.

Comment 5

[Chris Peterson [:cpeterson]]

Ally: does Andrey's nsIContentPolicy test case look like a Jetpack bug?

Comment 6

[Allison Naaktgeboren :ally]

It does.

Comment 7

[Tomislav Jovanovic :zombie]

(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?

Comment 8

[Andrey Meshkov]

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.

Comment 9

[Jorge Villalobos [:jorgev] (he/him)]

Andrey, are you still blocked by the issue on comment #8?

Comment 10

[Andrey Meshkov]

Hi Jorge!

We've just tested in the latest nightly and it seems like this bug is now fixed.

Comment 11

[Jorge Villalobos [:jorgev] (he/him)]

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.

Comment 12

[Andrey Meshkov]

You are in time:) Just uploaded it, version 2.0.4

Comment 13

[Jerod Lycett]

The latest version seems to work fine, poking Jorge.

Comment 14

[Andrey Meshkov]

Yep, we've fixed everything in 2.0.13.
Sorry, I should have left a comment here.

Thank you Jerod!

Comment 15

[Jorge Villalobos [:jorgev] (he/him)]

The latest version is now up on AMO.