Opened 9 months ago

Last modified 7 weeks ago

#6437 reviewing change

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/
https://codereview.adblockplus.org/29713565/
https://codereview.adblockplus.org/29714601/
https://codereview.adblockplus.org/29728579/
https://codereview.adblockplus.org/29730630/
https://codereview.adblockplus.org/29738589/

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.

Change History (23)

comment:1 Changed 9 months ago by mjethani

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

comment:2 Changed 9 months ago by mjethani

  • Review URL(s) modified (diff)

comment:3 Changed 8 months ago by mjethani

  • Keywords circumvention added

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

comment:4 Changed 8 months ago by mjethani

  • Blocking 6422 added

comment:5 Changed 8 months ago by amrmak

  • Cc amrmak added

comment:6 Changed 8 months ago by kzar

  • Priority changed from Unknown to P2
  • Ready set

comment:7 Changed 8 months ago by abpbot

A commit referencing this issue has landed:
Issue 6437 - Skip styles-only patterns on DOM mutations

comment:8 Changed 8 months ago by mjethani

  • Review URL(s) modified (diff)

comment:9 Changed 8 months ago by mjethani

  • Blocking 6504 added

comment:10 Changed 8 months ago by mjethani

  • Review URL(s) modified (diff)

comment:11 Changed 8 months ago by mjethani

  • Review URL(s) modified (diff)

comment:12 Changed 7 months ago by abpbot

A commit referencing this issue has landed:
Issue 6437 - Do not override scheduled full processing

comment:13 Changed 7 months ago by abpbot

A commit referencing this issue has landed:
Issue 6437 - Apply :-abp-contains() to matching roots only

comment:14 Changed 7 months ago by mjethani

  • Blocked By 6618 added

comment:15 Changed 7 months ago by abpbot

A commit referencing this issue has landed:
Issue 6437 - Filter out patterns that do not match DOM mutations

comment:16 Changed 6 months ago by arthur

  • Cc arthur added

comment:17 Changed 5 months ago by abpbot

A commit referencing this issue has landed:
Issue 6437 - Skip elements not affected by DOM mutations

comment:18 Changed 4 months ago 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 4 months ago by mjethani

  • Description modified (diff)

comment:20 Changed 4 months ago by mjethani

  • Blocked By 6680 added

comment:21 Changed 7 weeks ago by mjethani

  • Blocking 7000 added

comment:22 Changed 7 weeks ago by mjethani

  • Blocking 7000 removed

comment:23 Changed 7 weeks ago by mjethani

  • Blocking 7000 added
Note: See TracTickets for help on using tickets.