Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should dNR dynamic rules reset on update? #251

Closed
xeenon opened this issue Aug 4, 2022 · 5 comments
Closed

Should dNR dynamic rules reset on update? #251

xeenon opened this issue Aug 4, 2022 · 5 comments
Labels
inconsistency Inconsistent behavior across browsers opposed: chrome Opposed by Chrome opposed: firefox Opposed by Firefox opposed: safari Opposed by Safari topic: dnr Related to declarativeNetRequest

Comments

@xeenon
Copy link
Collaborator

xeenon commented Aug 4, 2022

Chrome currently resets any dynamic dNR rules on extension update in the effort to force extensions to keep their static rules and dynamic rules in sync. Without this reset, any previously added dynamic rules might conflict with newly added static rules in the update.

Safari does not reset dynamic dNR rules on update and the extension could clear them and reapply any if needed.

I feel that a reset like in Chrome could potentially and surprisingly delete user generated content blocking rules that would require additional storage and management steps by the extension to preserve the rules and reapply them.

@xeenon xeenon added the inconsistency Inconsistent behavior across browsers label Aug 4, 2022
@hfiguiere
Copy link

hfiguiere commented Aug 4, 2022

The Google documentation says:

Rules added using the updateDynamicRules API method are persisted across both sessions and extension updates.

So to me it looks like Chrome implementation doesn't follow it. And this is inconsistent.

Note on clearing the rules: this requires calling getDynamicRules() and then updateDynamicRules(). Shouldn't there be a (faster) "clear" function instead since this could slow down startup... (unless I really missed something)

@hanguokai
Copy link
Member

Except DynamicRules, there are other similar apis, like registerContentScripts, alarms, contextMenus... They create somethings dynamically when extension install/update or anytime when user want to change them. So what happen when browser start/quit, profile start/close, extension update and extension enable/disable? Does browser keep them automatically or clear them?

Because they are not clear, from a developer's point of view, I would clear and re-create them like below code, thus I don't have to care about how the browser handle them. There is a discussion for dynamic content script.

function onInstall(e) {
  // clear all dynamic content scripts, then re-register them.
  chrome.scripting.unregisterContentScripts(function() {
    chrome.scripting.registerContentScripts(one or more content scripts);
  });

  // clear all context menus, then re-create them.
  chrome.contextMenus.removeAll(function() {
    chrome.contextMenus.create(menu-1);
    chrome.contextMenus.create(menu-2);
    ……
  });

  // clear all alarms, then re-create them.
  chrome.alarms.clearAll(function() {
    chrome.alarms.create(name-1, config-1);
    chrome.alarms.create(name-2, config-2);
    ....
  });
  // if your alarm names are static, you can create them directly, no need to clear them first.
}

chrome.runtime.onInstalled.addListener(onInstall);
chrome.runtime.onStartup.addListener(onInstall);

Developers can save these dynamic created items in storage, so they can restore them when browser restart or extension update.

@bershanskiy
Copy link
Member

Because they are not clear, from a developer's point of view, I would clear and re-create them like below code, thus I don't have to care about how the browser handle them.

This is what I always do. Also, it is possible that during extension's lifespan, content script will be moved (path change), alarms renamed, contextMenu command names or button text updated, etc.. So registrations for older extension versions might not be relevant anymore and it is always conceptually easier/safer to just re-register them just in case.

@Rob--W
Copy link
Member

Rob--W commented Aug 26, 2022

Chrome currently resets any dynamic dNR rules on extension update in the effort to force extensions to keep their static rules and dynamic rules in sync.

@dotproto While you did mention that dynamic rules are cleared, I tested the implementation and it looks like that is not the case. On the contrary, there is a bug in Chrome that prevents dynamic rules from being removed on uninstall (https://crbug.com/1302900).

When I trace the implementation, it looks like dynamic rules are intentionally kept, and only (intended/supposed to be) removed on uninstall. For example, here's internal documentation on the intended behavior of dynamic rule updates: https://source.chromium.org/chromium/chromium/src/+/main:extensions/browser/extension_prefs.cc;l=223-230;drc=7aa346a8ba6f861859a392d241ee8246303e94c6

@xeenon xeenon added topic: dnr Related to declarativeNetRequest opposed: safari Opposed by Safari follow-up: chrome Needs a response from a Chrome representative labels Aug 31, 2022
@Rob--W Rob--W added the opposed: firefox Opposed by Firefox label Sep 22, 2022
@Rob--W Rob--W added opposed: chrome Opposed by Chrome and removed follow-up: chrome Needs a response from a Chrome representative labels Sep 13, 2023
@Rob--W
Copy link
Member

Rob--W commented Sep 13, 2023

Changing follow-up: chrome Needs a response from a Chrome representative to opposed: chrome Opposed by Chrome after discussing this with @rdcronin and @oliverdunk at TPAC 2023.

"opposed" in this group means "There is consensus that dynamic rules should not reset on update". In other words, that dynamic rules are persisted across extension updates.

Chrome will keep the current behavior (i.e. keeping dynamic rules on update). @oliverdunk will take care of ensuring that Chrome's documentation clarifies that dynamic rules are persistent.

I'll comment on the crbug to let the crbug owner know about this agreement, so that even if the crbug (https://crbug.com/1302900 - dynamic rules kept after uninstall) is fixed, that dynamic rules are still preserved across updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inconsistency Inconsistent behavior across browsers opposed: chrome Opposed by Chrome opposed: firefox Opposed by Firefox opposed: safari Opposed by Safari topic: dnr Related to declarativeNetRequest
Projects
None yet
Development

No branches or pull requests

5 participants