Opened 6 months ago

Closed 6 months ago

Last modified 6 months ago

#4576 closed defect (fixed)

please_kill_startup_performance=true kills the subscriptions + custom filters

Reported by: mapx Assignee: trev
Priority: P1 Milestone: Adblock-Plus-2.8.1-for-Firefox
Module: Adblock-Plus-for-Firefox Keywords:
Cc: trev, greiner Blocked By:
Blocking: #3993 Platform: Firefox
Ready: yes Confidential: no
Tester: Scheer Verified working: yes
Review URL(s):

https://codereview.adblockplus.org/29360036/
https://codereview.adblockplus.org/29360039/
https://codereview.adblockplus.org/29360048/

Description (last modified by trev)

Environment

ABP 2.8
various filter lists (only 2 enabled), various custom filters groups

How to reproduce

There are two known ways to trigger that issue. First one involves setting please_kill_startup_performance preference:

  • in about:config it's already please_kill_startup_performance=false
  • set it to true
  • close Firefox
  • open Firefox

Observed behaviour:

All the lists are gone, only easylist is still there (or added automatically by ABP because ...no lists)
Groups of custom filters ==> gone

If I switch back to please_kill_startup_performance=false, close / open Firefox:

  • all the lists / custom filters are back !

Second variant requires importing a large filter list:

Expected results:

The file should be imported into custom filters successfully.

Actual results:

The message "The file's data could not be processed, maybe this isn't an Adblock Plus backup file?" appears. Browser Console shows the error message "too much recursion."

Background

Reading filters errors out with "too much recursion." This recursion isn't ours but rather Task.jsm. It seems that Task.jsm doesn't handle it well, if a generator keeps yielding constants (meaning: non-promises) in a loop. It will recurse then and will explode eventually. Without please_kill_startup_performance we will yield a promise every now and then which makes the task return.

Change History (16)

comment:1 Changed 6 months ago by mapx

  • Cc greiner added

comment:2 Changed 6 months ago by trev

  • Owner set to trev
  • Priority changed from Unknown to P1
  • Ready set

comment:3 Changed 6 months ago by trev

  • Description modified (diff)

comment:4 Changed 6 months ago by trev

  • Blocking 3993 added

Caused by #3993 btw - landed in development builds six months ago and nobody seemed to have noticed.

comment:5 Changed 6 months ago by trev

  • Review URL(s) modified (diff)

comment:7 Changed 6 months ago by trev

  • Milestone set to Adblock-Plus-2.8.1-for-Firefox
  • Resolution set to fixed
  • Status changed from new to closed

comment:8 Changed 6 months ago by trev

  • Review URL(s) modified (diff)

Added review for integration test.

comment:9 Changed 6 months ago by trev

  • Description modified (diff)

Added additional steps to reproduce, it seems that there is a scenario that doesn't require please_kill_startup_performance switch.

comment:10 Changed 6 months ago by scheer

  • Tester changed from Unknown to Scheer
  • Verified working set
  • Custom Filters and Filter Subscriptions are kept when please_kill_startup_performance is set to true and Firefox is restarted. Upon setting please_kill_startup_performance back to false and also restarting, Custom Filters and Filter Subscriptions are still kept.
  • Backing up and restoring from 'Restore own backup' correctly restores your Custom Filters when please_kill_startup_performance is set to true and the browser restarted.

Firefox 38,39,48,49.
ABP for Firefox 2.8.0.4219-beta
Windows 10 64 Bit
Windows 7 64 Bit
English/French

comment:11 Changed 6 months ago by trev

  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening, I introduced a bug here. Not sure why but the integration tests don't catch it reliably - only sometimes.

comment:12 Changed 6 months ago by trev

  • Review URL(s) modified (diff)

comment:13 Changed 6 months ago by abpbot

A commit referencing this issue has landed:
Issue 4576 - Fixed a mistake introduced in previous commit

comment:14 Changed 6 months ago by trev

  • Resolution set to fixed
  • Status changed from reopened to closed
  • Verified working unset

Fixed again.

comment:16 Changed 6 months ago by scheer

  • Verified working set
  • Custom Filters and Filter Subscriptions are kept when please_kill_startup_performance is set to true and Firefox is restarted. Upon setting please_kill_startup_performance back to false and also restarting, Custom Filters and Filter Subscriptions are still kept.
  • Backing up and restoring from 'Restore own backup' correctly restores your Custom Filters when please_kill_startup_performance is set to true and the browser restarted.

Firefox 38,39,48,49.
ABP for Firefox 2.8.0.4220-beta
Windows 10 64 Bit

Note: See TracTickets for help on using tickets.