|
|
Created:
3 years, 7 months ago by Yusuke Sato Modified:
3 years, 7 months ago CC:
chromium-reviews, elijahtaylor+arcwatch_chromium.org, yusukes+watch_chromium.org, viettrungluu+watch_chromium.org, hidehiko+watch_chromium.org, yzshen+watch_chromium.org, abarth-chromium, asvitkine+watch_chromium.org, Aaron Boodman, lhchavez+watch_chromium.org, victorhsieh+watch_chromium.org, darin (slow to review), qsr+mojo_chromium.org, cywang Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionARC: Add BootType parameter to ReportBootProgress() in metrics.mojom
ARC has 3 different boot mode. Previously, ARC boot time was recorded
with the same UMA metric like Arc.AndroidBootTime regardless of the
boot mode. This CL allows Chrome to use a dedicated UMA metric for
each boot mode. This is better because a typical boot time is
different for each boot mode.
BUG=726346
TEST=tested with opt-in, ota, and regular ARC boot
Review-Url: https://codereview.chromium.org/2906503003
Cr-Commit-Position: refs/heads/master@{#475184}
Committed: https://chromium.googlesource.com/chromium/src/+/d6ea165b0d47b5442e7b87cbb0b0fd1006f2a115
Patch Set 1 #
Total comments: 10
Patch Set 2 : Address comments from Luis #
Total comments: 2
Patch Set 3 : Address comment #
Total comments: 14
Patch Set 4 : Address comments from Ilya #
Total comments: 5
Patch Set 5 : Address more comments #
Messages
Total messages: 48 (25 generated)
The CQ bit was checked by yusukes@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
yusukes@chromium.org changed reviewers: + cywang@chromium.org, lhchavez@chromium.org
PTAL
https://codereview.chromium.org/2906503003/diff/1/components/arc/metrics/arc_... File components/arc/metrics/arc_metrics_service.cc (right): https://codereview.chromium.org/2906503003/diff/1/components/arc/metrics/arc_... components/arc/metrics/arc_metrics_service.cc:19: UMA_HISTOGRAM_CUSTOM_TIMES("Arc.AndroidBootTime" suffix, (elapsed_time), \ You seem to use "UNKNOWN" for backward compatibility, isn't it more clear to have "Arc.AndroidBootTime2" instead to prevent from mixing up the counts of old and new boot times in the future? https://codereview.chromium.org/2906503003/diff/1/components/arc/metrics/arc_... components/arc/metrics/arc_metrics_service.cc:190: switch (boot_type) { std::string name = "Arc.AndroidBootTime2" + BootTypeToString(boot_type); UMA_HISTOGRAM_CUSTOM_TIMES(name, elapsed_time, base::TimeDelta::FromMilliseconds(1), base::TimeDelta::FromSeconds(max_seconds), 50);
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2906503003/diff/1/components/arc/metrics/arc_... File components/arc/metrics/arc_metrics_service.cc (right): https://codereview.chromium.org/2906503003/diff/1/components/arc/metrics/arc_... components/arc/metrics/arc_metrics_service.cc:190: switch (boot_type) { On 2017/05/25 04:26:06, cywang wrote: > std::string name = "Arc.AndroidBootTime2" + BootTypeToString(boot_type); > UMA_HISTOGRAM_CUSTOM_TIMES(name, elapsed_time, > base::TimeDelta::FromMilliseconds(1), > base::TimeDelta::FromSeconds(max_seconds), 50); IIUC, the macro UMA_HISTOGRAM_CUSTOM_TIMES requires a string literal. I checked the existing code with git grep, and it seemed like all UMA_HISTOGRAM_CUSTOM_TIMES invocations were with string literals.
https://codereview.chromium.org/2906503003/diff/1/components/arc/metrics/arc_... File components/arc/metrics/arc_metrics_service.cc (right): https://codereview.chromium.org/2906503003/diff/1/components/arc/metrics/arc_... components/arc/metrics/arc_metrics_service.cc:19: UMA_HISTOGRAM_CUSTOM_TIMES("Arc.AndroidBootTime" suffix, (elapsed_time), \ On 2017/05/25 04:26:06, cywang wrote: > You seem to use "UNKNOWN" for backward compatibility, isn't it more clear to > have "Arc.AndroidBootTime2" instead to prevent from mixing up the counts of old > and new boot times in the future? So the current idea is to keep "Arc.AndroidBootTime" for now for backward compatibility purposes, and also introduce "Arc.AndroidBootTime.<SUFFIX>". It does the same for Arc.boot_progesss* too. For example, this keeps "Arc.boot_progesss_enable_screen" and introduces "Arc.boot_progesss_enable_screen.<SUFFIX>". Adding "2" is fine for the former, but I'm not really sure where to insert "2" for the latter. Inserting "2" might make this code (and the xml) more complicated actually... As we chatted offline, let me ask the xml owner about this first.
Description was changed from ========== Add ReportBootProgressWithType() to ARC's metrics.mojom ARC has 3 different boot mode. Previously, ARC boot time was recorded with the same UMA metric like Arc.AndroidBootTime regardless of the boot mode. This CL allows Chrome to use a dedicated UMA metric for each boot mode. This is better because a typical boot time is different for each boot mode. BUG=b/62054963 TEST=tested with opt-in, ota, and regular ARC boot ========== to ========== Add ReportBootProgressWithType() to ARC's metrics.mojom ARC has 3 different boot mode. Previously, ARC boot time was recorded with the same UMA metric like Arc.AndroidBootTime regardless of the boot mode. This CL allows Chrome to use a dedicated UMA metric for each boot mode. This is better because a typical boot time is different for each boot mode. BUG=726346 TEST=tested with opt-in, ota, and regular ARC boot ==========
Luis: PTAL
https://codereview.chromium.org/2906503003/diff/1/components/arc/common/metri... File components/arc/common/metrics.mojom (right): https://codereview.chromium.org/2906503003/diff/1/components/arc/common/metri... components/arc/common/metrics.mojom:14: // ARC++ image. nit: s/ARC++/ARC/ https://codereview.chromium.org/2906503003/diff/1/components/arc/common/metri... components/arc/common/metrics.mojom:43: ReportBootProgress@0(array<BootProgressEvent> events); You don't need to deprecate this :) ReportBootProgress@0(array<BootProgressEvent> events, [MinVersion=1] BootType boot_type); Don't know if BootType should be optional (marked with a ?) or not, though. https://codereview.chromium.org/2906503003/diff/1/components/arc/metrics/arc_... File components/arc/metrics/arc_metrics_service.cc (right): https://codereview.chromium.org/2906503003/diff/1/components/arc/metrics/arc_... components/arc/metrics/arc_metrics_service.cc:28: const int kRequestProcessListPeriodInMinutes = 5; not your fault, but can these be constexpr? Can this in particular be a base::TimeDelta?
The CQ bit was checked by yusukes@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
PTAL https://codereview.chromium.org/2906503003/diff/1/components/arc/common/metri... File components/arc/common/metrics.mojom (right): https://codereview.chromium.org/2906503003/diff/1/components/arc/common/metri... components/arc/common/metrics.mojom:14: // ARC++ image. On 2017/05/25 16:00:35, Luis Héctor Chávez wrote: > nit: s/ARC++/ARC/ Done. https://codereview.chromium.org/2906503003/diff/1/components/arc/common/metri... components/arc/common/metrics.mojom:43: ReportBootProgress@0(array<BootProgressEvent> events); On 2017/05/25 16:00:35, Luis Héctor Chávez wrote: > You don't need to deprecate this :) > > ReportBootProgress@0(array<BootProgressEvent> events, > [MinVersion=1] BootType boot_type); > > Don't know if BootType should be optional (marked with a ?) or not, though. Done. Used BootType without '?' because '?' is not allowed for enums. Tested with new Chrome + new Android, old Chrome + new Android, and new Chrome + old Android. For the third one, confirmed that UNKNOWN (0) is passed to Chrome. https://codereview.chromium.org/2906503003/diff/1/components/arc/metrics/arc_... File components/arc/metrics/arc_metrics_service.cc (right): https://codereview.chromium.org/2906503003/diff/1/components/arc/metrics/arc_... components/arc/metrics/arc_metrics_service.cc:28: const int kRequestProcessListPeriodInMinutes = 5; On 2017/05/25 16:00:35, Luis Héctor Chávez wrote: > not your fault, but can these be constexpr? Can this in particular be a > base::TimeDelta? Done.
lgtm https://codereview.chromium.org/2906503003/diff/20001/components/arc/common/m... File components/arc/common/metrics.mojom (right): https://codereview.chromium.org/2906503003/diff/20001/components/arc/common/m... components/arc/common/metrics.mojom:10: UNKNOWN = 0, nit: Add a comment that this is only used for compatibility reasons with Version 0 instances.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Patchset #3 (id:40001) has been deleted
https://codereview.chromium.org/2906503003/diff/20001/components/arc/common/m... File components/arc/common/metrics.mojom (right): https://codereview.chromium.org/2906503003/diff/20001/components/arc/common/m... components/arc/common/metrics.mojom:10: UNKNOWN = 0, On 2017/05/25 18:24:30, Luis Héctor Chávez wrote: > nit: Add a comment that this is only used for compatibility reasons with Version > 0 instances. Done.
yusukes@chromium.org changed reviewers: + dcheng@chromium.org
+dcheng@ Could you do an OWNER review for components/arc/common/metrics.mojom?
yusukes@chromium.org changed reviewers: + isherman@chromium.org
+isherman@ too. Could you have a look at the xml change? This is for ARC (the Android runtime for Chrome OS.) We already have Arc.AndroidBootTime UMA metric, and I'm adding Arc.AndroidBootTime.<suffixes>. I'm doing the same for Arc.boot_progress* too. For example, we already use Arc.boot_progress_enable_screen and this CL adds Arc.boot_progress_enable.<suffixes>. I think it works, but please let me know if I'm wrong.
The CQ bit was checked by yusukes@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2017/05/25 20:01:19, Yusuke Sato wrote: > +isherman@ too. > > Could you have a look at the xml change? This is for ARC (the Android runtime > for Chrome OS.) > > We already have Arc.AndroidBootTime UMA metric, and I'm adding > Arc.AndroidBootTime.<suffixes>. I'm doing the same for Arc.boot_progress* too. > For example, we already use Arc.boot_progress_enable_screen and this CL adds > Arc.boot_progress_enable.<suffixes>. I think it works, but please let me know if > I'm wrong. Where are you implementing the changes for the Arc.boot_progress* histograms? https://codereview.chromium.org/2906503003/diff/60001/components/arc/metrics/... File components/arc/metrics/arc_metrics_service.cc (right): https://codereview.chromium.org/2906503003/diff/60001/components/arc/metrics/... components/arc/metrics/arc_metrics_service.cc:17: #define RECORD_ARC_UMA(suffix, elapsed_time, max_seconds) \ Why are you defining this as a macro rather than a function? https://codereview.chromium.org/2906503003/diff/60001/components/arc/metrics/... components/arc/metrics/arc_metrics_service.cc:180: base::HistogramBase::kUmaTargetedHistogramFlag); nit: Could you use base::UmaHistogramCustomTimes() here? https://codereview.chromium.org/2906503003/diff/60001/components/arc/metrics/... components/arc/metrics/arc_metrics_service.cc:191: RECORD_ARC_UMA(".FIRST_BOOT", elapsed_time, max_seconds); nit: Why ALL_CAPS? Typically, UMA histograms use Dotted.CamelCase https://codereview.chromium.org/2906503003/diff/60001/tools/metrics/histogram... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2906503003/diff/60001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:84498: + <suffix name="FIRST_BOOT" label="For the very first boot aka opt-in."/> nit: Please add a comma between "boot" and "aka" https://codereview.chromium.org/2906503003/diff/60001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:84501: + <suffix name="REGULAR_BOOT" label="For regular boot."/> nit: s/For/For a
Still addressing your comments, but let me answer the questions first: https://codereview.chromium.org/2906503003/diff/60001/components/arc/metrics/... File components/arc/metrics/arc_metrics_service.cc (right): https://codereview.chromium.org/2906503003/diff/60001/components/arc/metrics/... components/arc/metrics/arc_metrics_service.cc:17: #define RECORD_ARC_UMA(suffix, elapsed_time, max_seconds) \ On 2017/05/25 22:32:08, Ilya Sherman wrote: > Why are you defining this as a macro rather than a function? Since HISTOGRAM_POINTER_USE in base/metrics/histogram_macros_internal.h, which UMA_HISTOGRAM_CUSTOM_TIMES is expanded to, takes a parameter called |constant_histogram_name|, I assumed that the first parameter had to be a string literal, but apparently I was wrong. I'm switching to a function. Thanks. https://codereview.chromium.org/2906503003/diff/60001/components/arc/metrics/... components/arc/metrics/arc_metrics_service.cc:173: std::string title = "Arc." + event->event + BootTypeToString(boot_type); This is the change for Arc.boot_progress*.
The CQ bit was checked by yusukes@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Please take another look. https://codereview.chromium.org/2906503003/diff/60001/components/arc/metrics/... File components/arc/metrics/arc_metrics_service.cc (right): https://codereview.chromium.org/2906503003/diff/60001/components/arc/metrics/... components/arc/metrics/arc_metrics_service.cc:17: #define RECORD_ARC_UMA(suffix, elapsed_time, max_seconds) \ On 2017/05/25 22:48:00, Yusuke Sato wrote: > On 2017/05/25 22:32:08, Ilya Sherman wrote: > > Why are you defining this as a macro rather than a function? > > Since HISTOGRAM_POINTER_USE in base/metrics/histogram_macros_internal.h, which > UMA_HISTOGRAM_CUSTOM_TIMES is expanded to, takes a parameter called > |constant_histogram_name|, I assumed that the first parameter had to be a string > literal, but apparently I was wrong. I'm switching to a function. Thanks. Done. Replaced it with base::UmaHistogramCustomTimes() too. https://codereview.chromium.org/2906503003/diff/60001/components/arc/metrics/... components/arc/metrics/arc_metrics_service.cc:180: base::HistogramBase::kUmaTargetedHistogramFlag); On 2017/05/25 22:32:08, Ilya Sherman wrote: > nit: Could you use base::UmaHistogramCustomTimes() here? Done. https://codereview.chromium.org/2906503003/diff/60001/components/arc/metrics/... components/arc/metrics/arc_metrics_service.cc:191: RECORD_ARC_UMA(".FIRST_BOOT", elapsed_time, max_seconds); On 2017/05/25 22:32:08, Ilya Sherman wrote: > nit: Why ALL_CAPS? Typically, UMA histograms use Dotted.CamelCase Changed it to CamelCase, thanks. https://codereview.chromium.org/2906503003/diff/60001/tools/metrics/histogram... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2906503003/diff/60001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:84498: + <suffix name="FIRST_BOOT" label="For the very first boot aka opt-in."/> On 2017/05/25 22:32:08, Ilya Sherman wrote: > nit: Please add a comma between "boot" and "aka" Done. https://codereview.chromium.org/2906503003/diff/60001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:84501: + <suffix name="REGULAR_BOOT" label="For regular boot."/> On 2017/05/25 22:32:08, Ilya Sherman wrote: > nit: s/For/For a Done.
Metrics LGTM, thanks. https://codereview.chromium.org/2906503003/diff/60001/components/arc/metrics/... File components/arc/metrics/arc_metrics_service.cc (right): https://codereview.chromium.org/2906503003/diff/60001/components/arc/metrics/... components/arc/metrics/arc_metrics_service.cc:17: #define RECORD_ARC_UMA(suffix, elapsed_time, max_seconds) \ On 2017/05/25 22:48:00, Yusuke Sato wrote: > On 2017/05/25 22:32:08, Ilya Sherman wrote: > > Why are you defining this as a macro rather than a function? > > Since HISTOGRAM_POINTER_USE in base/metrics/histogram_macros_internal.h, which > UMA_HISTOGRAM_CUSTOM_TIMES is expanded to, takes a parameter called > |constant_histogram_name|, I assumed that the first parameter had to be a string > literal, but apparently I was wrong. I'm switching to a function. Thanks. Yeah, it's subtle. It needs to be a runtime-constant, but not necessarily a string literal. https://codereview.chromium.org/2906503003/diff/60001/components/arc/metrics/... components/arc/metrics/arc_metrics_service.cc:173: std::string title = "Arc." + event->event + BootTypeToString(boot_type); On 2017/05/25 22:48:00, Yusuke Sato wrote: > This is the change for Arc.boot_progress*. Ah, got it. Thanks!
Description was changed from ========== Add ReportBootProgressWithType() to ARC's metrics.mojom ARC has 3 different boot mode. Previously, ARC boot time was recorded with the same UMA metric like Arc.AndroidBootTime regardless of the boot mode. This CL allows Chrome to use a dedicated UMA metric for each boot mode. This is better because a typical boot time is different for each boot mode. BUG=726346 TEST=tested with opt-in, ota, and regular ARC boot ========== to ========== ARC: Add BootType parameter to ReportBootProgress() in metrics.mojom ARC has 3 different boot mode. Previously, ARC boot time was recorded with the same UMA metric like Arc.AndroidBootTime regardless of the boot mode. This CL allows Chrome to use a dedicated UMA metric for each boot mode. This is better because a typical boot time is different for each boot mode. BUG=726346 TEST=tested with opt-in, ota, and regular ARC boot ==========
On 2017/05/25 23:48:57, Ilya Sherman wrote: > Metrics LGTM, thanks. > > https://codereview.chromium.org/2906503003/diff/60001/components/arc/metrics/... > File components/arc/metrics/arc_metrics_service.cc (right): > > https://codereview.chromium.org/2906503003/diff/60001/components/arc/metrics/... > components/arc/metrics/arc_metrics_service.cc:17: #define RECORD_ARC_UMA(suffix, > elapsed_time, max_seconds) \ > On 2017/05/25 22:48:00, Yusuke Sato wrote: > > On 2017/05/25 22:32:08, Ilya Sherman wrote: > > > Why are you defining this as a macro rather than a function? > > > > Since HISTOGRAM_POINTER_USE in base/metrics/histogram_macros_internal.h, which > > UMA_HISTOGRAM_CUSTOM_TIMES is expanded to, takes a parameter called > > |constant_histogram_name|, I assumed that the first parameter had to be a > string > > literal, but apparently I was wrong. I'm switching to a function. Thanks. > > Yeah, it's subtle. It needs to be a runtime-constant, but not necessarily a > string literal. > > https://codereview.chromium.org/2906503003/diff/60001/components/arc/metrics/... > components/arc/metrics/arc_metrics_service.cc:173: std::string title = "Arc." + > event->event + BootTypeToString(boot_type); > On 2017/05/25 22:48:00, Yusuke Sato wrote: > > This is the change for Arc.boot_progress*. > > Ah, got it. Thanks! Thanks Ilya. Daniel, could you have a look?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
yusukes@chromium.org changed reviewers: - cywang@chromium.org
mojo lgtm https://codereview.chromium.org/2906503003/diff/80001/components/arc/metrics/... File components/arc/metrics/arc_metrics_service.cc (right): https://codereview.chromium.org/2906503003/diff/80001/components/arc/metrics/... components/arc/metrics/arc_metrics_service.cc:169: "Arc." + event->event + BootTypeToString(boot_type); It would be nice not to build the temporary BootTypeToString() object twice. https://codereview.chromium.org/2906503003/diff/80001/components/arc/metrics/... components/arc/metrics/arc_metrics_service.cc:175: base::UmaHistogramCustomTimes(name, elapsed_time, kUmaMinTime, max_time, FWIW, there is a macro to help with groups of UMAs like this, but I don't know if it's better or worse: see STATIC_HISTOGRAM_POINTER_GROUP
https://codereview.chromium.org/2906503003/diff/80001/components/arc/metrics/... File components/arc/metrics/arc_metrics_service.cc (right): https://codereview.chromium.org/2906503003/diff/80001/components/arc/metrics/... components/arc/metrics/arc_metrics_service.cc:169: "Arc." + event->event + BootTypeToString(boot_type); On 2017/05/26 20:53:45, dcheng wrote: > It would be nice not to build the temporary BootTypeToString() object twice. Done. https://codereview.chromium.org/2906503003/diff/80001/components/arc/metrics/... components/arc/metrics/arc_metrics_service.cc:175: base::UmaHistogramCustomTimes(name, elapsed_time, kUmaMinTime, max_time, On 2017/05/26 20:53:46, dcheng wrote: > FWIW, there is a macro to help with groups of UMAs like this, but I don't know > if it's better or worse: see STATIC_HISTOGRAM_POINTER_GROUP Thanks for the pointer, but for this CL, let me follow the recommendation from Ilya.
The CQ bit was checked by yusukes@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from lhchavez@chromium.org, isherman@chromium.org, dcheng@chromium.org Link to the patchset: https://codereview.chromium.org/2906503003/#ps100001 (title: "Address more comments")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2906503003/diff/80001/components/arc/metrics/... File components/arc/metrics/arc_metrics_service.cc (right): https://codereview.chromium.org/2906503003/diff/80001/components/arc/metrics/... components/arc/metrics/arc_metrics_service.cc:175: base::UmaHistogramCustomTimes(name, elapsed_time, kUmaMinTime, max_time, On 2017/05/26 20:53:46, dcheng wrote: > FWIW, there is a macro to help with groups of UMAs like this, but I don't know > if it's better or worse: see STATIC_HISTOGRAM_POINTER_GROUP I'd actually forgotten that macro exists =P My take on it is that it adds a lot of mental overhead for a small amount of performance gain (by caching the histogram pointers). I'd only recommend it over one of the options in //base/metrics/histogram_functions.h for unusually performance-sensitive code.
Oh, still LGTM, by the way. Hopefully I didn't just derail the commit queue...
CQ is committing da patch. Bot data: {"patchset_id": 100001, "attempt_start_ts": 1495834497545510, "parent_rev": "c33f7598aadfe913c6b4718e3b879ded0444140a", "commit_rev": "d6ea165b0d47b5442e7b87cbb0b0fd1006f2a115"}
Message was sent while issue was closed.
Description was changed from ========== ARC: Add BootType parameter to ReportBootProgress() in metrics.mojom ARC has 3 different boot mode. Previously, ARC boot time was recorded with the same UMA metric like Arc.AndroidBootTime regardless of the boot mode. This CL allows Chrome to use a dedicated UMA metric for each boot mode. This is better because a typical boot time is different for each boot mode. BUG=726346 TEST=tested with opt-in, ota, and regular ARC boot ========== to ========== ARC: Add BootType parameter to ReportBootProgress() in metrics.mojom ARC has 3 different boot mode. Previously, ARC boot time was recorded with the same UMA metric like Arc.AndroidBootTime regardless of the boot mode. This CL allows Chrome to use a dedicated UMA metric for each boot mode. This is better because a typical boot time is different for each boot mode. BUG=726346 TEST=tested with opt-in, ota, and regular ARC boot Review-Url: https://codereview.chromium.org/2906503003 Cr-Commit-Position: refs/heads/master@{#475184} Committed: https://chromium.googlesource.com/chromium/src/+/d6ea165b0d47b5442e7b87cbb0b0... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:100001) as https://chromium.googlesource.com/chromium/src/+/d6ea165b0d47b5442e7b87cbb0b0... |