Collect log messages from stability files

Details:
- Collection was added in https://codereview.chromium.org/2544853003/
- log messages are added to the stability report's top level and not to the process level. We may want to revisit that if/when reports start containing multiple processes

BUG=620813

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

components/browser_watcher/dump_postmortem_minidump_main_win.cc

@@ -50,7 +50,7 @@ void PrintActivity(FILE* out,
   Indent(out, indent_level + 1);
   fprintf(out, "time: %lld\n", activity.time());
   switch (activity.type()) {
-    case browser_watcher::Activity::NONE:
+    case browser_watcher::Activity::UNKNOWN:
       break;
     case browser_watcher::Activity::ACT_TASK_RUN:
       Indent(out, indent_level + 1);

components/browser_watcher/postmortem_report_collector.cc

@@ -100,7 +100,8 @@ PostmortemReportCollector::CollectAndSubmit(
   // Note: the code below involves two notions of report: chrome internal state
   // reports and the crashpad reports they get wrapped into.
 
-  // Collect the data from the debug file to a proto.
+  // Collect the data from the debug file to a proto. Note: a non-empty report
+  // is interpreted here as an unclean exit.
   std::unique_ptr<StabilityReport> report_proto;
   CollectionStatus status = Collect(file, &report_proto);
   if (status != SUCCESS) {
@@ -167,17 +168,23 @@ PostmortemReportCollector::CollectionStatus PostmortemReportCollector::Collect(
     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) {
-    // No data. This case happens in the case of a clean exit.
+  if (log_messages.empty() && !thread_analyzer) {
     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 /

components/browser_watcher/postmortem_report_collector.h

@@ -72,6 +72,9 @@ class PostmortemReportCollector {
   FRIEND_TEST_ALL_PREFIXES(PostmortemReportCollectorTest, CollectRandomFile);
   FRIEND_TEST_ALL_PREFIXES(PostmortemReportCollectorCollectionTest,
                            CollectSuccess);
+  FRIEND_TEST_ALL_PREFIXES(
+      PostmortemReportCollectorCollectionFromGlobalTrackerTest,
+      LogCollection);
 
   // Virtual for unittesting.
   virtual std::vector<base::FilePath> GetDebugStateFilePaths(

components/browser_watcher/postmortem_report_collector_unittest.cc

@@ -340,6 +340,9 @@ const int kAnotherThreadId = 45;
 
 }  // namespace
 
+// Sets up a file backed thread tracker for direct access. A
+// GlobalActivityTracker is not created, meaning there is no risk of
+// the instrumentation interfering with the file's content.
 class PostmortemReportCollectorCollectionTest : public testing::Test {
  public:
   // Create a proper debug file.
@@ -484,4 +487,55 @@ TEST_F(PostmortemReportCollectorCollectionTest, CollectSuccess) {
   }
 }
 
+class PostmortemReportCollectorCollectionFromGlobalTrackerTest
+    : public testing::Test {
+ public:
+  const int kMemorySize = 1 << 20;  // 1MiB
+
+  PostmortemReportCollectorCollectionFromGlobalTrackerTest() {}
+  ~PostmortemReportCollectorCollectionFromGlobalTrackerTest() override {
+    GlobalActivityTracker* global_tracker = GlobalActivityTracker::Get();
+    if (global_tracker) {
+      global_tracker->ReleaseTrackerForCurrentThreadForTesting();
+      delete global_tracker;
+    }
+  }
+
+  void SetUp() override {
+    testing::Test::SetUp();
+
+    // Set up a debug file path.
+    ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
+    debug_file_path_ = temp_dir_.GetPath().AppendASCII("debug.pma");
+  }
+
+  const base::FilePath& debug_file_path() { return debug_file_path_; }
+
+ protected:
+  base::ScopedTempDir temp_dir_;
+  base::FilePath debug_file_path_;
+};
+
+TEST_F(PostmortemReportCollectorCollectionFromGlobalTrackerTest,
+       LogCollection) {
+  // Record some log messages.
+  GlobalActivityTracker::CreateWithFile(debug_file_path(), kMemorySize, 0ULL,
+                                        "", 3);
+  GlobalActivityTracker::Get()->RecordLogMessage("hello world");
+  GlobalActivityTracker::Get()->RecordLogMessage("foo bar");
+
+  // Collect the stability report.
+  PostmortemReportCollector collector(kProductName, kVersionNumber,
+                                      kChannelName);
+  std::unique_ptr<StabilityReport> report;
+  ASSERT_EQ(PostmortemReportCollector::SUCCESS,
+            collector.Collect(debug_file_path(), &report));
+  ASSERT_NE(nullptr, report);
+
+  // Validate the report's log content.
+  ASSERT_EQ(2, report->log_messages_size());
+  ASSERT_EQ("hello world", report->log_messages(0));
+  ASSERT_EQ("foo bar", report->log_messages(1));
+}
+
 }  // namespace browser_watcher

components/browser_watcher/stability_report.proto

@@ -10,10 +10,12 @@ package browser_watcher;
 
 // The state of the system on which Chrome is running (shutting down, battery
 // level, load, etc.).
+// Next id: 1
 message SystemState {
   // TODO(manzagop): flesh out.
 }
 
+// Next id: 8
 message CodeModule {
   // The base address of this code module as it was loaded by the process.
   optional int64 base_address = 1;
@@ -53,7 +55,7 @@ message CodeModule {
 // Next id: 9
 message Activity {
   enum Type {
-    NONE = 0;
+    UNKNOWN = 0;
     ACT_TASK_RUN = 1;
     ACT_LOCK_ACQUIRE = 2;
     ACT_EVENT_WAIT = 3;
@@ -120,6 +122,7 @@ message ProcessState {
 // A stability report contains information pertaining to the execution of a
 // single logical instance of a "chrome browser". It is comprised of information
 // about the system state and about the chrome browser's processes.
+// Next id: 4
 message StabilityReport {
   optional SystemState system_state = 1;
   // TODO(manzagop): revisit whether a single repeated field should contain all
@@ -128,4 +131,7 @@ message StabilityReport {
   //     times (e.g. start time), hierarchical relationships (e.g. parent pid),
   //     command line, etc.
   repeated ProcessState process_states = 2;
+  // TODO(manzagop): if/when reports contain multiple processes, attribute and
+  //     relocate these to their process (and perhaps thread).
+  repeated string log_messages = 3;
 }