Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
382813002:
Added more policy UMA metrics. (Closed)
|
Created:
6 years, 5 months ago by Andrew T Wilson (Slow) Modified:
6 years, 4 months ago CC:
chromium-reviews, tim+watch_chromium.org, haitaol+watch_chromium.org, asvitkine+watch_chromium.org, zea+watch_chromium.org, maniscalco+watch_chromium.org, Thiemo Nagel Base URL:
svn://svn.chromium.org/chrome/trunk/src Project:
chromium Visibility:
Public. |
DescriptionAdded more policy UMA metrics.
Added missing UMA metrics, and updated histograms.xml to reflect existing
metrics.
BUG=160947
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=285903
Patch Set 1 #Patch Set 2 : Review feedback. #Patch Set 3 : Fixed code format issue. #
Total comments: 28
Patch Set 4 : Review feedback #
Total comments: 11
Patch Set 5 : Fixed nits/comments #
Total comments: 4
Patch Set 6 : More review feedback. #
Messages
Total messages: 20 (0 generated)
isherman: PTAL at histograms.xml joao: PTAL at the policy stuff
+Thiemo: FYI These changes lgtm. Do you think it's worth adding the UMA sampling suggested at https://codereview.chromium.org/386573002/diff/1/components/policy/core/brows... to this CL?
On 2014/07/11 12:55:42, Joao da Silva wrote: > +Thiemo: FYI > > These changes lgtm. > > Do you think it's worth adding the UMA sampling suggested at > https://codereview.chromium.org/386573002/diff/1/components/policy/core/brows... > to this CL? Made the change you suggested. PTAL if you want.
Add the new metrics to histograms.xml too. https://codereview.chromium.org/382813002/diff/40001/components/policy/core/b... File components/policy/core/browser/browser_policy_connector.cc (right): https://codereview.chromium.org/382813002/diff/40001/components/policy/core/b... components/policy/core/browser/browser_policy_connector.cc:59: index, max); WDYT of sampling the UErrorCode in "status" with UMA_HISTOGRAM_SPARSE_SLOWLY? That would tell us what is wrong with the pattern too (given that we can't repro this locally... it may point at a strange locale or whatever)
I already did add the new metrics to histograms.xml, I thought. Did I miss something? https://codereview.chromium.org/382813002/diff/40001/components/policy/core/b... File components/policy/core/browser/browser_policy_connector.cc (right): https://codereview.chromium.org/382813002/diff/40001/components/policy/core/b... components/policy/core/browser/browser_policy_connector.cc:59: index, max); On 2014/07/11 15:13:47, Joao da Silva wrote: > WDYT of sampling the UErrorCode in "status" with UMA_HISTOGRAM_SPARSE_SLOWLY? > That would tell us what is wrong with the pattern too (given that we can't repro > this locally... it may point at a strange locale or whatever) I considered that, but I don't know what UMA_HISTOGRAM_SPARSE_SLOWLY is because it's totally undocumented, so I'm uncomfortable using it. I can drive this in a separate CL once someone responds to my email to chromium-dev about it.
lgtm On 2014/07/11 15:46:22, Andrew T Wilson wrote: > I already did add the new metrics to histograms.xml, I thought. Did I miss > something? > Indeed :-) Turns out that histograms.xml is huge and rietveld doesn't have the base file version, so patch set 3 does not indicate that the file changed since patch sets 1 and 2... that's why I thought it wasn't updated. My bad.
https://codereview.chromium.org/382813002/diff/40001/components/policy/core/b... File components/policy/core/browser/browser_policy_connector.cc (right): https://codereview.chromium.org/382813002/diff/40001/components/policy/core/b... components/policy/core/browser/browser_policy_connector.cc:59: index, max); On 2014/07/11 15:46:21, Andrew T Wilson wrote: > On 2014/07/11 15:13:47, Joao da Silva wrote: > > WDYT of sampling the UErrorCode in "status" with UMA_HISTOGRAM_SPARSE_SLOWLY? > > That would tell us what is wrong with the pattern too (given that we can't > repro > > this locally... it may point at a strange locale or whatever) > > I considered that, but I don't know what UMA_HISTOGRAM_SPARSE_SLOWLY is because > it's totally undocumented, so I'm uncomfortable using it. I can drive this in a > separate CL once someone responds to my email to chromium-dev about it. UMA_HISTOGRAM_SPARSE_SLOWLY just uses a map rather than a vector. https://codereview.chromium.org/382813002/diff/40001/components/policy/core/b... components/policy/core/browser/browser_policy_connector.cc:59: index, max); Hmm, I'm concerned that |max| is being passed in as a parameter -- is it not a compile-time constant? UMA_HISTOGRAM_ENUMERATION defines a fixed-size vector for the number of buckets specified by the third parameter (in this case, max), so it's important that this be a constant. https://codereview.chromium.org/382813002/diff/40001/components/policy/core/b... components/policy/core/browser/browser_policy_connector.cc:62: UMA_HISTOGRAM_BOOLEAN("Enterprise.DomainWhitelistRegexSuccess", true); Please define a wrapper method around emitting to this histogram, so that there's less chance of a typo sneaking into just one of the code paths. https://codereview.chromium.org/382813002/diff/40001/components/policy/core/b... components/policy/core/browser/browser_policy_connector.cc:220: if (MatchDomain(domain, pattern, i, arraysize(kNonManagedDomainPatterns))) Hmm, UMA is not intended to be used to track user navigation history. Please check with the privacy team whether this is an appropriate metric to record via UMA. Note that if you are really quite interested in this metric, you could also use RAPPOR, which has higher privacy guarantees, and hence is more appropriate for measuring data in aggregate that would be PII at the individual level. https://codereview.chromium.org/382813002/diff/40001/tools/metrics/histograms... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/382813002/diff/40001/tools/metrics/histograms... tools/metrics/histograms/histograms.xml:5318: +<histogram name="Enterprise.DomainWhitelistRegexFailureIndex"> Please associate an enum with this histogram, rather than just tracking an opaque index. https://codereview.chromium.org/382813002/diff/40001/tools/metrics/histograms... tools/metrics/histograms/histograms.xml:5321: + Tracks which domain caused an IcuMatcher initialization failure, to help What is an IcuMatcher? Codesearch gives zero hits for this term. https://codereview.chromium.org/382813002/diff/40001/tools/metrics/histograms... tools/metrics/histograms/histograms.xml:5326: +<histogram name="Enterprise.DomainWhitelistRegexSuccess" enum="Boolean"> Please associate a more semantically contentful enum. It's currently unclear to me, from the combination of the <summary> and the enum, what this histogram is actually measuring. https://codereview.chromium.org/382813002/diff/40001/tools/metrics/histograms... tools/metrics/histograms/histograms.xml:5380: +<histogram name="Enterprise.PolicyHasVerifiedCachedKey" enum="Boolean"> Please associate a more semantically contentful enum.
https://codereview.chromium.org/382813002/diff/40001/chrome/browser/ui/sync/o... File chrome/browser/ui/sync/one_click_signin_sync_starter.cc (right): https://codereview.chromium.org/382813002/diff/40001/chrome/browser/ui/sync/o... chrome/browser/ui/sync/one_click_signin_sync_starter.cc:55: SIGNIN_CHOICE_CANCEL, I'd suggest adding explicit integer values because that makes it easier to validate against histograms.xml and also drives the point that it is a bad idea to add another value in between existing values. https://codereview.chromium.org/382813002/diff/40001/chrome/browser/ui/sync/o... chrome/browser/ui/sync/one_click_signin_sync_starter.cc:58: SIGNIN_CHOICE_SIZE, Could you please document that this is a counter and not a real signin choice? https://codereview.chromium.org/382813002/diff/40001/components/policy/core/c... File components/policy/core/common/cloud/user_cloud_policy_store.cc (right): https://codereview.chromium.org/382813002/diff/40001/components/policy/core/c... components/policy/core/common/cloud/user_cloud_policy_store.cc:21: enum PolicyLoadStatus { Could you please add a note that the enum must be kept in sync with histograms.xml? There's boilerplate at http://www.chromium.org/Home/user-metrics . https://codereview.chromium.org/382813002/diff/40001/components/policy/core/c... components/policy/core/common/cloud/user_cloud_policy_store.cc:31: LOAD_RESULT_SIZE, Please add a comment that SIZE must be last and is not a real PolicyLoadStatus.
PTAL https://codereview.chromium.org/382813002/diff/40001/chrome/browser/ui/sync/o... File chrome/browser/ui/sync/one_click_signin_sync_starter.cc (right): https://codereview.chromium.org/382813002/diff/40001/chrome/browser/ui/sync/o... chrome/browser/ui/sync/one_click_signin_sync_starter.cc:55: SIGNIN_CHOICE_CANCEL, On 2014/07/18 12:12:26, Thiemo Nagel wrote: > I'd suggest adding explicit integer values because that makes it easier to > validate against histograms.xml and also drives the point that it is a bad idea > to add another value in between existing values. Done. https://codereview.chromium.org/382813002/diff/40001/chrome/browser/ui/sync/o... chrome/browser/ui/sync/one_click_signin_sync_starter.cc:58: SIGNIN_CHOICE_SIZE, On 2014/07/18 12:12:26, Thiemo Nagel wrote: > Could you please document that this is a counter and not a real signin choice? Done. https://codereview.chromium.org/382813002/diff/40001/components/policy/core/b... File components/policy/core/browser/browser_policy_connector.cc (right): https://codereview.chromium.org/382813002/diff/40001/components/policy/core/b... components/policy/core/browser/browser_policy_connector.cc:59: index, max); On 2014/07/11 22:54:12, Ilya Sherman wrote: > Hmm, I'm concerned that |max| is being passed in as a parameter -- is it not a > compile-time constant? UMA_HISTOGRAM_ENUMERATION defines a fixed-size vector > for the number of buckets specified by the third parameter (in this case, max), > so it's important that this be a constant. Done. https://codereview.chromium.org/382813002/diff/40001/components/policy/core/b... components/policy/core/browser/browser_policy_connector.cc:59: index, max); On 2014/07/11 22:54:12, Ilya Sherman wrote: > On 2014/07/11 15:46:21, Andrew T Wilson wrote: > > On 2014/07/11 15:13:47, Joao da Silva wrote: > > > WDYT of sampling the UErrorCode in "status" with > UMA_HISTOGRAM_SPARSE_SLOWLY? > > > That would tell us what is wrong with the pattern too (given that we can't > > repro > > > this locally... it may point at a strange locale or whatever) > > > > I considered that, but I don't know what UMA_HISTOGRAM_SPARSE_SLOWLY is > because > > it's totally undocumented, so I'm uncomfortable using it. I can drive this in > a > > separate CL once someone responds to my email to chromium-dev about it. > > UMA_HISTOGRAM_SPARSE_SLOWLY just uses a map rather than a vector. So, what's the right way to use UMA_HISTOGRAM_SPARSE_SLOWLY() in my case? The value I'm logging varies from -128 through around 0x10501 (from third_party/icu/source/common/unicode/utypes.h). Do I need to try to add an enum to histograms.xml for every single expected value, or is it possible to just have it track this as an unlabeled int? What's the right way to do this? Use a "units" attribute? https://codereview.chromium.org/382813002/diff/40001/components/policy/core/b... components/policy/core/browser/browser_policy_connector.cc:62: UMA_HISTOGRAM_BOOLEAN("Enterprise.DomainWhitelistRegexSuccess", true); On 2014/07/11 22:54:12, Ilya Sherman wrote: > Please define a wrapper method around emitting to this histogram, so that > there's less chance of a typo sneaking into just one of the code paths. Done. https://codereview.chromium.org/382813002/diff/40001/components/policy/core/b... components/policy/core/browser/browser_policy_connector.cc:220: if (MatchDomain(domain, pattern, i, arraysize(kNonManagedDomainPatterns))) On 2014/07/11 22:54:12, Ilya Sherman wrote: > Hmm, UMA is not intended to be used to track user navigation history. Please > check with the privacy team whether this is an appropriate metric to record via > UMA. Note that if you are really quite interested in this metric, you could > also use RAPPOR, which has higher privacy guarantees, and hence is more > appropriate for measuring data in aggregate that would be PII at the individual > level. I'm not tracking user navigation. During signin, we generate a set of ICUMatchers that are used to check if the signin domain is known to be enterprise or not. The creation of the ICUMatcher is failing, and we want to know which one - we're just returning the index of a hard-coded regex, not anything related to the user data (it has nothing to do with the actual user's domain, so no PII is being transmitted). https://codereview.chromium.org/382813002/diff/40001/components/policy/core/c... File components/policy/core/common/cloud/user_cloud_policy_store.cc (right): https://codereview.chromium.org/382813002/diff/40001/components/policy/core/c... components/policy/core/common/cloud/user_cloud_policy_store.cc:21: enum PolicyLoadStatus { On 2014/07/18 12:12:26, Thiemo Nagel wrote: > Could you please add a note that the enum must be kept in sync with > histograms.xml? There's boilerplate at > http://www.chromium.org/Home/user-metrics . Done. https://codereview.chromium.org/382813002/diff/40001/components/policy/core/c... components/policy/core/common/cloud/user_cloud_policy_store.cc:31: LOAD_RESULT_SIZE, On 2014/07/18 12:12:26, Thiemo Nagel wrote: > Please add a comment that SIZE must be last and is not a real PolicyLoadStatus. Done. https://codereview.chromium.org/382813002/diff/40001/tools/metrics/histograms... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/382813002/diff/40001/tools/metrics/histograms... tools/metrics/histograms/histograms.xml:5318: +<histogram name="Enterprise.DomainWhitelistRegexFailureIndex"> On 2014/07/11 22:54:13, Ilya Sherman wrote: > Please associate an enum with this histogram, rather than just tracking an > opaque index. Done. https://codereview.chromium.org/382813002/diff/40001/tools/metrics/histograms... tools/metrics/histograms/histograms.xml:5321: + Tracks which domain caused an IcuMatcher initialization failure, to help On 2014/07/11 22:54:12, Ilya Sherman wrote: > What is an IcuMatcher? Codesearch gives zero hits for this term. Updated this to icu::RegexMatcher https://codereview.chromium.org/382813002/diff/40001/tools/metrics/histograms... tools/metrics/histograms/histograms.xml:5326: +<histogram name="Enterprise.DomainWhitelistRegexSuccess" enum="Boolean"> On 2014/07/11 22:54:13, Ilya Sherman wrote: > Please associate a more semantically contentful enum. It's currently unclear to > me, from the combination of the <summary> and the enum, what this histogram is > actually measuring. OK, I didn't realize that best practices recommended that we add new BooleanXXXX enums to capture this semantic information. I've added this. https://codereview.chromium.org/382813002/diff/40001/tools/metrics/histograms... tools/metrics/histograms/histograms.xml:5380: +<histogram name="Enterprise.PolicyHasVerifiedCachedKey" enum="Boolean"> On 2014/07/11 22:54:12, Ilya Sherman wrote: > Please associate a more semantically contentful enum. Done.
https://codereview.chromium.org/382813002/diff/40001/components/policy/core/b... File components/policy/core/browser/browser_policy_connector.cc (right): https://codereview.chromium.org/382813002/diff/40001/components/policy/core/b... components/policy/core/browser/browser_policy_connector.cc:59: index, max); On 2014/07/24 15:25:56, Andrew T Wilson wrote: > On 2014/07/11 22:54:12, Ilya Sherman wrote: > > On 2014/07/11 15:46:21, Andrew T Wilson wrote: > > > On 2014/07/11 15:13:47, Joao da Silva wrote: > > > > WDYT of sampling the UErrorCode in "status" with > > UMA_HISTOGRAM_SPARSE_SLOWLY? > > > > That would tell us what is wrong with the pattern too (given that we can't > > > repro > > > > this locally... it may point at a strange locale or whatever) > > > > > > I considered that, but I don't know what UMA_HISTOGRAM_SPARSE_SLOWLY is > > because > > > it's totally undocumented, so I'm uncomfortable using it. I can drive this > in > > a > > > separate CL once someone responds to my email to chromium-dev about it. > > > > UMA_HISTOGRAM_SPARSE_SLOWLY just uses a map rather than a vector. > > So, what's the right way to use UMA_HISTOGRAM_SPARSE_SLOWLY() in my case? The > value I'm logging varies from -128 through around 0x10501 (from > third_party/icu/source/common/unicode/utypes.h). Do I need to try to add an enum > to histograms.xml for every single expected value, or is it possible to just > have it track this as an unlabeled int? What's the right way to do this? Use a > "units" attribute? It's up to you what values you list in histograms.xml -- if you prefer not to include labels for any of the logged values, that's ok. However, my recommendation would be to include all of the constants from utypes.h as labels. You could probably write a quick script to generate these labels, and then you would have an easier time making sense of the histogram from the dashboard. https://codereview.chromium.org/382813002/diff/40001/components/policy/core/b... components/policy/core/browser/browser_policy_connector.cc:220: if (MatchDomain(domain, pattern, i, arraysize(kNonManagedDomainPatterns))) On 2014/07/24 15:25:56, Andrew T Wilson wrote: > On 2014/07/11 22:54:12, Ilya Sherman wrote: > > Hmm, UMA is not intended to be used to track user navigation history. Please > > check with the privacy team whether this is an appropriate metric to record > via > > UMA. Note that if you are really quite interested in this metric, you could > > also use RAPPOR, which has higher privacy guarantees, and hence is more > > appropriate for measuring data in aggregate that would be PII at the > individual > > level. > > I'm not tracking user navigation. During signin, we generate a set of > ICUMatchers that are used to check if the signin domain is known to be > enterprise or not. The creation of the ICUMatcher is failing, and we want to > know which one - we're just returning the index of a hard-coded regex, not > anything related to the user data (it has nothing to do with the actual user's > domain, so no PII is being transmitted). I'm confused -- the failure is independent of the user input? If so, why not just debug locally, rather than via UMA data? https://codereview.chromium.org/382813002/diff/60001/components/policy/core/b... File components/policy/core/browser/browser_policy_connector.cc (right): https://codereview.chromium.org/382813002/diff/60001/components/policy/core/b... components/policy/core/browser/browser_policy_connector.cc:48: nit: Spurious newline https://codereview.chromium.org/382813002/diff/60001/components/policy/core/b... components/policy/core/browser/browser_policy_connector.cc:52: const wchar_t* kNonManagedDomainPatterns[] = { nit: Can this be more const? https://codereview.chromium.org/382813002/diff/60001/components/policy/core/b... components/policy/core/browser/browser_policy_connector.cc:56: L"hotmail(\\.co|\\.com|)\\.[^.]+", // hotmail.com, hotmail.it, hotmail.co.uk Hmm, this is a slightly strange regex -- why specify the ".co" suffix explicitly, if you then allow any suffix later on? https://codereview.chromium.org/382813002/diff/60001/components/policy/core/b... components/policy/core/browser/browser_policy_connector.cc:81: index, arraysize(kNonManagedDomainPatterns)); Note that as you have this set up, the only valid change to the array is to append at the end; otherwise, changes to the array will confuse the metrics. I'd recommend having a function that explicitly maps from strings to enum constants instead. https://codereview.chromium.org/382813002/diff/60001/tools/metrics/histograms... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/382813002/diff/60001/tools/metrics/histograms... tools/metrics/histograms/histograms.xml:5378: +<histogram name="Enterprise.DomainWhitelistRegexFailureIndex" Now that you've associated an enum with this histogram, there's probably no need to mention "index" in the name, nor in the description. https://codereview.chromium.org/382813002/diff/60001/tools/metrics/histograms... tools/metrics/histograms/histograms.xml:5400: + Tracks instances of IcuMatcher initialization, to help figure out the cause nit: Please update "IcuMatcher" -> "icu::RegexMatcher" throughout. https://codereview.chromium.org/382813002/diff/60001/tools/metrics/histograms... tools/metrics/histograms/histograms.xml:37363: + <int value="0" label="AOL"/> nit: Why is "AOL" capitalized, but the rest all in lowercase?
On 2014/07/24 17:29:33, Ilya Sherman wrote: > > I'm confused -- the failure is independent of the user input? If so, why not > just debug locally, rather than via UMA data? Because we've never reproduced the crash - it's rare. We basically worked around the crash by exiting if there's an error, so now we want to gather information about the type of error icu is generating and which regex it's choking on so we can see if we can fix the error.
I'll address your other comments in an update to this CL tomorrow, but wanted to reply to a few now. https://codereview.chromium.org/382813002/diff/60001/components/policy/core/b... File components/policy/core/browser/browser_policy_connector.cc (right): https://codereview.chromium.org/382813002/diff/60001/components/policy/core/b... components/policy/core/browser/browser_policy_connector.cc:56: L"hotmail(\\.co|\\.com|)\\.[^.]+", // hotmail.com, hotmail.it, hotmail.co.uk On 2014/07/24 17:29:33, Ilya Sherman wrote: > Hmm, this is a slightly strange regex -- why specify the ".co" suffix > explicitly, if you then allow any suffix later on? Agreed, I can fix this in a separate CL (this isn't new code - I just moved it up here so I could access arraysize(kNonManagedDomainPatterns) from here. https://codereview.chromium.org/382813002/diff/60001/components/policy/core/b... components/policy/core/browser/browser_policy_connector.cc:81: index, arraysize(kNonManagedDomainPatterns)); On 2014/07/24 17:29:33, Ilya Sherman wrote: > Note that as you have this set up, the only valid change to the array is to > append at the end; otherwise, changes to the array will confuse the metrics. > I'd recommend having a function that explicitly maps from strings to enum > constants instead. Yes. My goal isn't to have this be a long-term UMA, but rather to try to gather information about a crash.
On 2014/07/24 19:49:21, Andrew T Wilson wrote: > On 2014/07/24 17:29:33, Ilya Sherman wrote: > > > > I'm confused -- the failure is independent of the user input? If so, why not > > just debug locally, rather than via UMA data? > > Because we've never reproduced the crash - it's rare. We basically worked around > the crash by exiting if there's an error, so now we want to gather information > about the type of error icu is generating and which regex it's choking on so we > can see if we can fix the error. Fair enough -- thanks for clarifying that for me. It'd be great to also clarify that in the histogram descriptions :)
Also updated the histogram description as you suggested. PTAL. https://codereview.chromium.org/382813002/diff/60001/components/policy/core/b... File components/policy/core/browser/browser_policy_connector.cc (right): https://codereview.chromium.org/382813002/diff/60001/components/policy/core/b... components/policy/core/browser/browser_policy_connector.cc:48: On 2014/07/24 17:29:32, Ilya Sherman wrote: > nit: Spurious newline Done. https://codereview.chromium.org/382813002/diff/60001/components/policy/core/b... components/policy/core/browser/browser_policy_connector.cc:52: const wchar_t* kNonManagedDomainPatterns[] = { On 2014/07/24 17:29:33, Ilya Sherman wrote: > nit: Can this be more const? Done.
Thanks. LGTM % my remaining comments. https://codereview.chromium.org/382813002/diff/80001/components/policy/core/b... File components/policy/core/browser/browser_policy_connector.cc (right): https://codereview.chromium.org/382813002/diff/80001/components/policy/core/b... components/policy/core/browser/browser_policy_connector.cc:80: UMA_HISTOGRAM_ENUMERATION("Enterprise.DomainWhitelistRegexFailureIndex", Please update the histogram name here as well ;) https://codereview.chromium.org/382813002/diff/80001/tools/metrics/histograms... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/382813002/diff/80001/tools/metrics/histograms... tools/metrics/histograms/histograms.xml:5402: + of http://crbug.com/365351. nit: Is this also a temporary metric?
https://codereview.chromium.org/382813002/diff/80001/components/policy/core/b... File components/policy/core/browser/browser_policy_connector.cc (right): https://codereview.chromium.org/382813002/diff/80001/components/policy/core/b... components/policy/core/browser/browser_policy_connector.cc:80: UMA_HISTOGRAM_ENUMERATION("Enterprise.DomainWhitelistRegexFailureIndex", On 2014/07/25 17:34:05, Ilya Sherman wrote: > Please update the histogram name here as well ;) Good catch, thanks! https://codereview.chromium.org/382813002/diff/80001/tools/metrics/histograms... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/382813002/diff/80001/tools/metrics/histograms... tools/metrics/histograms/histograms.xml:5402: + of http://crbug.com/365351. On 2014/07/25 17:34:05, Ilya Sherman wrote: > nit: Is this also a temporary metric? Done.
The CQ bit was checked by atwilson@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/atwilson@chromium.org/382813002/100001
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: linux_chromium_chromeos_clang_dbg on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) win_chromium_rel on tryserver.chromium.win (http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel/...)
Message was sent while issue was closed.
Change committed as 285903 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
