Collect log messages from stability files

components/browser_watcher/postmortem_report_collector.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/browser_watcher/postmortem_report_collector.h"
 
 #include <utility>
 
 #include "base/debug/activity_analyzer.h"
 #include "base/files/file_enumerator.h"
@@ skipping 149 matching lines @@
   DCHECK_NE(nullptr, report);
   report->reset();
 
   // Create a global analyzer.
   std::unique_ptr<GlobalActivityAnalyzer> global_analyzer =
       GlobalActivityAnalyzer::CreateWithFile(debug_state_file);
   if (!global_analyzer)
     return ANALYZER_CREATION_FAILED;
 
   // Early exit if there is no data.
+  std::vector<std::string> log_messages = global_analyzer->GetLogMessages();
   ThreadActivityAnalyzer* thread_analyzer = global_analyzer->GetFirstAnalyzer();
-  if (!thread_analyzer) {
+  if (log_messages.empty() && !thread_analyzer) {

[bcwhite, 2016/12/09 00:34:22]

It's possible to have log messages even on a clean exit.  Right now it's limited
to FATAL messages but that's not part of the definition.

[manzagop (departed), 2016/12/09 22:41:09]

Right: if we stop deleting the file on clean exit, then we'll need to add some
logic to know if we should send a report (ie if the exit was clean or not).

Do you feel a change is needed?

[bcwhite, 2016/12/09 23:17:06]

log_messages being empty or non-empty doesn't consistently indicate crash-exit
or clean-exit.

I don't think you should check it at all when deciding whether it was a clean
exit.

[manzagop (departed), 2016/12/12 15:59:32]

I've removed the mention of clean exit: this should only be about whether the
file is empty. 

Our current decision of clean vs not is whether the file exists, since we open
it for deletion on clean exit. You're right that we *also* don't send a report
if it's fully empty.

I've also updated the comment where this function is called to state the current
behavior: existing file that is non-empty = send a report.

[bcwhite, 2016/12/13 13:22:55]

So to be clear...  You _do_ want to send a report if _any_ log messages were
collected, keeping in mind that these messages may not be "fatal" messages.

Though this is currently the case but may not be so in the future which would
mean, should it change, that a report would get sent every time.

[manzagop (departed), 2016/12/13 14:01:39]

That's right: short term we rely on MarkStabilityFileForDeletion to know if the
exit was unclean:
https://cs.chromium.org/chromium/src/components/browser_watcher/stability_deb...

Longer term we'll get rid of MarkStabilityFileForDeletion and cook up some kind
of analysis for which presence of logs won't be an indicator of unclean
shutdown.

As we found out, having thread activity data also isn't an indicator of unclean
shutdown.

     // No data. This case happens in the case of a clean exit.
     return DEBUG_FILE_NO_DATA;
   }
 
-  // Iterate through the thread analyzers, fleshing out the report.
+  // Create the report, then flesh it out.
   report->reset(new StabilityReport());
+
+  // Collect log messages.
+  for (const std::string& message : log_messages) {
+    (*report)->add_log_messages(message);
+  }
+
+  // Collect thread activity data.
   // Note: a single process is instrumented.
   ProcessState* process_state = (*report)->add_process_states();
-
   for (; thread_analyzer != nullptr;
        thread_analyzer = global_analyzer->GetNextAnalyzer()) {
     // Only valid analyzers are expected per contract of GetFirstAnalyzer /
     // GetNextAnalyzer.
     DCHECK(thread_analyzer->IsValid());
 
     if (!process_state->has_process_id()) {
       process_state->set_process_id(
           thread_analyzer->activity_snapshot().process_id);
     }
@@ skipping 64 matching lines @@
 #if defined(ARCH_CPU_X86)
   minidump_info.platform = std::string("Win32");
 #elif defined(ARCH_CPU_X86_64)
   minidump_info.platform = std::string("Win64");
 #endif
 
   return WritePostmortemDump(minidump_file, report, minidump_info);
 }
 
 }  // namespace browser_watcher