Looks like it's a false positive - it's inside a comment.

Thanks. find_histograms_unmapped.h removes comments from inside macros, but not from the rest of the file.

Removing comments from the whole file is tricky because I need to avoid messing up the line-number information. I will think about whether there's a clean way to do it. I suspect this is the only example, in which case it may not be worth fixing.

Okay, there's another example:  issue 643550 .

I think that means there is sufficient reason to fix this.

While correcting this issue I found two more: DiskCache.2.MyExperiment_530 and DiskCache.2.MyName. I have updated  issue 643561  to indicate that these don't need to be fixed.

Hey, thanks for the CL. Thinking about this - we still don't really want histograms to be mentioned in comments that don't exist anymore either, so if the script gave a warning that had someone update a comment to the relevant histogram - that seems like a plus to me. If comments are referencing old histograms that seems like a documentation bug.

On the other hand, the script isn't intended to fix obsolete comments, so I'm also sympathetic to removing it too.

My intuition is still a bit to keep warning on these so they get fixed... what do you guys think?

The few cases I think we've seen were not meant to refer to real histograms - it was either ones that haven't been logged ever or used as an example.

I think if a comment is genuinely trying to refer to an existing histogram, it wouldn't usually surround its name with a macro.

So I support ignoring these from the find_unmapped script. The point of it is to find things that are being logged but don't exist in XML - and comments are outside of that scope.

What asvitkine said. I think it makes sense for people to use nonexistent names when writing examples in comments. It probably reduces confusion.

I like to minimise false positives because it makes it easier to act on the output.

I might even go as far as outputting a warning when run without the --extra_histograms-file option, on the grounds that the internal histograms file hasn't been supplied and so the output is not reliable.

Ok, sounds good. Thanks!

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/edc98c959010e4dcbe85322c6fe4900af7e9a027

commit edc98c959010e4dcbe85322c6fe4900af7e9a027
Author: ricea <ricea@chromium.org>
Date: Tue Sep 20 02:16:59 2016

Ignore commented-out histograms in find_unmapped_histograms.py

The source code currently contains 4 histograms that are commented out
for documentation purposes. These should not be treated as unmapped
histograms.

Change the behaviour of the script to remove comments from the whole
text of the source file before looking for histograms. Newlines in
comments are preserved so that line number information is not lost.

Before/after output diff:

@@ -112,8 +112,6 @@
 INFO: components/data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client.cc:115: DataReductionProxy.ClientConfig.AuthExpiredSessionKey - 0xbc3e8360e9a88fa3
-INFO: net/disk_cache/blockfile/histogram_macros.h:88: DiskCache.2.MyExperiment_530 - 0x96d64ee0c844abe8
-INFO: net/disk_cache/blockfile/histogram_macros.h:87: DiskCache.2.MyName - 0x375b236e75b046a3
 INFO: net/disk_cache/blockfile/block_files.cc:412: DiskCache.BlockLoad_0 - 0xc71c5b8eeddcbc13
@@ -196,7 +194,6 @@
 INFO: chrome/browser/android/history_report/usage_reports_buffer_backend.cc:42: LevelDB.Open.UsageReportsBufferBackend - 0x3c3398b51e4f2c94
-INFO: content/browser/leveldb_wrapper_impl.cc:313: LevelDBWrapper.CommitDelay - 0x7b3688b99064caf4
 INFO: chrome/browser/local_discovery/service_discovery_client_mac.mm:228: LocalDiscovery.MacBrowseCallTimes - 0x68d6ea27ac6cc254
@@ -204,7 +201,6 @@
 INFO: components/storage_monitor/media_storage_util.cc:235: MediaDeviceNotifications.DeviceInfo - 0x55f5f86156ae430d
-INFO: base/metrics/field_trial.h:27: Memory.RendererTotal - 0x2e84018fc64715ff
 INFO: chrome/browser/chromeos/preferences.cc:578: Mouse.PrimaryButtonRight.Changed - 0x102a33e3a331c943

BUG= 642625 

Review-Url: https://codereview.chromium.org/2343443003
Cr-Commit-Position: refs/heads/master@{#419652}

[modify] https://crrev.com/edc98c959010e4dcbe85322c6fe4900af7e9a027/tools/metrics/histograms/find_unmapped_histograms.py