Normalizing source file names before calculating hash

chrome/browser/metrics/metrics_log.cc

Index: chrome/browser/metrics/metrics_log.cc
diff --git a/chrome/browser/metrics/metrics_log.cc b/chrome/browser/metrics/metrics_log.cc
index f9721f4a5c02158cb7821e1970d187d1c4e01631..87b060a77bbe38137a84fae6f223378306cdf141 100644
--- a/chrome/browser/metrics/metrics_log.cc
+++ b/chrome/browser/metrics/metrics_log.cc
@@ -273,6 +273,14 @@ std::string MapThreadName(const std::string& thread_name) {
   return thread_name.substr(0, i) + '*';
 }
 
+// Normalizes a source filename (which is platform- and build-method-dependent)
+// by extracting the last component of the full file name.
+// Example: "c:\b\build\slave\win\build\src\chrome\app\chrome_main.cc" =>
+// "chrome_main.cc".
+std::string NormalizeFileName(const std::string& file_name) {
+  return file_name.substr(file_name.find_last_of("\\/") + 1);

[jar (doing other things), 2014/04/01 18:46:12]

I think the code works... but it probably shouldn't be trusted.

When there is no character found, you get back string::npos.

It turns out that this is usually -1.  Hence you are counting on that, and
adding one, to get 0.  The function could instead have been defined to return
-1, but that is not the API promize.

I don't believe you should count on that use of -1.

size_t offset = file_name.find_last_of("\\/");
offset = (offset == string::npos) ? 0 : offset + 1;
...

[vadimt, 2014/04/01 18:59:01]

It's not a problem for me to change the code, but "-1" is official:
http://www.cplusplus.com/reference/string/string/npos/

[jar (doing other things), 2014/04/01 19:16:17]

OK... I can live with that.  I hadn't seen it.

Thanks.

[Ilya Sherman, 2014/04/02 06:56:15]

I agree with Jim that relying on (string::npos + 1 == 0) in this way is a little
too likely to confuse people, even if it is guaranteed to be correct.  I'd
prefer to separate out the case where no file path component is found from the
case where one is.  Alternately, we could just DCHECK that one of the characters
exists in the file_name, since that should always be the case.

[vadimt, 2014/04/02 17:50:25]

Done.

+}
+
 void WriteProfilerData(const ProcessDataSnapshot& profiler_data,
                        int process_type,
                        ProfilerEventProto* performance_profile) {
@@ -287,7 +295,7 @@ void WriteProfilerData(const ProcessDataSnapshot& profiler_data,
     tracked_object->set_exec_thread_name_hash(
         MetricsLogBase::Hash(MapThreadName(it->death_thread_name)));
     tracked_object->set_source_file_name_hash(
-        MetricsLogBase::Hash(it->birth.location.file_name));
+        MetricsLogBase::Hash(NormalizeFileName(it->birth.location.file_name)));
     tracked_object->set_source_function_name_hash(
         MetricsLogBase::Hash(it->birth.location.function_name));
     tracked_object->set_source_line_number(it->birth.location.line_number);