Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(822)

Issue 2385103002: Deprecate StaleWhileRevalidateMetricsObserver (Closed)

Created:
4 years, 2 months ago by Takashi Toyoshima
Modified:
4 years, 2 months ago
CC:
chromium-reviews, csharrison+watch_chromium.org, loading-reviews+metrics_chromium.org, kenjibaheux
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Deprecate StaleWhileRevalidateMetricsObserver Since we are not running this experiments at this point, and metrics are currently marked as obsolete, let me remove them for now. BUG=348877 Committed: https://crrev.com/898bb9b5b621d892470f68c99b08425942c53960 Cr-Commit-Position: refs/heads/master@{#423095}

Patch Set 1 #

Patch Set 2 : build rule #

Unified diffs Side-by-side diffs Delta from patch set Stats (+0 lines, -89 lines) Patch
M chrome/browser/BUILD.gn View 1 chunk +0 lines, -2 lines 0 comments Download
D chrome/browser/page_load_metrics/observers/stale_while_revalidate_metrics_observer.h View 1 chunk +0 lines, -29 lines 0 comments Download
D chrome/browser/page_load_metrics/observers/stale_while_revalidate_metrics_observer.cc View 1 chunk +0 lines, -53 lines 0 comments Download
M chrome/browser/page_load_metrics/page_load_metrics_initialize.cc View 2 chunks +0 lines, -4 lines 0 comments Download
M chrome/browser/ui/BUILD.gn View 1 1 chunk +0 lines, -1 line 0 comments Download

Messages

Total messages: 21 (8 generated)
Takashi Toyoshima
Adam, shall we just remove these metrics? SWR's metrics are defined with suffix rules and ...
4 years, 2 months ago (2016-10-03 09:56:06 UTC) #3
Adam Rice
Kenji, this CL removes the metrics PageLoad.Clients.StaleWhileRevalidateExperiment.Timing2.NavigationToLoadEventFired PageLoad.Clients.StaleWhileRevalidateExperiment.Timing2.NavigationToFirstLayout PageLoad.Clients.StaleWhileRevalidateExperiment.Timing2.NavigationToFirstTextPaint I don't think we need these ...
4 years, 2 months ago (2016-10-03 10:03:22 UTC) #5
Adam Rice
toyoshim: If Kenji gives the okay to remove these, we should also remove net/base/stale_while_revalidate_experiment_domains* We ...
4 years, 2 months ago (2016-10-03 10:07:35 UTC) #6
kenjibaheux
Yes, we can remove the metrics and code related to the list of experiment domains. ...
4 years, 2 months ago (2016-10-04 07:16:38 UTC) #7
Adam Rice
lgtm Optional: remove net/base/stale_while_revalidate_experiment_domains* I am happy to do that in a follow-up CL if ...
4 years, 2 months ago (2016-10-04 08:08:37 UTC) #8
Takashi Toyoshima
+csharrison@chromium.org for owner's review in page_load_metrics. Adam, yeah you would be better to make followup ...
4 years, 2 months ago (2016-10-04 08:50:15 UTC) #10
Adam Rice
On 2016/10/04 08:50:15, toyoshim wrote: > Adam, yeah you would be better to make followup ...
4 years, 2 months ago (2016-10-04 09:29:35 UTC) #11
Charlie Harrison
page_load_metrics LGTM thanks for the cleanup.
4 years, 2 months ago (2016-10-04 12:08:51 UTC) #12
Takashi Toyoshima
Adam, chrome/browser/ui/BUILD.gn has per-file OWNERS rule that allows everyone review it. So probably, it would ...
4 years, 2 months ago (2016-10-05 06:13:39 UTC) #13
Takashi Toyoshima
Ah, but yes, dropping it in my CL seems to be more reasonable because this ...
4 years, 2 months ago (2016-10-05 06:16:35 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2385103002/20001
4 years, 2 months ago (2016-10-05 06:18:27 UTC) #17
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 2 months ago (2016-10-05 07:16:24 UTC) #19
commit-bot: I haz the power
4 years, 2 months ago (2016-10-05 07:17:58 UTC) #21
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/898bb9b5b621d892470f68c99b08425942c53960
Cr-Commit-Position: refs/heads/master@{#423095}

Powered by Google App Engine
This is Rietveld 408576698