Opened on 03/01/2018 at 05:05:20 PM
Closed on 03/16/2019 at 03:32:13 PM
#6437 closed change (fixed)
Optimize element hiding emulation for DOM modifications
| Reported by: | mjethani | Assignee: | mjethani |
|---|---|---|---|
| Priority: | P2 | Milestone: | |
| Module: | Core | Keywords: | circumvention |
| Cc: | hfiguiere, kzar, sergz, amrmak, arthur | Blocked By: | #6618, #6680 |
| Blocking: | #6422, #6504, #7000 | Platform: | Unknown / Cross platform |
| Ready: | yes | Confidential: | no |
| Tester: | Ross | Verified working: | yes |
| Review URL(s): |
https://codereview.adblockplus.org/29712655/ |
||
Description (last modified by mjethani)
Background
Element hiding emulation has an optimization for the case where only a new style sheet has been added to the document. In this case it does not do any processing for patterns containing only -abp-has and -abp-contains selectors. However this optimization does not exist for the other way. i.e. when there's a DOM mutation but no style sheet changes, -abp-properties selectors are still processed, even though they should be skipped (unless nested inside -abp-has of course). The code unnecessarily parses each CSS rule within each style sheet in the entire document for every batch of DOM mutations reported by MutationObserver.
Given that DOM mutations occur way more frequently than style sheet updates, this case should be optimized for first and foremost.
What to change
Implement optimized paths for DOM mutations without style sheet changes. In particular, patterns that do not contain any DOM-dependent selectors (e.g. div:-abp-properties(width: 100px;)) should not be processed. If there are only such patterns available and no others, the document's style sheets should not be parsed.
Needless to say, this should be done without breaking any functionality.
Hints for testers
Make sure that element hiding emulation works as it did before these optimizations.
Attachments (0)
Change History (25)
comment:1 Changed on 03/02/2018 at 01:56:48 PM by mjethani
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:3 Changed on 03/07/2018 at 04:27:13 PM by mjethani
- Keywords circumvention added
comment:4 Changed on 03/07/2018 at 04:28:01 PM by mjethani
- Blocking 6422 added
comment:5 Changed on 03/08/2018 at 09:22:59 PM by amrmak
- Cc amrmak added
comment:6 Changed on 03/19/2018 at 08:04:19 PM by kzar
- Priority changed from Unknown to P2
- Ready set
comment:7 Changed on 03/20/2018 at 08:55:28 AM by abpbot
A commit referencing this issue has landed:
Issue 6437 - Skip styles-only patterns on DOM mutations
comment:9 Changed on 03/20/2018 at 11:15:19 AM by mjethani
- Blocking 6504 added
comment:10 Changed on 03/22/2018 at 04:34:13 PM by mjethani
- Review URL(s) modified (diff)
comment:11 Changed on 03/31/2018 at 02:22:08 PM by mjethani
- Review URL(s) modified (diff)
comment:12 Changed on 04/18/2018 at 06:20:20 AM by abpbot
A commit referencing this issue has landed:
Issue 6437 - Do not override scheduled full processing
comment:13 Changed on 04/19/2018 at 07:22:40 PM by abpbot
A commit referencing this issue has landed:
Issue 6437 - Apply :-abp-contains() to matching roots only
comment:14 Changed on 04/24/2018 at 12:55:32 PM by mjethani
- Blocked By 6618 added
comment:15 Changed on 04/30/2018 at 02:23:10 PM by abpbot
A commit referencing this issue has landed:
Issue 6437 - Filter out patterns that do not match DOM mutations
comment:16 Changed on 05/14/2018 at 08:44:45 AM by arthur
- Cc arthur added
comment:17 Changed on 06/26/2018 at 07:04:38 PM by abpbot
A commit referencing this issue has landed:
Issue 6437 - Skip elements not affected by DOM mutations
comment:18 Changed on 07/05/2018 at 12:06:40 PM by Ross
- Tester changed from Unknown to Ross
- Verified working set
This has been done and extended element hiding filters still look to be working as expected.
ABP 3.1.0.2069
Chrome 67 / 64 / 49 / Windows 7
Firefox 60 / 55 / 51 / Windows 7
Opera 52 / 45 / 38 / Windows 7
comment:19 Changed on 07/20/2018 at 12:21:47 PM by mjethani
- Description modified (diff)
comment:20 Changed on 07/24/2018 at 07:35:39 PM by mjethani
- Blocked By 6680 added
comment:21 Changed on 09/30/2018 at 09:49:20 AM by mjethani
- Blocking 7000 added
comment:22 Changed on 09/30/2018 at 09:53:25 AM by mjethani
- Blocking 7000 removed
comment:23 Changed on 09/30/2018 at 09:53:53 AM by mjethani
- Blocking 7000 added
comment:24 Changed on 03/16/2019 at 03:31:58 PM by mjethani
We made a number of changes here that have already shipped. We are not going to make any more changes for this. Accordingly I am closing this ticket.
comment:25 Changed on 03/16/2019 at 03:32:13 PM by mjethani
- Resolution set to fixed
- Status changed from reviewing to closed
Adding the circumvention tag since this is going to be more important once we switch to user style sheets for all emulation hiding filters (#6422).