Histograms.xml python script housekeeping

Histograms.xml python script housekeeping:
 - Add an autogeneration script for the bad_message.h files.
 - add histogram_paths.py, to help deal with paths consistently.
 - Specify filenames relative to src/ in all scripts. This makes
   things more greppable, and eliminates a problem where the os-
   specific path separator would seep into the histograms.xml
   if you ran the scripts on Windows.
 - Make the scripts run properly from any directory (previously they
   had to be run from the histograms directory).
 - Update some docstring comments (google python style)

Changes outside of tools\metrics:
 - From bad_message.h files, mention the autogen script.

And finally:
 - Run all the scripts, which revealed some neglected enum values.

TEST=Run tools/metrics/.../*.py on Windows
BUG=338781, 485227
Committed: https://crrev.com/0b08cdaeab9210611012014b9ba7ac93d7efb5c7
Cr-Commit-Position: refs/heads/master@{#332724}

[ncarter (slow), 2015-05-26 23:15:36]

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

[ncarter (slow), 2015-05-26 23:15:38]

Hi isherman, Please review! Note that this CL depends on two unlanded issues:
issue 1145013004 (which adds the new bad_message.h files) and issue 1153503003
(split off from this; it fixes up an ordering mistake with one of the extensions
permissions enums).

[Ilya Sherman, 2015-05-27 00:12:25]

Thanks for the cleanup!

https://codereview.chromium.org/1143323006/diff/60001/tools/metrics/histogram...
File tools/metrics/histograms/histograms_path.py (right):

https://codereview.chromium.org/1143323006/diff/60001/tools/metrics/histogram...
tools/metrics/histograms/histograms_path.py:1: # Copyright 2013 The Chromium
Authors. All rights reserved.
nit: 2015

https://codereview.chromium.org/1143323006/diff/60001/tools/metrics/histogram...
tools/metrics/histograms/histograms_path.py:19: def
GetInputFile(src_relative_file_path):
Is it worth providing this at a higher level, so that the actions and rappor
scripts can also be cleaned up to use this?

https://codereview.chromium.org/1143323006/diff/60001/tools/metrics/histogram...
tools/metrics/histograms/histograms_path.py:22: return os.path.join(*(depth +
src_relative_file_path.split('/')))
Out of curiousity, what does the "*" operator do?

https://codereview.chromium.org/1143323006/diff/60001/tools/metrics/histogram...
File tools/metrics/histograms/update_editor_commands.py (right):

https://codereview.chromium.org/1143323006/diff/60001/tools/metrics/histogram...
tools/metrics/histograms/update_editor_commands.py:19: import histograms_path
nit: Alphabetize?

https://codereview.chromium.org/1143323006/diff/60001/tools/metrics/histogram...
File tools/metrics/histograms/update_use_counter_feature_enum.py (left):

https://codereview.chromium.org/1143323006/diff/60001/tools/metrics/histogram...
tools/metrics/histograms/update_use_counter_feature_enum.py:16:
sys.path.append(os.path.join(os.path.dirname(__file__), '..', 'common'))
Why is this line no longer needed?  I believe that it was previously added to
allow the script to be run from any directory.

[ncarter (slow), 2015-05-29 23:01:11]

PTAL. I had to sync (sorry) to make sure histograms.xml was the newest so you
may be best off with a fresh review instead of comparing to patchset 5.

Also, I realized there were a couple places where I had the style guide wrong so
I've reverted a few of my fixups. In particular, the Chrome python style guide
apparently allows imports of functions, not just modules.

https://codereview.chromium.org/1143323006/diff/60001/tools/metrics/histogram...
File tools/metrics/histograms/histograms_path.py (right):

https://codereview.chromium.org/1143323006/diff/60001/tools/metrics/histogram...
tools/metrics/histograms/histograms_path.py:1: # Copyright 2013 The Chromium
Authors. All rights reserved.
On 2015/05/27 00:12:24, Ilya Sherman wrote:
> nit: 2015

Done.

https://codereview.chromium.org/1143323006/diff/60001/tools/metrics/histogram...
tools/metrics/histograms/histograms_path.py:19: def
GetInputFile(src_relative_file_path):
On 2015/05/27 00:12:24, Ilya Sherman wrote:
> Is it worth providing this at a higher level, so that the actions and rappor
> scripts can also be cleaned up to use this?

I moved it to common (as path_util.py). I also added __init__.py files to both
common and histograms, so that the import statements give a little more of a
hint where these files are coming from.

https://codereview.chromium.org/1143323006/diff/60001/tools/metrics/histogram...
tools/metrics/histograms/histograms_path.py:22: return os.path.join(*(depth +
src_relative_file_path.split('/')))
On 2015/05/27 00:12:24, Ilya Sherman wrote:
> Out of curiousity, what does the "*" operator do?

https://docs.python.org/2/tutorial/controlflow.html#unpacking-argument-lists

Basically, the join() function accepts a variable number of positional
arguments, but here we have a list of strings to be joined. "*" lets you unpack
the list so that each list element is passed an argument.

https://codereview.chromium.org/1143323006/diff/60001/tools/metrics/histogram...
File tools/metrics/histograms/update_editor_commands.py (right):

https://codereview.chromium.org/1143323006/diff/60001/tools/metrics/histogram...
tools/metrics/histograms/update_editor_commands.py:19: import histograms_path
On 2015/05/27 00:12:24, Ilya Sherman wrote:
> nit: Alphabetize?

Done.

https://codereview.chromium.org/1143323006/diff/60001/tools/metrics/histogram...
File tools/metrics/histograms/update_use_counter_feature_enum.py (left):

https://codereview.chromium.org/1143323006/diff/60001/tools/metrics/histogram...
tools/metrics/histograms/update_use_counter_feature_enum.py:16:
sys.path.append(os.path.join(os.path.dirname(__file__), '..', 'common'))
On 2015/05/27 00:12:24, Ilya Sherman wrote:
> Why is this line no longer needed?  I believe that it was previously added to
> allow the script to be run from any directory.

This statement would allow a script in tools/metrics/histogram to use the
"import" keyword to pull in a script from tools/metrics/common as a module. It
works by modifying the import search path. Other files in this directory did
this to pull in diff_util from common. It looks like it was just mistakenly
copied to this file, because no module from common is actually imported here.

In the new patch set I've changed how these scripts handle imports (they adjust
the search path to include 'tools/metrics' and then import relative from there).

[Ilya Sherman, 2015-05-31 00:41:03]

LGTM, thanks.

https://codereview.chromium.org/1143323006/diff/120001/tools/metrics/histogra...
File tools/metrics/histograms/find_unmapped_histograms.py (right):

https://codereview.chromium.org/1143323006/diff/120001/tools/metrics/histogra...
tools/metrics/histograms/find_unmapped_histograms.py:148: if not histogram or
histogram[0] != '"' or histogram[-1] != '"':
Why did you need to add "not histogram" to this condition?  I'm not following
what change caused the need for that.

[ncarter (slow), 2015-06-01 17:04:13]

https://codereview.chromium.org/1143323006/diff/120001/tools/metrics/histogra...
File tools/metrics/histograms/find_unmapped_histograms.py (right):

https://codereview.chromium.org/1143323006/diff/120001/tools/metrics/histogra...
tools/metrics/histograms/find_unmapped_histograms.py:148: if not histogram or
histogram[0] != '"' or histogram[-1] != '"':
On 2015/05/31 00:41:02, Ilya Sherman wrote:
> Why did you need to add "not histogram" to this condition?  I'm not following
> what change caused the need for that.

This comment caused this script to die because we got a null match in
HISTOGRAM_REGEX (the pattern matched but there was nothing in the capture group)
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/net...

The other way to fix this would be to change the * inside the first-parameter
capture group for HISTOGRAM_REGEX to be a +. [I considered trying to filter out
comment lines using lookaround but my regex foo isn't strong enough and it
seemed like a yak shave]

Let me know which you prefer.

[chromium-reviews, 2015-06-01 18:45:11]

Can we instead ignore comment lines?

On Mon, Jun 1, 2015 at 1:04 PM, <nick@chromium.org> wrote:

>
>
>
https://codereview.chromium.org/1143323006/diff/120001/tools/metrics/histogra...
> File tools/metrics/histograms/find_unmapped_histograms.py (right):
>
>
>
https://codereview.chromium.org/1143323006/diff/120001/tools/metrics/histogra...
> tools/metrics/histograms/find_unmapped_histograms.py:148: if not
> histogram or histogram[0] != '"' or histogram[-1] != '"':
> On 2015/05/31 00:41:02, Ilya Sherman wrote:
>
>> Why did you need to add "not histogram" to this condition?  I'm not
>>
> following
>
>> what change caused the need for that.
>>
>
> This comment caused this script to die because we got a null match in
> HISTOGRAM_REGEX (the pattern matched but there was nothing in the
> capture group)
>
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/net...
>
> The other way to fix this would be to change the * inside the
> first-parameter capture group for HISTOGRAM_REGEX to be a +. [I
> considered trying to filter out comment lines using lookaround but my
> regex foo isn't strong enough and it seemed like a yak shave]
>
> Let me know which you prefer.
>
> https://codereview.chromium.org/1143323006/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Ilya Sherman, 2015-06-01 20:25:45]

https://codereview.chromium.org/1143323006/diff/120001/tools/metrics/histogra...
File tools/metrics/histograms/find_unmapped_histograms.py (right):

https://codereview.chromium.org/1143323006/diff/120001/tools/metrics/histogra...
tools/metrics/histograms/find_unmapped_histograms.py:148: if not histogram or
histogram[0] != '"' or histogram[-1] != '"':
On 2015/06/01 17:04:12, ncarter wrote:
> On 2015/05/31 00:41:02, Ilya Sherman wrote:
> > Why did you need to add "not histogram" to this condition?  I'm not
following
> > what change caused the need for that.
> 
> This comment caused this script to die because we got a null match in
> HISTOGRAM_REGEX (the pattern matched but there was nothing in the capture
group)
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/net...
> 
> The other way to fix this would be to change the * inside the first-parameter
> capture group for HISTOGRAM_REGEX to be a +. [I considered trying to filter
out
> comment lines using lookaround but my regex foo isn't strong enough and it
> seemed like a yak shave]
> 
> Let me know which you prefer.

Okay, thanks for the explanation.  I was mostly just curious, and am fine with
either solution.  It sounds like Alexei would prefer to ignore comment lines
entirely.  Alexei, could you possibly help craft a regex that does this?  I'm
not sure offhand how to do that, and it sounds like Nick is also unsure.

[chromium-reviews, 2015-06-01 20:47:11]

I don't feel super strongly. I do prefer to ignore comments if we can, but
am OK if we don't want to spend the effort trying to craft a regex for it.

On Mon, Jun 1, 2015 at 4:25 PM, <isherman@chromium.org> wrote:

>
>
>
https://codereview.chromium.org/1143323006/diff/120001/tools/metrics/histogra...
> File tools/metrics/histograms/find_unmapped_histograms.py (right):
>
>
>
https://codereview.chromium.org/1143323006/diff/120001/tools/metrics/histogra...
> tools/metrics/histograms/find_unmapped_histograms.py:148: if not
> histogram or histogram[0] != '"' or histogram[-1] != '"':
> On 2015/06/01 17:04:12, ncarter wrote:
>
>> On 2015/05/31 00:41:02, Ilya Sherman wrote:
>> > Why did you need to add "not histogram" to this condition?  I'm not
>>
> following
>
>> > what change caused the need for that.
>>
>
>  This comment caused this script to die because we got a null match in
>> HISTOGRAM_REGEX (the pattern matched but there was nothing in the
>>
> capture group)
>
>
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/net...
>
>  The other way to fix this would be to change the * inside the
>>
> first-parameter
>
>> capture group for HISTOGRAM_REGEX to be a +. [I considered trying to
>>
> filter out
>
>> comment lines using lookaround but my regex foo isn't strong enough
>>
> and it
>
>> seemed like a yak shave]
>>
>
>  Let me know which you prefer.
>>
>
> Okay, thanks for the explanation.  I was mostly just curious, and am
> fine with either solution.  It sounds like Alexei would prefer to ignore
> comment lines entirely.  Alexei, could you possibly help craft a regex
> that does this?  I'm not sure offhand how to do that, and it sounds like
> Nick is also unsure.
>
> https://codereview.chromium.org/1143323006/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[ncarter (slow), 2015-06-01 21:36:34]

On 2015/06/01 20:47:11, chromium-reviews wrote:
> I don't feel super strongly. I do prefer to ignore comments if we can, but
> am OK if we don't want to spend the effort trying to craft a regex for it.
> 
> On Mon, Jun 1, 2015 at 4:25 PM, <mailto:isherman@chromium.org> wrote:
> 
> >
> >
> >
>
https://codereview.chromium.org/1143323006/diff/120001/tools/metrics/histogra...
> > File tools/metrics/histograms/find_unmapped_histograms.py (right):
> >
> >
> >
>
https://codereview.chromium.org/1143323006/diff/120001/tools/metrics/histogra...
> > tools/metrics/histograms/find_unmapped_histograms.py:148: if not
> > histogram or histogram[0] != '"' or histogram[-1] != '"':
> > On 2015/06/01 17:04:12, ncarter wrote:
> >
> >> On 2015/05/31 00:41:02, Ilya Sherman wrote:
> >> > Why did you need to add "not histogram" to this condition?  I'm not
> >>
> > following
> >
> >> > what change caused the need for that.
> >>
> >
> >  This comment caused this script to die because we got a null match in
> >> HISTOGRAM_REGEX (the pattern matched but there was nothing in the
> >>
> > capture group)
> >
> >
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/net...
> >
> >  The other way to fix this would be to change the * inside the
> >>
> > first-parameter
> >
> >> capture group for HISTOGRAM_REGEX to be a +. [I considered trying to
> >>
> > filter out
> >
> >> comment lines using lookaround but my regex foo isn't strong enough
> >>
> > and it
> >
> >> seemed like a yak shave]
> >>
> >
> >  Let me know which you prefer.
> >>
> >
> > Okay, thanks for the explanation.  I was mostly just curious, and am
> > fine with either solution.  It sounds like Alexei would prefer to ignore
> > comment lines entirely.  Alexei, could you possibly help craft a regex
> > that does this?  I'm not sure offhand how to do that, and it sounds like
> > Nick is also unsure.
> >
> > https://codereview.chromium.org/1143323006/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

I think we can do it with a second regex or something, but I'll send that out as
a follow up change; right now this CL is blocking other stuff I want to land.

[ncarter (slow), 2015-06-01 21:37:29]

nick@chromium.org changed reviewers:
+ jam@chromium.org

[ncarter (slow), 2015-06-01 21:37:30]

jam: need approval for the changes outside of tools/metrics

[chromium-reviews, 2015-06-01 21:38:29]

SG, you have Ilya's LGTM so should be good to go, I think. :)

On Mon, Jun 1, 2015 at 5:36 PM, <nick@chromium.org> wrote:

> On 2015/06/01 20:47:11, chromium-reviews wrote:
>
>> I don't feel super strongly. I do prefer to ignore comments if we can, but
>> am OK if we don't want to spend the effort trying to craft a regex for it.
>>
>
>  On Mon, Jun 1, 2015 at 4:25 PM, <mailto:isherman@chromium.org> wrote:
>>
>
>  >
>> >
>> >
>>
>
>
>
https://codereview.chromium.org/1143323006/diff/120001/tools/metrics/histogra...
>
>> > File tools/metrics/histograms/find_unmapped_histograms.py (right):
>> >
>> >
>> >
>>
>
>
>
https://codereview.chromium.org/1143323006/diff/120001/tools/metrics/histogra...
>
>> > tools/metrics/histograms/find_unmapped_histograms.py:148: if not
>> > histogram or histogram[0] != '"' or histogram[-1] != '"':
>> > On 2015/06/01 17:04:12, ncarter wrote:
>> >
>> >> On 2015/05/31 00:41:02, Ilya Sherman wrote:
>> >> > Why did you need to add "not histogram" to this condition?  I'm not
>> >>
>> > following
>> >
>> >> > what change caused the need for that.
>> >>
>> >
>> >  This comment caused this script to die because we got a null match in
>> >> HISTOGRAM_REGEX (the pattern matched but there was nothing in the
>> >>
>> > capture group)
>> >
>> >
>> >
>>
>
>
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/net...
>
>> >
>> >  The other way to fix this would be to change the * inside the
>> >>
>> > first-parameter
>> >
>> >> capture group for HISTOGRAM_REGEX to be a +. [I considered trying to
>> >>
>> > filter out
>> >
>> >> comment lines using lookaround but my regex foo isn't strong enough
>> >>
>> > and it
>> >
>> >> seemed like a yak shave]
>> >>
>> >
>> >  Let me know which you prefer.
>> >>
>> >
>> > Okay, thanks for the explanation.  I was mostly just curious, and am
>> > fine with either solution.  It sounds like Alexei would prefer to ignore
>> > comment lines entirely.  Alexei, could you possibly help craft a regex
>> > that does this?  I'm not sure offhand how to do that, and it sounds like
>> > Nick is also unsure.
>> >
>> > https://codereview.chromium.org/1143323006/
>> >
>>
>
>  To unsubscribe from this group and stop receiving emails from it, send an
>>
> email
>
>> to mailto:chromium-reviews+unsubscribe@chromium.org.
>>
>
> I think we can do it with a second regex or something, but I'll send that
> out as
> a follow up change; right now this CL is blocking other stuff I want to
> land.
>
> https://codereview.chromium.org/1143323006/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[jam, 2015-06-02 17:56:16]

On 2015/06/01 21:37:30, ncarter wrote:
> jam: need approval for the changes outside of tools/metrics

lgtm

[ncarter (slow), 2015-06-03 17:41:04]

The CQ bit was checked by nick@chromium.org

[commit-bot: I haz the power, 2015-06-03 17:41:39]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1143323006/120001

[commit-bot: I haz the power, 2015-06-03 17:49:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-06-03 17:49:44]

Try jobs failed on following builders:
  linux_chromium_chromeos_compile_dbg_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[ncarter (slow), 2015-06-03 20:15:31]

The CQ bit was checked by nick@chromium.org

[ncarter (slow), 2015-06-03 20:15:31]

The patchset sent to the CQ was uploaded after l-g-t-m from
isherman@chromium.org, jam@chromium.org
Link to the patchset: https://codereview.chromium.org/1143323006/#ps160001
(title: "Re run scripts")

[commit-bot: I haz the power, 2015-06-03 20:16:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1143323006/160001

[commit-bot: I haz the power, 2015-06-03 20:28:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-06-03 20:28:21]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[ncarter (slow), 2015-06-03 21:51:54]

isherman: please take another look (ps9 vs. ps10); I had to back out the
__init__.py / module import changes due to a conflict between
build/scripts/common/ and tools/metrics/common :(

[Ilya Sherman, 2015-06-03 21:56:26]

LGTM, thanks.

[ncarter (slow), 2015-06-03 22:05:53]

The CQ bit was checked by nick@chromium.org

[ncarter (slow), 2015-06-03 22:05:53]

The patchset sent to the CQ was uploaded after l-g-t-m from jam@chromium.org
Link to the patchset: https://codereview.chromium.org/1143323006/#ps180001
(title: "Fix imports (sadface)")

[commit-bot: I haz the power, 2015-06-03 22:07:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1143323006/180001

[commit-bot: I haz the power, 2015-06-03 23:27:53]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2015-06-03 23:28:54]

Patchset 10 (id:??) landed as
https://crrev.com/0b08cdaeab9210611012014b9ba7ac93d7efb5c7
Cr-Commit-Position: refs/heads/master@{#332724}