New issue
Advanced search Search tips

Issue 693243 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 25
Cc:
Components:
EstimatedDays: ----
NextAction: 2018-09-22
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

UMA metric Extensions.NetworkDelayRegistryLoad shows delay on Stable channel.

Project Member Reported by karandeepb@chromium.org, Feb 16 2017

Issue description

The UMA metric Extensions.NetworkDelayRegistryLoad measures the time for which a network request is blocked due to the loading of the declarative web request rules registry. The declarative web request API is currently disabled on Stable, yet the UMA stats show significant delay. (Note: the metric will be logged only for specific web requests which were delayed by the loading of the rules registry). The mean delay is around 1.7 sec and the median delay is around 50 ms. 

Link - https://uma.googleplex.com/p/chrome/histograms/?endDate=20170215&dayCount=28&histograms=Extensions.NetworkDelayRegistryLoad&fixupData=true&showAverages=true&showMax=true&analysis=0.1%200.2%200.3%200.4%200.5%200.6%200.7%200.75%200.8%200.9%200.95%200.98%200.99%200.995&filters=channel%2Ceq%2C4%2Cisofficial%2Ceq%2CTrue&implicitFilters=isofficial.
 
Cc: rdevlin....@chromium.org battre@chromium.org
Think we can disable creation of the rules registry if DWR is not enabled on the current channel? It is created here - https://cs.chromium.org/chromium/src/extensions/browser/api/declarative/rules_registry_service.cc?q=rules_registry_service.cc&sq=package:chromium&dr&l=85

Also, I am not sure of the reason behind the delay being so significant.
Project Member

Comment 2 by bugdroid1@chromium.org, Feb 22 2017

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

commit a77b53baca5f2a4ff102592c17c8ea3f53dd5307
Author: karandeepb <karandeepb@chromium.org>
Date: Wed Feb 22 22:37:32 2017

Extensions: Only create Web request rules registry if Declarative Web Request is enabled.

Currently the Web Request rules registry required by the Declarative Web
Request API is created on all Chrome channels, even though the Declarative Web
Request API is disabled on Stable. The UMA metric
Extensions.NetworkDelayRegistryLoad shows significant delay for some web
requests which are blocked due to the loading of the Web Request rules
registry even on Stable. The mean delay is around 1.7 sec and the median delay
is around 50ms.

This CL modifies RulesRegistryService::EnsureDefaultRulesRegistriesRegistered
to ensure the Web Request rules registry is only created for the channels on
which the Declarative Web Request API is enabled. To do this,
Feature::IsAvailableToChannel is introduced.

BUG= 693243 

Review-Url: https://codereview.chromium.org/2705513002
Cr-Commit-Position: refs/heads/master@{#452240}

[modify] https://crrev.com/a77b53baca5f2a4ff102592c17c8ea3f53dd5307/extensions/browser/api/declarative/rules_registry_service.cc
[modify] https://crrev.com/a77b53baca5f2a4ff102592c17c8ea3f53dd5307/extensions/common/features/complex_feature.cc
[modify] https://crrev.com/a77b53baca5f2a4ff102592c17c8ea3f53dd5307/extensions/common/features/complex_feature.h
[modify] https://crrev.com/a77b53baca5f2a4ff102592c17c8ea3f53dd5307/extensions/common/features/feature.h
[modify] https://crrev.com/a77b53baca5f2a4ff102592c17c8ea3f53dd5307/extensions/common/features/simple_feature.cc
[modify] https://crrev.com/a77b53baca5f2a4ff102592c17c8ea3f53dd5307/extensions/common/features/simple_feature.h
[modify] https://crrev.com/a77b53baca5f2a4ff102592c17c8ea3f53dd5307/extensions/common/features/simple_feature_unittest.cc

Status: Fixed (was: Assigned)
NextAction: 2017-05-05
Action: Verify from metrics that there's no longer a delay on Stable.
Labels: Merge-Request-58
Status: Assigned (was: Fixed)
Merge Request: r452240 landed in M58 but was reverted via r459269 in M59. This is to request a merge of the revert commit (r459269) to M58.

Project Member

Comment 6 by sheriffbot@chromium.org, Mar 28 2017

Labels: -Merge-Request-58 Hotlist-Merge-Approved Merge-Approved-58
Your change meets the bar and is auto-approved for M58. Please go ahead and merge the CL to branch 3029 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 7 by bugdroid1@chromium.org, Mar 28 2017

Labels: -merge-approved-58 merge-merged-3029
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/96768d9da47097e7cbc5162a24bbe636ad6223f5

commit 96768d9da47097e7cbc5162a24bbe636ad6223f5
Author: Karan Bhatia <karandeepb@chromium.org>
Date: Tue Mar 28 22:42:11 2017

[Merge-m58] Revert of Extensions: Only create Web request rules registry if Declarative Web Request is enabled. (patchset #4 id:60001 of https://codereview.chromium.org/2705513002/ )

Reason for revert:
Declarative Web Request is enabled on Stable through <webview>. Hence we can't disable creation of rules registry on Stable.

Original issue's description:
> Extensions: Only create Web request rules registry if Declarative Web Request is enabled.
>
> Currently the Web Request rules registry required by the Declarative Web
> Request API is created on all Chrome channels, even though the Declarative Web
> Request API is disabled on Stable. The UMA metric
> Extensions.NetworkDelayRegistryLoad shows significant delay for some web
> requests which are blocked due to the loading of the Web Request rules
> registry even on Stable. The mean delay is around 1.7 sec and the median delay
> is around 50ms.
>
> This CL modifies RulesRegistryService::EnsureDefaultRulesRegistriesRegistered
> to ensure the Web Request rules registry is only created for the channels on
> which the Declarative Web Request API is enabled. To do this,
> Feature::IsAvailableToChannel is introduced.
>
> BUG= 693243 
>
> Review-Url: https://codereview.chromium.org/2705513002
> Cr-Commit-Position: refs/heads/master@{#452240}
> Committed: https://chromium.googlesource.com/chromium/src/+/a77b53baca5f2a4ff102592c17c8ea3f53dd5307

BUG= 693243 

Review-Url: https://codereview.chromium.org/2773593003
Cr-Commit-Position: refs/heads/master@{#459269}
(cherry picked from commit 9df2cccc337ca57188f9709b2205439347870606)

Review-Url: https://codereview.chromium.org/2783543003 .
Cr-Commit-Position: refs/branch-heads/3029@{#462}
Cr-Branched-From: 939b32ee5ba05c396eef3fd992822fcca9a2e262-refs/heads/master@{#454471}

[modify] https://crrev.com/96768d9da47097e7cbc5162a24bbe636ad6223f5/extensions/browser/api/declarative/rules_registry_service.cc
[modify] https://crrev.com/96768d9da47097e7cbc5162a24bbe636ad6223f5/extensions/common/features/complex_feature.cc
[modify] https://crrev.com/96768d9da47097e7cbc5162a24bbe636ad6223f5/extensions/common/features/complex_feature.h
[modify] https://crrev.com/96768d9da47097e7cbc5162a24bbe636ad6223f5/extensions/common/features/feature.h
[modify] https://crrev.com/96768d9da47097e7cbc5162a24bbe636ad6223f5/extensions/common/features/simple_feature.cc
[modify] https://crrev.com/96768d9da47097e7cbc5162a24bbe636ad6223f5/extensions/common/features/simple_feature.h
[modify] https://crrev.com/96768d9da47097e7cbc5162a24bbe636ad6223f5/extensions/common/features/simple_feature_unittest.cc

Status: WontFix (was: Assigned)
Landed the revert on M58. Closing the original issue as WontFix since Declarative Web Request is shipped on Stable through <webview>.
Status: Assigned (was: WontFix)
Think this can still be fixed for the non-webview case. Reopening.
Project Member

Comment 10 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 11 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

NextAction: 2018-06-12
Should be fixed. Setting a next action date to verify.
NextAction: 2018-08-22
The NextAction date has arrived: 2018-08-22
NextAction: 2018-09-22
The NextAction date has arrived: 2018-09-22
Status: Fixed (was: Assigned)
This should be fixed. The no. of samples for this histogram has decreased significantly from M68->M69. This is because we only register web-view rules registry now on Stable.

14 day aggregated data:

69
Aggregate number of samples: 2,723,548
Aggregate sum of samples: 4,399,827,012
Sample mean: 1,615

68
Aggregate number of samples: 4,151,928,821
Aggregate sum of samples: 30,856,350,413,336
Sample mean: 7,432

Sign in to add a comment