Fix presubmit_scheme_histograms.py and presubmit_bad_message_reasons.py

Fix presubmit_scheme_histograms.py and presubmit_bad_message_reasons.py

This CL fixes presubmit_scheme_histograms.py and
presubmit_bad_message_reasons.py which after r462591 would always report
presubmit errors (because it started returning a tuple from HistogramNeedsUpdate
which always coerces to True in the broken presubmit scripts).

After r462591 HistogramNeedsUpdate effectively checks all
histogram-enum-related presubmits (both for updating histograms.xml and
for detecting duplicate values).  Therefore the current CL renames this
function to CheckPresubmitErrors and makes it return a presubmit error
(rather than a tuple of (needs_updating, duplicates)).  This refactoring
has a nice side benefit - after this CL, all presubmits going through
CheckPresubmitErrors will check for duplicate values (r462591 only added
this check for UseCounter histograms).

This CL also tweaks content/browser/PRESUBMIT.py so that it applies
*both* to upload-time and commit-time checks (without this change
|git cl presubmit| would not hit the checks related to
content/browser/bad_message.h - they would only be hit when running
|git cl presubmit --upload|).

Manual testing done:
- Make changes in content/browser/bad_message.h,
  chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc,
  third_party/WebKit/Source/code/frame
- Run git cl presubmit and verify that the presubmit
  checks detect 1) duplicates and 2) the need to update histograms.xml

BUG=577772
TEST=See "Manual testing done" above.
TBR=sky@chromium.org, jochen@chromium.org, rdevlin.cronin@chromium.org

Review-Url: https://codereview.chromium.org/2841823007
Cr-Commit-Position: refs/heads/master@{#468003}
Committed: https://chromium.googlesource.com/chromium/src/+/14d9a1a8cade0d1b9878c4bda0bbc8417914893f

[Łukasz Anforowicz, 2017-04-26 22:15:58]

Description was changed from

==========
Fix presubmit_scheme_histograms.py and presubmit_bad_message_reasons.py

This CL fixes presubmit_scheme_histograms.py and
presubmit_bad_message_reasons.py which after r462591 would always report
presubmit errors (because it started returning a tuple from HistogramNeedsUpdate
which always coerces to True in the broken presubmit scripts).

After r462591 HistogramNeedsUpdate effectively checks all
histogram-enum-related presubmits (both for updating histograms.xml and
for detecting duplicate values).  Therefore the current renames this
function to CheckPresubmitErrors and makes it return a presubmit error
(rather than a tuple of (needs_updating, duplicates)).  This refactoring
has a nice side benefit - after this CL, all presubmits going through
CheckPresubmitErrors will check for duplicate values (r462591 only added
this check for UseCounter histograms).

This CL also tweaks content/browser/PRESUBMIT.py so that it applies
*both* to upload-time and commit-time checks (without this change
|git cl presubmit| would not hit the checks related to
content/browser/bad_message.h - they would only be hit when running
|git cl presubmit --upload|).

Test steps:
- Make changes in content/browser/bad_message.h,
  chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc,
  third_party/WebKit/Source/code/frame
- Run git cl presubmit and verify that the presubmit
  checks detect 1) duplicates and 2) the need to update histograms.xml

BUG=577772
==========

to

==========
Fix presubmit_scheme_histograms.py and presubmit_bad_message_reasons.py

This CL fixes presubmit_scheme_histograms.py and
presubmit_bad_message_reasons.py which after r462591 would always report
presubmit errors (because it started returning a tuple from HistogramNeedsUpdate
which always coerces to True in the broken presubmit scripts).

After r462591 HistogramNeedsUpdate effectively checks all
histogram-enum-related presubmits (both for updating histograms.xml and
for detecting duplicate values).  Therefore the current renames this
function to CheckPresubmitErrors and makes it return a presubmit error
(rather than a tuple of (needs_updating, duplicates)).  This refactoring
has a nice side benefit - after this CL, all presubmits going through
CheckPresubmitErrors will check for duplicate values (r462591 only added
this check for UseCounter histograms).

This CL also tweaks content/browser/PRESUBMIT.py so that it applies
*both* to upload-time and commit-time checks (without this change
|git cl presubmit| would not hit the checks related to
content/browser/bad_message.h - they would only be hit when running
|git cl presubmit --upload|).

Test steps:
- Make changes in content/browser/bad_message.h,
  chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc,
  third_party/WebKit/Source/code/frame
- Run git cl presubmit and verify that the presubmit
  checks detect 1) duplicates and 2) the need to update histograms.xml

BUG=577772
==========

[Łukasz Anforowicz, 2017-04-26 22:15:58]

lukasza@chromium.org changed reviewers:
+ isherman@chromium.org

[Łukasz Anforowicz, 2017-04-26 22:17:46]

Description was changed from

==========
Fix presubmit_scheme_histograms.py and presubmit_bad_message_reasons.py

This CL fixes presubmit_scheme_histograms.py and
presubmit_bad_message_reasons.py which after r462591 would always report
presubmit errors (because it started returning a tuple from HistogramNeedsUpdate
which always coerces to True in the broken presubmit scripts).

After r462591 HistogramNeedsUpdate effectively checks all
histogram-enum-related presubmits (both for updating histograms.xml and
for detecting duplicate values).  Therefore the current renames this
function to CheckPresubmitErrors and makes it return a presubmit error
(rather than a tuple of (needs_updating, duplicates)).  This refactoring
has a nice side benefit - after this CL, all presubmits going through
CheckPresubmitErrors will check for duplicate values (r462591 only added
this check for UseCounter histograms).

This CL also tweaks content/browser/PRESUBMIT.py so that it applies
*both* to upload-time and commit-time checks (without this change
|git cl presubmit| would not hit the checks related to
content/browser/bad_message.h - they would only be hit when running
|git cl presubmit --upload|).

Test steps:
- Make changes in content/browser/bad_message.h,
  chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc,
  third_party/WebKit/Source/code/frame
- Run git cl presubmit and verify that the presubmit
  checks detect 1) duplicates and 2) the need to update histograms.xml

BUG=577772
==========

to

==========
Fix presubmit_scheme_histograms.py and presubmit_bad_message_reasons.py

This CL fixes presubmit_scheme_histograms.py and
presubmit_bad_message_reasons.py which after r462591 would always report
presubmit errors (because it started returning a tuple from HistogramNeedsUpdate
which always coerces to True in the broken presubmit scripts).

After r462591 HistogramNeedsUpdate effectively checks all
histogram-enum-related presubmits (both for updating histograms.xml and
for detecting duplicate values).  Therefore the current renames this
function to CheckPresubmitErrors and makes it return a presubmit error
(rather than a tuple of (needs_updating, duplicates)).  This refactoring
has a nice side benefit - after this CL, all presubmits going through
CheckPresubmitErrors will check for duplicate values (r462591 only added
this check for UseCounter histograms).

This CL also tweaks content/browser/PRESUBMIT.py so that it applies
*both* to upload-time and commit-time checks (without this change
|git cl presubmit| would not hit the checks related to
content/browser/bad_message.h - they would only be hit when running
|git cl presubmit --upload|).

Manual testing done:
- Make changes in content/browser/bad_message.h,
  chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc,
  third_party/WebKit/Source/code/frame
- Run git cl presubmit and verify that the presubmit
  checks detect 1) duplicates and 2) the need to update histograms.xml

BUG=577772
TEST=See "Manual testing done" above.
==========

[Łukasz Anforowicz, 2017-04-26 22:20:54]

isherman@, could you PTAL?

https://codereview.chromium.org/2841823007/diff/20001/content/browser/PRESUBM...
File content/browser/PRESUBMIT.py (right):

https://codereview.chromium.org/2841823007/diff/20001/content/browser/PRESUBM...
content/browser/PRESUBMIT.py:14: 
I don't understand why we have separate CheckChangeOnCommit and a separate
CheckChangeOnUpload.  Sometimes commit happens a little while after an upload
and it seems useful to have the same checks apply in both cases (in case
something landed recently that would have messed up the histogram enums).

I see that other presubmit scripts (i.e. the one in
//chrome/browser/PRESUBMIT.py) use the _CommonChecks pattern to work around
this.

https://codereview.chromium.org/2841823007/diff/20001/tools/metrics/histogram...
File tools/metrics/histograms/update_histogram_enum.py (left):

https://codereview.chromium.org/2841823007/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/update_histogram_enum.py:95: label = label[1:]
Stripping of 'k' prefix needs to happen *before* reporting duplicates (otherwise
the |label| doesn't have the prefix stripped, but |result[enum_value]| does have
the prefix stripped) and this results in an inconsistently looking error message
being returned from CheckPresubmitErrors below.

[Łukasz Anforowicz, 2017-04-26 22:30:23]

Description was changed from

==========
Fix presubmit_scheme_histograms.py and presubmit_bad_message_reasons.py

This CL fixes presubmit_scheme_histograms.py and
presubmit_bad_message_reasons.py which after r462591 would always report
presubmit errors (because it started returning a tuple from HistogramNeedsUpdate
which always coerces to True in the broken presubmit scripts).

After r462591 HistogramNeedsUpdate effectively checks all
histogram-enum-related presubmits (both for updating histograms.xml and
for detecting duplicate values).  Therefore the current renames this
function to CheckPresubmitErrors and makes it return a presubmit error
(rather than a tuple of (needs_updating, duplicates)).  This refactoring
has a nice side benefit - after this CL, all presubmits going through
CheckPresubmitErrors will check for duplicate values (r462591 only added
this check for UseCounter histograms).

This CL also tweaks content/browser/PRESUBMIT.py so that it applies
*both* to upload-time and commit-time checks (without this change
|git cl presubmit| would not hit the checks related to
content/browser/bad_message.h - they would only be hit when running
|git cl presubmit --upload|).

Manual testing done:
- Make changes in content/browser/bad_message.h,
  chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc,
  third_party/WebKit/Source/code/frame
- Run git cl presubmit and verify that the presubmit
  checks detect 1) duplicates and 2) the need to update histograms.xml

BUG=577772
TEST=See "Manual testing done" above.
==========

to

==========
Fix presubmit_scheme_histograms.py and presubmit_bad_message_reasons.py

This CL fixes presubmit_scheme_histograms.py and
presubmit_bad_message_reasons.py which after r462591 would always report
presubmit errors (because it started returning a tuple from HistogramNeedsUpdate
which always coerces to True in the broken presubmit scripts).

After r462591 HistogramNeedsUpdate effectively checks all
histogram-enum-related presubmits (both for updating histograms.xml and
for detecting duplicate values).  Therefore the current CL renames this
function to CheckPresubmitErrors and makes it return a presubmit error
(rather than a tuple of (needs_updating, duplicates)).  This refactoring
has a nice side benefit - after this CL, all presubmits going through
CheckPresubmitErrors will check for duplicate values (r462591 only added
this check for UseCounter histograms).

This CL also tweaks content/browser/PRESUBMIT.py so that it applies
*both* to upload-time and commit-time checks (without this change
|git cl presubmit| would not hit the checks related to
content/browser/bad_message.h - they would only be hit when running
|git cl presubmit --upload|).

Manual testing done:
- Make changes in content/browser/bad_message.h,
  chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc,
  third_party/WebKit/Source/code/frame
- Run git cl presubmit and verify that the presubmit
  checks detect 1) duplicates and 2) the need to update histograms.xml

BUG=577772
TEST=See "Manual testing done" above.
==========

[Ilya Sherman, 2017-04-26 23:19:08]

Thanks!  It seems that we need better testing of this system somehow, given the
number of fixes that have been needed.

Before I review, would you mind taking a look at two related CLs, and seeing
whether there are any additional changes needed w.r.t. them? 
https://codereview.chromium.org/2815773004 previously landed and updates a
couple of calls that you are not updating in this CL -- should you be? 
https://codereview.chromium.org/2835733002/ is currently in review, and seems
somewhat related to this CL.  Should that review be abandoned?  Are there any
changes within that CL that make sense to combine with your CL?

Thanks!

[Łukasz Anforowicz, 2017-04-27 02:34:46]

On 2017/04/26 23:19:08, Ilya Sherman wrote:
> Thanks!  It seems that we need better testing of this system somehow, given
the
> number of fixes that have been needed.

Good point.  I wonder if these presubmits would be possible to cover with
chromium/src/PRESUBMIT_test.py - if yes, then I guess we should add a test for
each of the 3 covered files.

Feel free to take a look at the changes I have so far, but we probably shouldn't
land this CL until the testing part is figured out (I do hope that the existing
PRESUBMIT_test.py harness is sufficient here).

> Before I review, would you mind taking a look at two related CLs, and seeing
> whether there are any additional changes needed w.r.t. them? 
> https://codereview.chromium.org/2815773004 previously landed and updates a
> couple of calls that you are not updating in this CL -- should you be? 

This other CL deals with callers of ReadHistogramValues, which I am not touching
in the CL under review.  AFAICT, I've modified and tested all the callers of the
old HistogramNeedsUpdate (at least the ones found by chromium code search).

> https://codereview.chromium.org/2835733002/ is currently in review, and seems
> somewhat related to this CL.  Should that review be abandoned?  Are there any
> changes within that CL that make sense to combine with your CL?

Hmmm... I can see that one might prefer to report duplicates as an exception
(rather than as an extra output from ReadHistogramValues).  OTOH, I think the
other CL is kind of orthogonal to the current one: the current CL under review
fixes presubmit checks in 2 places, the other CL is a refactoring that shouldn't
change behavior.

I am not sure what is the best way to go forward.  I think we can proceed with
both CLs independently (i.e. if the other CL lands first, I can check for an
exception inside HistogramNeedsUpdate/CheckPresubmitErrors).  WDYT?

[Łukasz Anforowicz, 2017-04-27 16:42:40]

On 2017/04/27 02:34:46, Łukasz A. wrote:
> On 2017/04/26 23:19:08, Ilya Sherman wrote:
> > Thanks!  It seems that we need better testing of this system somehow, given
> the
> > number of fixes that have been needed.
> 
> Good point.  I wonder if these presubmits would be possible to cover with
> chromium/src/PRESUBMIT_test.py - if yes, then I guess we should add a test for
> each of the 3 covered files.
> 
> Feel free to take a look at the changes I have so far, but we probably
shouldn't
> land this CL until the testing part is figured out (I do hope that the
existing
> PRESUBMIT_test.py harness is sufficient here).

I've found ./PRESUBMIT_test.py and , but I see that they are not run as part of
CQ.  I've opened https://crbug.com/716067 to track this (not sure who can help
with this - I am not very familiar with how CQ operates).

Given the above will likely take a while, I think we should proceed with just
reviewing and landing the fixes from the CL under review (i.e. without
corresponding tests... :-/ ).

Sounds good?

[Łukasz Anforowicz, 2017-04-27 16:44:15]

On 2017/04/27 16:42:40, Łukasz A. wrote:
> On 2017/04/27 02:34:46, Łukasz A. wrote:
> > On 2017/04/26 23:19:08, Ilya Sherman wrote:
> > > Thanks!  It seems that we need better testing of this system somehow,
given
> > the
> > > number of fixes that have been needed.
> > 
> > Good point.  I wonder if these presubmits would be possible to cover with
> > chromium/src/PRESUBMIT_test.py - if yes, then I guess we should add a test
for
> > each of the 3 covered files.
> > 
> > Feel free to take a look at the changes I have so far, but we probably
> shouldn't
> > land this CL until the testing part is figured out (I do hope that the
> existing
> > PRESUBMIT_test.py harness is sufficient here).
> 
> I've found ./PRESUBMIT_test.py and , but I see that they are not run as part
of
> CQ.  I've opened https://crbug.com/716067 to track this (not sure who can help
> with this - I am not very familiar with how CQ operates).
> 
> Given the above will likely take a while, I think we should proceed with just
> reviewing and landing the fixes from the CL under review (i.e. without
> corresponding tests... :-/ ).
> 
> Sounds good?

Errr... I should proof-read before hitting send :-).  I meant to say:

I've found ./PRESUBMIT_test.py and chrome/browser/resources/PRESUBMIT_test.py,
but I see that
1. they are not run as part of CQ
2. there is not PRESUBMIT_test.py covering presubmit_scheme_histograms.py and
presubmit_bad_message_reasons.py
My attempt at a CQ dry-run to see if PRESUBMIT_test.py is run is here:
https://codereview.chromium.org/2852503002/

[Ilya Sherman, 2017-04-27 22:28:09]

Okay, I've looked through the files in detail now.  The changes look great --
thanks for the fix!  LGTM % nits:

https://codereview.chromium.org/2841823007/diff/20001/tools/metrics/histogram...
File tools/metrics/histograms/update_histogram_enum.py (right):

https://codereview.chromium.org/2841823007/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/update_histogram_enum.py:202: A string with presubmit
failure description, or None (of no failures).
nit: s/of/if

https://codereview.chromium.org/2841823007/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/update_histogram_enum.py:209: return ('%s enum has been
updated and there exists '
nit: s/exists/exist

[Łukasz Anforowicz, 2017-04-27 22:35:41]

Thanks!  I'll push to CQ in a minute.

https://codereview.chromium.org/2841823007/diff/20001/tools/metrics/histogram...
File tools/metrics/histograms/update_histogram_enum.py (right):

https://codereview.chromium.org/2841823007/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/update_histogram_enum.py:202: A string with presubmit
failure description, or None (of no failures).
On 2017/04/27 22:28:09, Ilya Sherman wrote:
> nit: s/of/if

Done.

https://codereview.chromium.org/2841823007/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/update_histogram_enum.py:209: return ('%s enum has been
updated and there exists '
On 2017/04/27 22:28:09, Ilya Sherman wrote:
> nit: s/exists/exist

Done.

[Łukasz Anforowicz, 2017-04-27 22:35:46]

The CQ bit was checked by lukasza@chromium.org

[Łukasz Anforowicz, 2017-04-27 22:35:46]

The patchset sent to the CQ was uploaded after l-g-t-m from
isherman@chromium.org
Link to the patchset: https://codereview.chromium.org/2841823007/#ps40001
(title: "Fixed typos / nits in comments and error strings.")

[commit-bot: I haz the power, 2017-04-27 22:36:43]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Łukasz Anforowicz, 2017-04-27 23:27:57]

lukasza@chromium.org changed reviewers:
+ bokan@chromium.org, nick@chromium.org

[Łukasz Anforowicz, 2017-04-27 23:27:58]

+nick@ for content/browser/PRESUBMIT.py
+bokan@ for third_party/WebKit/Source/core/frame/PRESUBMIT.py

[ncarter (slow), 2017-04-27 23:34:25]

lgtm

[ncarter (slow), 2017-04-27 23:35:52]

https://codereview.chromium.org/2841823007/diff/40001/content/browser/PRESUBM...
File content/browser/PRESUBMIT.py (right):

https://codereview.chromium.org/2841823007/diff/40001/content/browser/PRESUBM...
content/browser/PRESUBMIT.py:17: # Setup sys.path so that we can call histrogram
code
"histrogram" is a typo

[Łukasz Anforowicz, 2017-04-27 23:52:41]

Patchset #4 (id:60001) has been deleted

[Łukasz Anforowicz, 2017-04-27 23:54:26]

Patchset #4 (id:80001) has been deleted

[Łukasz Anforowicz, 2017-04-27 23:57:07]

Description was changed from

==========
Fix presubmit_scheme_histograms.py and presubmit_bad_message_reasons.py

This CL fixes presubmit_scheme_histograms.py and
presubmit_bad_message_reasons.py which after r462591 would always report
presubmit errors (because it started returning a tuple from HistogramNeedsUpdate
which always coerces to True in the broken presubmit scripts).

After r462591 HistogramNeedsUpdate effectively checks all
histogram-enum-related presubmits (both for updating histograms.xml and
for detecting duplicate values).  Therefore the current CL renames this
function to CheckPresubmitErrors and makes it return a presubmit error
(rather than a tuple of (needs_updating, duplicates)).  This refactoring
has a nice side benefit - after this CL, all presubmits going through
CheckPresubmitErrors will check for duplicate values (r462591 only added
this check for UseCounter histograms).

This CL also tweaks content/browser/PRESUBMIT.py so that it applies
*both* to upload-time and commit-time checks (without this change
|git cl presubmit| would not hit the checks related to
content/browser/bad_message.h - they would only be hit when running
|git cl presubmit --upload|).

Manual testing done:
- Make changes in content/browser/bad_message.h,
  chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc,
  third_party/WebKit/Source/code/frame
- Run git cl presubmit and verify that the presubmit
  checks detect 1) duplicates and 2) the need to update histograms.xml

BUG=577772
TEST=See "Manual testing done" above.
==========

to

==========
Fix presubmit_scheme_histograms.py and presubmit_bad_message_reasons.py

This CL fixes presubmit_scheme_histograms.py and
presubmit_bad_message_reasons.py which after r462591 would always report
presubmit errors (because it started returning a tuple from HistogramNeedsUpdate
which always coerces to True in the broken presubmit scripts).

After r462591 HistogramNeedsUpdate effectively checks all
histogram-enum-related presubmits (both for updating histograms.xml and
for detecting duplicate values).  Therefore the current CL renames this
function to CheckPresubmitErrors and makes it return a presubmit error
(rather than a tuple of (needs_updating, duplicates)).  This refactoring
has a nice side benefit - after this CL, all presubmits going through
CheckPresubmitErrors will check for duplicate values (r462591 only added
this check for UseCounter histograms).

This CL also tweaks content/browser/PRESUBMIT.py so that it applies
*both* to upload-time and commit-time checks (without this change
|git cl presubmit| would not hit the checks related to
content/browser/bad_message.h - they would only be hit when running
|git cl presubmit --upload|).

Manual testing done:
- Make changes in content/browser/bad_message.h,
  chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc,
  third_party/WebKit/Source/code/frame
- Run git cl presubmit and verify that the presubmit
  checks detect 1) duplicates and 2) the need to update histograms.xml

BUG=577772
TEST=See "Manual testing done" above.
TBR=sky@chromium.org, jochen@chromium.org, rdevlin.cronin@chromium.org
==========

[Łukasz Anforowicz, 2017-04-27 23:59:04]

lukasza@chromium.org changed reviewers:
+ jochen@chromium.org, rdevlin.cronin@chromium.org, sky@chromium.org

[Łukasz Anforowicz, 2017-04-27 23:59:04]

+sky@chromium.org, jochen@chromium.org, rdevlin.cronin@chromium.org - FYI, I am
TBR-ing fixing of a small typo in a comment in //chrome, //components,
//extensions.

https://codereview.chromium.org/2841823007/diff/40001/content/browser/PRESUBM...
File content/browser/PRESUBMIT.py (right):

https://codereview.chromium.org/2841823007/diff/40001/content/browser/PRESUBM...
content/browser/PRESUBMIT.py:17: # Setup sys.path so that we can call histrogram
code
On 2017/04/27 23:35:52, ncarter wrote:
> "histrogram" is a typo

Done (in 5 cases where this typo got copy&pasted :-P ).

[bokan, 2017-04-28 12:49:51]

Source/core lgtm

[Łukasz Anforowicz, 2017-04-28 14:10:12]

The CQ bit was checked by lukasza@chromium.org

[Łukasz Anforowicz, 2017-04-28 14:10:12]

The patchset sent to the CQ was uploaded after l-g-t-m from nick@chromium.org,
isherman@chromium.org
Link to the patchset: https://codereview.chromium.org/2841823007/#ps100001
(title: "Fix an old typo in _RunHistogramChecks.")

[commit-bot: I haz the power, 2017-04-28 14:10:59]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-28 15:24:01]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1493388612036870,
"parent_rev": "de0f39d2a625433e60b5e22e1d2d3803f3d219e5", "commit_rev":
"14d9a1a8cade0d1b9878c4bda0bbc8417914893f"}

[commit-bot: I haz the power, 2017-04-28 15:24:10]

Description was changed from

==========
Fix presubmit_scheme_histograms.py and presubmit_bad_message_reasons.py

This CL fixes presubmit_scheme_histograms.py and
presubmit_bad_message_reasons.py which after r462591 would always report
presubmit errors (because it started returning a tuple from HistogramNeedsUpdate
which always coerces to True in the broken presubmit scripts).

After r462591 HistogramNeedsUpdate effectively checks all
histogram-enum-related presubmits (both for updating histograms.xml and
for detecting duplicate values).  Therefore the current CL renames this
function to CheckPresubmitErrors and makes it return a presubmit error
(rather than a tuple of (needs_updating, duplicates)).  This refactoring
has a nice side benefit - after this CL, all presubmits going through
CheckPresubmitErrors will check for duplicate values (r462591 only added
this check for UseCounter histograms).

This CL also tweaks content/browser/PRESUBMIT.py so that it applies
*both* to upload-time and commit-time checks (without this change
|git cl presubmit| would not hit the checks related to
content/browser/bad_message.h - they would only be hit when running
|git cl presubmit --upload|).

Manual testing done:
- Make changes in content/browser/bad_message.h,
  chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc,
  third_party/WebKit/Source/code/frame
- Run git cl presubmit and verify that the presubmit
  checks detect 1) duplicates and 2) the need to update histograms.xml

BUG=577772
TEST=See "Manual testing done" above.
TBR=sky@chromium.org, jochen@chromium.org, rdevlin.cronin@chromium.org
==========

to

==========
Fix presubmit_scheme_histograms.py and presubmit_bad_message_reasons.py

This CL fixes presubmit_scheme_histograms.py and
presubmit_bad_message_reasons.py which after r462591 would always report
presubmit errors (because it started returning a tuple from HistogramNeedsUpdate
which always coerces to True in the broken presubmit scripts).

After r462591 HistogramNeedsUpdate effectively checks all
histogram-enum-related presubmits (both for updating histograms.xml and
for detecting duplicate values).  Therefore the current CL renames this
function to CheckPresubmitErrors and makes it return a presubmit error
(rather than a tuple of (needs_updating, duplicates)).  This refactoring
has a nice side benefit - after this CL, all presubmits going through
CheckPresubmitErrors will check for duplicate values (r462591 only added
this check for UseCounter histograms).

This CL also tweaks content/browser/PRESUBMIT.py so that it applies
*both* to upload-time and commit-time checks (without this change
|git cl presubmit| would not hit the checks related to
content/browser/bad_message.h - they would only be hit when running
|git cl presubmit --upload|).

Manual testing done:
- Make changes in content/browser/bad_message.h,
  chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc,
  third_party/WebKit/Source/code/frame
- Run git cl presubmit and verify that the presubmit
  checks detect 1) duplicates and 2) the need to update histograms.xml

BUG=577772
TEST=See "Manual testing done" above.
TBR=sky@chromium.org, jochen@chromium.org, rdevlin.cronin@chromium.org

Review-Url: https://codereview.chromium.org/2841823007
Cr-Commit-Position: refs/heads/master@{#468003}
Committed:
https://chromium.googlesource.com/chromium/src/+/14d9a1a8cade0d1b9878c4bda0bb...
==========

[commit-bot: I haz the power, 2017-04-28 15:24:12]

Committed patchset #4 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/14d9a1a8cade0d1b9878c4bda0bb...