New issue
Advanced search Search tips

Issue 777717 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Declarative Web Request API causes renderer's cache to flush on each extension load/unload/uninstall.

Project Member Reported by karandeepb@chromium.org, Oct 24 2017

Issue description

This happens on all channels. 

E.g. code path for extension load:

- RulesRegistryService::OnExtensionLoaded
- RulesRegistryService::NotifyRegistriesHelper
- RulesRegistry::OnExtensionLoaded
- WebRequestRulesRegistry::AddRulesImpl
- WebRequestRulesRegistry::ClearCacheOnNavigation(

This is quite wasteful as this means that the cache of all renderers will be cleared for no reason on each extension load/unload/uninstall. 

Note: DWR is only enabled on Stable for webviews.

 
Cc: battre@chromium.org
Components: Platform>Extensions>API
We flush on unload to ensure that resources modified by an extension are now re-requested from the network. We flush on load so resources that the extension might want to intercept are actually routed through the extension and not served from the memory cache.

I'd say this is "WAI"
The way I read the code, it looks like we clear on extension load/unload even if the extension doesn't have access to the declarativeWebRequest API, and thus couldn't have possibly (modulo bugs) modified resources.  It seems like we should be able to change this to:

if (!ExtensionCanUseDeclarativeWebRequest())
  return;
ClearCache();  // Clear all resources so that any modified by the extension are re-fetched from the network.
Yes, I agree. Karan, will you take care of this?
Yeah as Devlin pointed out, we do this for every extension load/unload even if it's not necessary. Will take care of this.
Project Member

Comment 5 by bugdroid1@chromium.org, May 29 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4bcb1ed99da68fd3afb0c3b8776cc0c713603c8c

commit 4bcb1ed99da68fd3afb0c3b8776cc0c713603c8c
Author: karandeepb <karandeepb@chromium.org>
Date: Tue May 29 22:12:14 2018

Declarative Web Request: Restructure code related to rules registry registration.

This CL restructures code related to the creation and registration of rules
registries for the declarativeWebRequest API. The
EnsureDefaultRulesRegistriesRegistered method which is also confusingly used for
non-default (webview) registries is now only used for the default registries.
This makes the code easier to understand. This CL should have no behavior
changes.

BUG= 693243 ,  777717 

Change-Id: I627e95916be09b1b609cb9c4f771e3c0959e5bc6
Reviewed-on: https://chromium-review.googlesource.com/1069911
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562622}
[modify] https://crrev.com/4bcb1ed99da68fd3afb0c3b8776cc0c713603c8c/extensions/browser/api/declarative/rules_registry_service.cc
[modify] https://crrev.com/4bcb1ed99da68fd3afb0c3b8776cc0c713603c8c/extensions/browser/api/declarative/rules_registry_service.h

Project Member

Comment 6 by bugdroid1@chromium.org, May 30 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4f46cc0ed1e1aa68e0453e1ccefdb9ee600177be

commit 4f46cc0ed1e1aa68e0453e1ccefdb9ee600177be
Author: Karan Bhatia <karandeepb@chromium.org>
Date: Wed May 30 23:09:17 2018

Declarative Web Request: Only register default web request rules registries if needed.

Currently we create default web request rules registries for the declarative web
request API even if the API is not available to the current environment. This is
wasteful and detrimental to performance, since the initial registry load blocks
the first network request (tracked via Extensions.NetworkDelayRegistryLoad UMA).

This CL changes RulesRegistryService so that the default WebRequestRulesRegistry
is registered only if the declarative web request API is avaialble to the
current environment. This, for example, means that no default web request rule
registries would be created on the stable channel. Rules registries
corresponding to webviews and the default content rules registry would still be
created.

This also helps fix  issue 777717  and renderer cache is not cleared redundantly
on each extension load/unload/uninstall when the API is not available.

BUG= 693243 ,  777717 

Change-Id: I7384fed71a86aea3f5cc8d2eafd1445b439dd76a
Reviewed-on: https://chromium-review.googlesource.com/1072497
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563051}
[modify] https://crrev.com/4f46cc0ed1e1aa68e0453e1ccefdb9ee600177be/chrome/browser/apps/guest_view/web_view_browsertest.cc
[modify] https://crrev.com/4f46cc0ed1e1aa68e0453e1ccefdb9ee600177be/chrome/browser/extensions/api/declarative/rules_registry_service_unittest.cc
[modify] https://crrev.com/4f46cc0ed1e1aa68e0453e1ccefdb9ee600177be/extensions/browser/api/declarative/rules_registry_service.cc

Status: Fixed (was: Assigned)

Sign in to add a comment