930787 - (e10s-abp) [e10s] Adblock Plus addon support for e10s

Status: RESOLVED FIXED

(Firefox :: Extension Compatibility, defect)

Description

[Chris Peterson [:cpeterson]]

https://addons.mozilla.org/en-US/firefox/addon/adblock-plus/

Comment 1

[Chris Peterson [:cpeterson]]

WFM, AFAICT :)

Comment 2

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

I'd like to keep this open to track the changes needed to convert adblock to use the message manager. It will make adblock faster and also fix some bugs. For example, "Open blockable items" isn't likely to work in e10s now.

Comment 3

[Wladimir Palant]

What's the timeline for this, could you point me to some documentation? From my previous experience with e10s, the required effort (both implementation and subsequent maintenance) will be massive because much of the code needs to be duplicated. This isn't something that can be done in a few hours so knowing how urgent it is would help a lot.

Comment 4

[Chris Peterson [:cpeterson]]

(In reply to Wladimir Palant from comment #3)
> What's the timeline for this, could you point me to some documentation? From
> my previous experience with e10s, the required effort (both implementation
> and subsequent maintenance) will be massive because much of the code needs
> to be duplicated. This isn't something that can be done in a few hours so
> knowing how urgent it is would help a lot.

Wladimar: e10s support is not urgent. e10s development will continue for much of 2014. So no hurries or worries! :)

I'm working with MDN to clean up our documentation, but here are some relevant links about the Message Manager (for async messaging between chrome and content processes) and Cross-Process Object Wrappers (CPOWs) for special cases that requite synchronous messages:

http://timtaubert.de/blog/2011/08/firefox-electrolysis-101/

https://developer.mozilla.org/en-US/docs/The_message_manager

https://developer.mozilla.org/en-US/docs/Cross_Process_Object_Wrappers

Comment 5

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

I'm going to close this. All the dependent bugs are fixed. If there's any more Adblock-related work to be done, we can open a new bug.

Comment 6

[The 8472]

ABP is listed as "it works" on http://arewee10syet.com/

I tested FF Nightly 34.0a1 (2014-08-21), linux x64 with both ABP 2.6.4 and their current development build and browser.tabs.remote.autostart = true.

In neither case the "open blockable items" view showed any entries after loading a page.
Disabling e10s restored functionality.

Comment 7

[Chris Peterson [:cpeterson]]

Thanks, 8472. I opened a new bug to track the issue you found: bug 1057467

Comment 8

[Robert Johnston]

(In reply to The 8472 from comment #6)
> ABP is listed as "it works" on http://arewee10syet.com/
> 
> I tested FF Nightly 34.0a1 (2014-08-21), linux x64 with both ABP 2.6.4 and
> their current development build and browser.tabs.remote.autostart = true.
> 
> In neither case the "open blockable items" view showed any entries after
> loading a page.
> Disabling e10s restored functionality.

Erm, this isn't "Fixed" at all. While it may not be crashing, ABP doesn't actually do anything (It doesn't block any ads, which is kinda it's raison d'être). It should definitely NOT be listed as "It works" on arewee10syet.com (as it doesn't). Can we re-open this, or should this discussion continue in #1057467 (and if so, shouldn't arewee10syet.com be updates to point to that bug?)

Comment 9

[Chris Peterson [:cpeterson]]

Robert: what websites are you testing? What version of ABP are you running? ABP 2.6.4 with e10s enabled works as expected for me on cnn.com and nytimes.com.

Comment 10

[Robert Johnston]

(In reply to Chris Peterson (:cpeterson) from comment #9)
> Robert: what websites are you testing? What version of ABP are you running?
> ABP 2.6.4 with e10s enabled works as expected for me on cnn.com and
> nytimes.com.

Here's a side-by-side view of CNN.com, left is e10s (Note the underline), right is normal. Note the "Death Row Stories" ad on the e10s version that's blocked on the normal one. Same thing happens on every other page I come across. Normal blocks the ads, e10s doesn't.

https://www.dropbox.com/s/7uq7palwcykqspj/Screenshot%202014-09-02%2013.08.27.png?dl=0

As mentioned, this is using ABP version 2.6.4.3861

Comment 11

[Chris Peterson [:cpeterson]]

(In reply to Robert Johnston from comment #10)
> Here's a side-by-side view of CNN.com, left is e10s (Note the underline),
> right is normal. Note the "Death Row Stories" ad on the e10s version that's
> blocked on the normal one. Same thing happens on every other page I come
> across. Normal blocks the ads, e10s doesn't.

Which Adblock Plus filter lists are you using? When I installed the ABP development version (2.6.4.3861) over the stable version, some of my filter lists (like EasyList) seemed to have gotten disabled. But when I created a new, clean profile and installed the ABP development version, the EasyList was enabled as the default filter list, as expected. Can you verify that your filter lists are enabled?

> As mentioned, this is using ABP version 2.6.4.3861

oh, you hadn't mentioned you were testing the development version. Can you reproduce the problem with the stable version (2.6.4) published on addons.mozilla.org? Or in a new profile?

Comment 12

[Robert Johnston]

(In reply to Chris Peterson (:cpeterson) from comment #11)
> (In reply to Robert Johnston from comment #10)
> > Here's a side-by-side view of CNN.com, left is e10s (Note the underline),
> > right is normal. Note the "Death Row Stories" ad on the e10s version that's
> > blocked on the normal one. Same thing happens on every other page I come
> > across. Normal blocks the ads, e10s doesn't.
> 
> Which Adblock Plus filter lists are you using? When I installed the ABP
> development version (2.6.4.3861) over the stable version, some of my filter
> lists (like EasyList) seemed to have gotten disabled. But when I created a
> new, clean profile and installed the ABP development version, the EasyList
> was enabled as the default filter list, as expected. Can you verify that
> your filter lists are enabled?

Both lists (Easylist and Warning Removal List) are enabled and working. As mentioned, this was the same Firefox session running with 2 windows on a dual monitor computer, one using e10s and one not. It's not cut together, both windows were open at the same time, in the same browser, on the same page. the only difference was that one had e10s enabled, and the other didn't.

> > As mentioned, this is using ABP version 2.6.4.3861
> 
> oh, you hadn't mentioned you were testing the development version. Can you
> reproduce the problem with the stable version (2.6.4) published on
> addons.mozilla.org? Or in a new profile?

I was using 2.6.4 stable and had this issue. I figured that the development version would fix it, or at least be a little more up-to-date so any other issues wouldn't be interfering.

Comment 13

[Chris Peterson [:cpeterson]]

(In reply to Robert Johnston from comment #12)
> Both lists (Easylist and Warning Removal List) are enabled and working. As
> mentioned, this was the same Firefox session running with 2 windows on a
> dual monitor computer, one using e10s and one not. It's not cut together,
> both windows were open at the same time, in the same browser, on the same
> page. the only difference was that one had e10s enabled, and the other
> didn't.

To enable "full e10s", you must set the about:config pref "browser.tabs.remote.autostart" to true and restart Nightly. The "File > Open e10s Window" menu item does open a new e10s process, but it does not enable the add-on compatibility checks that "browser.tabs.remote.autostart" pref does. We should probably remove the "Open e10s Window" menu item because it is old test code.

Comment 14

[Robert Johnston]

(In reply to Chris Peterson (:cpeterson) from comment #13)
> (In reply to Robert Johnston from comment #12)
> > Both lists (Easylist and Warning Removal List) are enabled and working. As
> > mentioned, this was the same Firefox session running with 2 windows on a
> > dual monitor computer, one using e10s and one not. It's not cut together,
> > both windows were open at the same time, in the same browser, on the same
> > page. the only difference was that one had e10s enabled, and the other
> > didn't.
> 
> To enable "full e10s", you must set the about:config pref
> "browser.tabs.remote.autostart" to true and restart Nightly. The "File >
> Open e10s Window" menu item does open a new e10s process, but it does not
> enable the add-on compatibility checks that "browser.tabs.remote.autostart"
> pref does. We should probably remove the "Open e10s Window" menu item
> because it is old test code.

I was actually setting that pref to true and opening a new window. However, toggling that preference and restarting the browser leads to exactly the same situation - When e10s is enabled (browser.tabs.remote.autostart = true), ads are not blocked. When e10s is disabled (browser.tabs.remote.autostart = false), ads are blocked as usual.

Comment 15

[Wladimir Palant]

For me, ads are being blocked with |browser.tabs.remote.autostart = true|. However, Blockable items are broken. The following line triggers "unable to modify interposed property content" message:

>  Object.defineProperty(window, "content", { get: () => getBrowser(mainWin).contentWindow });

Note that this code isn't running in the browser window - it's a frame inside the browser window, one where the content property normally shouldn't be defined. With e10s enabled it is defined however (value is null) and apparently cannot be redefined.

Comment 16

[Wladimir Palant]

I filed https://issues.adblockplus.org/ticket/1301 to clean up this code and stop faking window.content property. However, the priority of the issue is very low. I can raise it if not being able to redefine window.content isn't considered an issue worth fixing on Mozilla's end.

Comment 17

[Robert Johnston]

If it helps (it probably won't), the Browser Console when opening cnn.com with e10s enabled looks like this:

NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIProperties.get] FileUtils.jsm:63
Permission denied to pass object to chrome
Permission denied to pass object to chrome
uncaught exception: undefined
uncaught exception: undefined
Permission denied to pass object to chrome
uncaught exception: undefined
Permission denied to pass object to chrome
uncaught exception: undefined
TypeError: window.safari is undefined safaripush.js:102
Permission denied to pass object to chrome
uncaught exception: undefined
Permission denied to pass object to chrome
Permission denied to pass object to chrome
uncaught exception: undefined
Permission denied to pass object to chrome
Permission denied to pass object to chrome
Permission denied to pass object to chrome
Permission denied to pass object to chrome
uncaught exception: undefined
Permission denied to pass object to chrome
uncaught exception: undefined
uncaught exception: undefined
uncaught exception: undefined
Permission denied to pass object to chrome
Permission denied to pass object to chrome
uncaught exception: undefined
Permission denied to pass object to chrome
uncaught exception: undefined
uncaught exception: undefined
Permission denied to pass object to chrome
Permission denied to pass object to chrome
Use of Mutation Events is deprecated. Use MutationObserver instead. vrs.js:1
uncaught exception: undefined
Permission denied to pass object to chrome
uncaught exception: undefined
Permission denied to pass object to chrome
uncaught exception: undefined
Permission denied to pass object to chrome
uncaught exception: undefined
Permission denied to pass object to chrome
uncaught exception: undefined
Error: Permission denied to access property 'toString'
Permission denied to pass object to chrome
uncaught exception: undefined
Permission denied to pass object to chrome
uncaught exception: undefined
Permission denied to pass object to chrome
uncaught exception: undefined
ReferenceError: cnnad_id is not defined hplib-min.js:23
Permission denied to pass object to chrome
uncaught exception: undefined
Permission denied to pass object to chrome
uncaught exception: undefined
Use of getPreventDefault() is deprecated.  Use defaultPrevented instead. jquery-1.7.2.min.js:3
Empty string passed to getElementById(). hplib-min.js:11
Permission denied to pass object to chrome
Permission denied to pass object to chrome
Permission denied to pass object to chrome
uncaught exception: undefined
Permission denied to pass object to chrome
Permission denied to pass object to chrome
uncaught exception: undefined
Permission denied to pass object to chrome
uncaught exception: undefined
Permission denied to pass object to chrome
uncaught exception: undefined
Permission denied to pass object to chrome
uncaught exception: undefined

Doing the same with e10s disabled produces the following:

content is null tab-events.js:40
Using //@ to indicate sourceMappingURL pragmas is deprecated. Use //# instead jquery-1.10.2.min.js:1
TypeError: window.safari is undefined safaripush.js:102
content is null tab-events.js:40
Using //@ to indicate sourceMappingURL pragmas is deprecated. Use //# instead jquery-1.10.2.min.js:1
Use of Mutation Events is deprecated. Use MutationObserver instead. vrs.js:1
content is null tab-events.js:40
Use of getPreventDefault() is deprecated.  Use defaultPrevented instead. jquery-1.7.2.min.js:3
"repainting..." hplib-min.js:23
"ad-48df790ef051ce72" hplib-min.js:23
"ad-76fb9165d4dc83bd" hplib-min.js:23
"ad-9c86913b431b114f" hplib-min.js:23
"ad-3ba9728dd75a6f0e" hplib-min.js:23
"ad-977b69f1d2ca7a72" hplib-min.js:23
"ad-691428c40a9e49e6" hplib-min.js:23
"ad-60ea2f29a4a38689" hplib-min.js:23
"ad-f9de57574daeedc1" hplib-min.js:23
"ad-f72763bd1282af92" hplib-min.js:23
"ad-6310e5bc3ba9cc0f" hplib-min.js:23
content is null tab-events.js:40
Empty string passed to getElementById(). hplib-min.js:11
content is null tab-events.js:40
Using //@ to indicate sourceMappingURL pragmas is deprecated. Use //# instead jquery-1.10.2.min.js:1
content is null tab-events.js:40
Using //@ to indicate sourceMappingURL pragmas is deprecated. Use //# instead jquery-1.10.2.min.js:1
content is null tab-events.js:40
Using //@ to indicate sourceMappingURL pragmas is deprecated. Use //# instead jquery-1.10.2.min.js:1
content is null tab-events.js:40
Using //@ to indicate sourceMappingURL pragmas is deprecated. Use //# instead jquery-1.10.2.min.js:1
content is null tab-events.js:40
Using //@ to indicate sourceMappingURL pragmas is deprecated. Use //# instead jquery-1.10.2.min.js:1
content is null tab-events.js:40

Comment 18

[Leman Bennett [Omega]]

This bug should be reopened as it does not function with e10s enabled.

Comment 19

[Axel]

Yep, not working with Nightly 35.0a1 (2014-09-11), element hiding works but blocking does not, see http://simple-adblock.com/faq/testing-your-adblocker/ (matching with my personal experience, some things are blocked, others are not)

Comment 20

[Chris Peterson [:cpeterson]]

This bug is a meta bug. Let's track this regression in bug 1042710.

Comment 21

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

I filed bug 1066837 to fix the problem in comment 16.

I'm not sure what to do about these other reports. I can't reproduce them myself. If you're having problems, can you try a fresh profile? Then we could try to figure out what exactly is causing the problem.

Comment 22

[Axel]

@billm - good point, checked back today, it works at home on my Linux (all green on testing-your-adblocker and a clean spiegel.de). going to try a new profile on Monday on Windows in the office and report back. (I'll keep a copy of the old profile for further investigations, though)

Comment 23

[Leman Bennett [Omega]]

(In reply to Bill McCloskey (:billm) from comment #21)
> I filed bug 1066837 to fix the problem in comment 16.
> 
> I'm not sure what to do about these other reports. I can't reproduce them
> myself. If you're having problems, can you try a fresh profile? Then we
> could try to figure out what exactly is causing the problem.

I just poked at it again and found something interesting:

- Turn on e10s by default. Restart. Works, Ads are blocked.

- Turn off e10s, open up an e10s window. Ads are not blocked.

Reproducible every time.

Comment 24

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

(In reply to Leman Bennett [Omega] from comment #23)
> (In reply to Bill McCloskey (:billm) from comment #21)
> > I filed bug 1066837 to fix the problem in comment 16.
> > 
> > I'm not sure what to do about these other reports. I can't reproduce them
> > myself. If you're having problems, can you try a fresh profile? Then we
> > could try to figure out what exactly is causing the problem.
> 
> I just poked at it again and found something interesting:
> 
> - Turn on e10s by default. Restart. Works, Ads are blocked.
> 
> - Turn off e10s, open up an e10s window. Ads are not blocked.
> 
> Reproducible every time.

Yes, please see comment 13. That is a known issue.

Comment 25

[Axel]

New profile solved it for me.
Cheers, A.

Comment 26

[Allison Naaktgeboren :ally]

Re-opening. Telemetry indicates that adp regularly throws an exception in the file contentPolicy.js line 404.

This makes up nearly a fifth of all reported addon js exceptions.

Comment 27

[Allison Naaktgeboren :ally]

We are also seeing contentPolicy.js line 625 throw an exception.

Comment 28

[Allison Naaktgeboren :ally]

The file objectTabs.js line 302 also throws as well.

Comment 29

[Allison Naaktgeboren :ally]

objectTabs.js also throws an exception on line 364.

Comment 30

[Sampick69]

it no longer blocks the pub

Comment 31

[csamborski]

I confirm that Ad Block Plus does not block anything on Firefox 40 (currently Aurora) with e10s activated.

Comment 32

[laz2727]

Confirm that ABP works with latest Nightly.

Comment 33

[Sampick69]

(In reply to laz2727 from comment #32)
> Confirm that ABP works with latest Nightly.

Yep it works for me ;)

Comment 34

[Tracy Walker [:tracy]]

Since this is the meta bug for Adblock Block Plus adding some tracking information:

Assignee: :tracy
Link to add-on: https://addons.mozilla.org/en-US/firefox/addon/adblock-plus/?src=ss
Contact info for add-on: https://addons.mozilla.org/en-US/firefox/user/Wladimir-Palant/
- http://adblockplus.org/
Bug #: This bug 930787 and depends bugs
Add-on ID: {d10d0bf8-f5b5-c8b4-a8b2-2b9879e08c5d}
How well does it work?: 60%
See depends bugs
Any obvious performance problems? likely to have performance problems as this uses CPOWs heavily.
Chromium version: https://chrome.google.com/webstore/detail/adblock-plus/cfhdojbkjhnklbpkdaibdccddilifddb

Comment 35

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Tracy, a new version of Adblock was released that should have much better performance. Could you go through the performance-related dependencies here and close them if you can't reproduce?

Comment 36

[Tracy Walker [:tracy]]

I'll look at the dependencies, but within a minute or so of installing ABP 2.7, on latest Nightly, I got the "Adblock Plus might be making Nightly run slowly" warning bar.

Comment 37

[Marco Castelluccio [:marco]]

> I'll look at the dependencies, but within a minute or so of installing ABP 2.7, on latest Nightly, I got the "Adblock Plus might be making Nightly run slowly" warning bar.

I'm still getting the slow addon warning all the time for ABP.

Comment 38

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Hmm, that suggests a false positive in the slow add-on warning. As far as I know, Adblock Plus 2.7 doesn't use any CPOWs unless you do some unusual things.

David, can you look into this?

Comment 39

[Marco Castelluccio [:marco]]

Looks like the slow addon warning isn't triggered if you disable the "Count filter hits" option (which is enabled by default).
I haven't used Firefox enough since I changed that pref, so I can't really say for certain.

Comment 40

[Tracy Walker [:tracy]]

Yes, it appears the remaining perf issue is centered on the "Count filter hits" option. Disabling it dramatically improves page load performance at reported sites like http://www.jshine.net/astronomy/dark_sky/ (bug 1156124) and https://en.wikipedia.org/wiki/List_of_country_calling_codes (bug 1192585)

Comment 41

[Marco Castelluccio [:marco]]

(In reply to Marco Castelluccio [:marco] from comment #39)
> Looks like the slow addon warning isn't triggered if you disable the "Count
> filter hits" option (which is enabled by default).
> I haven't used Firefox enough since I changed that pref, so I can't really
> say for certain.

I just got the slow addon warning again, even with "Coult filter hits" disabled.
Firefox is noticeably faster at loading pages anyway with that option disabled.

Comment 42

[Wladimir Palant]

(In reply to [:tracy] Tracy Walker - QA Mentor from comment #40)
> Yes, it appears the remaining perf issue is centered on the "Count filter
> hits" option. Disabling it dramatically improves page load performance at
> reported sites like http://www.jshine.net/astronomy/dark_sky/ (bug 1156124)
> and https://en.wikipedia.org/wiki/List_of_country_calling_codes (bug 1192585)

This is weird, the code path dependent on this setting never leaves the chrome process - no additional IPC traffic, no reason why it should be different with E10S. It does cause additional I/O however, particularly on sites with many hits, so my immediate suspect would be OS.File being significantly slower with E10S for some reason. I'll look into this.

Comment 43

[Wladimir Palant]

I filed https://issues.adblockplus.org/ticket/3473 on this issue (not related to E10S at all), it should be resolved soon. Adblock Plus 2.7.1 is to be released on January 19, by then we will hopefully stop using shims altogether.

Comment 44

[David Teller [:Yoric] - still alive but not very active]

From the top of my head, I can't think of any reason why OS.File would be slower than e10s, but hey, it's a brand new world in which we have little experience so far. Wladimir, if you find anything, I'm definitely interested. Note that it doesn't have to be a CPOW to cause the slow add-on warning, just a large use of CPU will also trigger it.

Just to be sure, when you write that the code path doesn't leave the chrome process, have you checked if it doesn't indirectly perform a CPOW call somewhere?

Finally, I'm working on my side to try and bring down not-user-visible slowdown alerts.

Comment 45

[Wladimir Palant]

That was merely a guess, nothing that I could confirm. The code path triggered a few hundred file writes due to a bug on our end (already fixed in the latest development build, will be fixed with Adblock Plus 2.7.1 for stable releases). There is no IPC, CPOW or anything else E10S-related involved. I'm not sure why this issue got more visible with E10S, probably simply because of the chrome process being blocked and unable to respond to messages

Comment 46

[Andy McKay]

AdBlock Plus is being ported to a WebExtension in bug 1226547. Don't think this bug serves much value anymore.