#3208 closed change (fixed)

Inject our nsIContentPolicy implementation into all processes

Reported by: trev Assignee: trev
Priority: P2 Milestone: Adblock-Plus-2.7-for-Firefox
Module: Adblock-Plus-for-Firefox Keywords: e10s 2015q4
Cc: arthur Blocked By: #3222, #3223, #3224, #3226, #3228, #3251, #3274
Blocking: Platform: Firefox
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29329367/
https://codereview.adblockplus.org/29329390/
https://codereview.adblockplus.org/29329393/
https://codereview.adblockplus.org/29329399/
https://codereview.adblockplus.org/29329467/
https://codereview.adblockplus.org/29329550/
https://codereview.adblockplus.org/29329554/
https://codereview.adblockplus.org/29329396/
https://codereview.adblockplus.org/29329409/
https://codereview.adblockplus.org/29329534/
https://codereview.adblockplus.org/29329419/
https://codereview.adblockplus.org/29329450/
https://codereview.adblockplus.org/29329470/
https://codereview.adblockplus.org/29329527/
https://codereview.adblockplus.org/29329547/
https://codereview.adblockplus.org/29329562/
https://codereview.adblockplus.org/29329565/
https://codereview.adblockplus.org/29329568/
https://codereview.adblockplus.org/29329486/
https://codereview.adblockplus.org/29329571/
https://codereview.adblockplus.org/29329680/

Description (last modified by trev)

Background

We are currently registering our content policy in the main process only. This works thanks to the E10S compatibility shim but causes lots of CPOW traffic as we inspect the context of the requests - along with warnings in the Error Console and slowdowns.

What to change

Inject our nsIContentPolicy implementation into all processes, similar to how it was done for the about: module in #3108. Use synchronous messaging to ask a central instance to make a decision. Data should be attached to nodes in the content process, meaning that RequestNotifier needs to be reworked as well.

In particular, this means splitting up our current contentPolicy module into contentPolicy and child/contentPolicy. The latter should contain all the low-level code: actual content policy implementation, redirect and pop-up listeners. The remaining code in contentPolicy would only be responsible for making decisions when messaged from child/contentPolicy (synchronously, because content policy responses have to be synchronous).

Change History (48)

comment:1 Changed 22 months ago by trev

  • Owner set to trev

comment:2 Changed 22 months ago by trev

  • Description modified (diff)

comment:3 Changed 22 months ago by trev

  • Blocking 3222 added

comment:4 Changed 22 months ago by trev

  • Blocking 3223 added

comment:5 Changed 22 months ago by trev

  • Blocking 3224 added

comment:6 Changed 22 months ago by trev

  • Blocking 3225 added

comment:7 Changed 22 months ago by trev

  • Blocking 3226 added

comment:8 Changed 22 months ago by trev

  • Blocking 3228 added

comment:9 Changed 22 months ago by trev

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:10 Changed 22 months ago by trev

  • Blocking 443 added

comment:11 Changed 22 months ago by trev

  • Blocking 3222, 3223, 3224, 3225, 3226, 3228 removed

comment:12 Changed 22 months ago by trev

  • Blocked By 3222, 3223, 3224, 3225, 3226, 3228 added

comment:13 Changed 22 months ago by trev

  • Review URL(s) modified (diff)

Started splitting up the large change into many small ones.

comment:14 Changed 22 months ago by trev

  • Review URL(s) modified (diff)

comment:15 Changed 22 months ago by trev

  • Review URL(s) modified (diff)

comment:16 Changed 22 months ago by trev

  • Review URL(s) modified (diff)

comment:17 Changed 22 months ago by trev

  • Review URL(s) modified (diff)

comment:18 Changed 22 months ago by trev

  • Blocking 443 removed

comment:19 Changed 22 months ago by trev

  • Review URL(s) modified (diff)

comment:20 Changed 22 months ago by trev

  • Review URL(s) modified (diff)

comment:21 Changed 22 months ago by trev

  • Review URL(s) modified (diff)

comment:22 Changed 22 months ago by trev

  • Review URL(s) modified (diff)

comment:23 Changed 22 months ago by trev

  • Review URL(s) modified (diff)

comment:24 Changed 22 months ago by trev

  • Review URL(s) modified (diff)

comment:25 Changed 22 months ago by trev

  • Review URL(s) modified (diff)

comment:26 Changed 22 months ago by trev

  • Review URL(s) modified (diff)

comment:27 Changed 22 months ago by trev

  • Review URL(s) modified (diff)

comment:28 Changed 22 months ago by trev

  • Review URL(s) modified (diff)

comment:29 Changed 22 months ago by trev

  • Review URL(s) modified (diff)

comment:30 Changed 22 months ago by trev

  • Review URL(s) modified (diff)

comment:31 Changed 22 months ago by trev

  • Review URL(s) modified (diff)

comment:32 Changed 22 months ago by trev

  • Review URL(s) modified (diff)

That should be the last review...

comment:33 Changed 22 months ago by trev

  • Blocked By 3251 added

comment:36 Changed 22 months ago by trev

  • Review URL(s) modified (diff)

comment:39 Changed 22 months ago by trev

  • Blocked By 3274 added

comment:40 Changed 22 months ago by trev

  • Blocking 3213 added

comment:41 Changed 22 months ago by mario

  • Keywords 2015q4 added

comment:42 Changed 22 months ago by trev

  • Blocking 3290 added

comment:43 Changed 22 months ago by arthur

  • Cc arthur added

comment:45 Changed 22 months ago by trev

  • Blocking 3213 removed

comment:46 Changed 22 months ago by trev

  • Review URL(s) modified (diff)

Pushed unit tests change: https://hg.adblockplus.org/adblockplustests/rev/94a67e14fda9

The remaining changes landed on a branch and will be merged once the blockers are resolved.

comment:47 Changed 21 months ago by trev

  • Blocking 3290 removed

comment:48 Changed 21 months ago by trev

  • Blocked By 3225 removed
  • Milestone set to Adblock-Plus-for-Firefox-next
  • Resolution set to fixed
  • Status changed from reviewing to closed

This is now fixed in the development builds.

Note: See TracTickets for help on using tickets.