Separate collection logic from the extraction of the report

components/browser_watcher/postmortem_report_collector.cc

Index: components/browser_watcher/postmortem_report_collector.cc
diff --git a/components/browser_watcher/postmortem_report_collector.cc b/components/browser_watcher/postmortem_report_collector.cc
index 837f0232974f6db15c8681b122e3198146e7e3fe..7d375c2762fa595a0453a066d05e953d31b82a8c 100644
--- a/components/browser_watcher/postmortem_report_collector.cc
+++ b/components/browser_watcher/postmortem_report_collector.cc
@@ -6,7 +6,6 @@
 
 #include <utility>
 
-#include "base/debug/activity_analyzer.h"
 #include "base/files/file_enumerator.h"
 #include "base/files/file_util.h"
 #include "base/logging.h"
@@ -17,139 +16,14 @@
 #include "base/strings/utf_string_conversions.h"
 #include "components/browser_watcher/postmortem_minidump_writer.h"
 #include "components/browser_watcher/stability_data_names.h"
-#include "components/variations/active_field_trials.h"
 #include "third_party/crashpad/crashpad/client/settings.h"
 #include "third_party/crashpad/crashpad/util/misc/uuid.h"
 
-using base::FilePath;
-
 namespace browser_watcher {
 
-using ActivitySnapshot = base::debug::ThreadActivityAnalyzer::Snapshot;
-using base::debug::ActivityUserData;
-using base::debug::GlobalActivityAnalyzer;
-using base::debug::GlobalActivityTracker;
-using base::debug::ThreadActivityAnalyzer;
+using base::FilePath;
 using crashpad::CrashReportDatabase;
 
-namespace {
-
-const char kFieldTrialKeyPrefix[] = "FieldTrial.";
-
-// Collects stability user data from the recorded format to the collected
-// format.
-void CollectUserData(
-    const ActivityUserData::Snapshot& recorded_map,
-    google::protobuf::Map<std::string, TypedValue>* collected_map,
-    StabilityReport* report) {
-  DCHECK(collected_map);
-
-  for (const auto& name_and_value : recorded_map) {
-    const std::string& key = name_and_value.first;
-    const ActivityUserData::TypedValue& recorded_value = name_and_value.second;
-    TypedValue collected_value;
-
-    switch (recorded_value.type()) {
-      case ActivityUserData::END_OF_VALUES:
-        NOTREACHED();
-        break;
-      case ActivityUserData::RAW_VALUE: {
-        base::StringPiece raw = recorded_value.Get();
-        collected_value.set_bytes_value(raw.data(), raw.size());
-        break;
-      }
-      case ActivityUserData::RAW_VALUE_REFERENCE: {
-        base::StringPiece recorded_ref = recorded_value.GetReference();
-        TypedValue::Reference* collected_ref =
-            collected_value.mutable_bytes_reference();
-        collected_ref->set_address(
-            reinterpret_cast<uintptr_t>(recorded_ref.data()));
-        collected_ref->set_size(recorded_ref.size());
-        break;
-      }
-      case ActivityUserData::STRING_VALUE: {
-        base::StringPiece value = recorded_value.GetString();
-
-        if (report && base::StartsWith(key, kFieldTrialKeyPrefix,
-                                       base::CompareCase::SENSITIVE)) {
-          // This entry represents an active Field Trial.
-          std::string trial_name =
-              key.substr(std::strlen(kFieldTrialKeyPrefix));
-          variations::ActiveGroupId group_id =
-              variations::MakeActiveGroupId(trial_name, value.as_string());
-          FieldTrial* field_trial = report->add_field_trials();
-          field_trial->set_name_id(group_id.name);
-          field_trial->set_group_id(group_id.group);
-          continue;
-        }
-
-        collected_value.set_string_value(value.data(), value.size());
-        break;
-      }
-      case ActivityUserData::STRING_VALUE_REFERENCE: {
-        base::StringPiece recorded_ref = recorded_value.GetStringReference();
-        TypedValue::Reference* collected_ref =
-            collected_value.mutable_string_reference();
-        collected_ref->set_address(
-            reinterpret_cast<uintptr_t>(recorded_ref.data()));
-        collected_ref->set_size(recorded_ref.size());
-        break;
-      }
-      case ActivityUserData::CHAR_VALUE: {
-        char char_value = recorded_value.GetChar();
-        collected_value.set_char_value(&char_value, 1);
-        break;
-      }
-      case ActivityUserData::BOOL_VALUE:
-        collected_value.set_bool_value(recorded_value.GetBool());
-        break;
-      case ActivityUserData::SIGNED_VALUE:
-        collected_value.set_signed_value(recorded_value.GetInt());
-        break;
-      case ActivityUserData::UNSIGNED_VALUE:
-        collected_value.set_unsigned_value(recorded_value.GetUint());
-        break;
-    }
-
-    (*collected_map)[key].Swap(&collected_value);
-  }
-}
-
-void CollectModuleInformation(
-    const std::vector<GlobalActivityTracker::ModuleInfo>& modules,
-    ProcessState* process_state) {
-  DCHECK(process_state);
-
-  char code_identifier[17];
-  char debug_identifier[41];
-
-  for (const GlobalActivityTracker::ModuleInfo& recorded : modules) {
-    CodeModule* collected = process_state->add_modules();
-    collected->set_base_address(recorded.address);
-    collected->set_size(recorded.size);
-    collected->set_code_file(recorded.file);
-
-    // Compute the code identifier using the required format.
-    snprintf(code_identifier, sizeof(code_identifier), "%08X%zx",
-             recorded.timestamp, recorded.size);
-    collected->set_code_identifier(code_identifier);
-    collected->set_debug_file(recorded.debug_file);
-
-    // Compute the debug identifier using the required format.
-    const crashpad::UUID* uuid =
-        reinterpret_cast<const crashpad::UUID*>(recorded.identifier);
-    snprintf(debug_identifier, sizeof(debug_identifier),
-             "%08X%04X%04X%02X%02X%02X%02X%02X%02X%02X%02X%x", uuid->data_1,
-             uuid->data_2, uuid->data_3, uuid->data_4[0], uuid->data_4[1],
-             uuid->data_5[0], uuid->data_5[1], uuid->data_5[2], uuid->data_5[3],
-             uuid->data_5[4], uuid->data_5[5], recorded.age);
-    collected->set_debug_identifier(debug_identifier);
-    collected->set_is_unloaded(!recorded.is_loaded);
-  }
-}
-
-}  // namespace
-
 PostmortemReportCollector::PostmortemReportCollector(
     const std::string& product_name,
     const std::string& version_number,
@@ -158,10 +32,12 @@ PostmortemReportCollector::PostmortemReportCollector(
       version_number_(version_number),
       channel_name_(channel_name) {}
 
+PostmortemReportCollector::~PostmortemReportCollector() {}
+
 int PostmortemReportCollector::CollectAndSubmitForUpload(
-    const base::FilePath& debug_info_dir,
-    const base::FilePath::StringType& debug_file_pattern,
-    const std::set<base::FilePath>& excluded_debug_files,
+    const FilePath& debug_info_dir,
+    const FilePath::StringType& debug_file_pattern,
+    const std::set<FilePath>& excluded_debug_files,
     crashpad::CrashReportDatabase* report_database) {
   DCHECK_NE(true, debug_info_dir.empty());
   DCHECK_NE(true, debug_file_pattern.empty());
@@ -198,8 +74,8 @@ int PostmortemReportCollector::CollectAndSubmitForUpload(
 }
 
 std::vector<FilePath> PostmortemReportCollector::GetDebugStateFilePaths(
-    const base::FilePath& debug_info_dir,
-    const base::FilePath::StringType& debug_file_pattern,
+    const FilePath& debug_info_dir,
+    const FilePath::StringType& debug_file_pattern,
     const std::set<FilePath>& excluded_debug_files) {
   DCHECK_NE(true, debug_info_dir.empty());
   DCHECK_NE(true, debug_file_pattern.empty());
@@ -216,8 +92,7 @@ std::vector<FilePath> PostmortemReportCollector::GetDebugStateFilePaths(
   return paths;
 }
 
-PostmortemReportCollector::CollectionStatus
-PostmortemReportCollector::CollectAndSubmit(
+CollectionStatus PostmortemReportCollector::CollectAndSubmit(
     const crashpad::UUID& client_id,
     const FilePath& file,
     crashpad::CrashReportDatabase* report_database) {
@@ -228,7 +103,7 @@ PostmortemReportCollector::CollectAndSubmit(
 
   // 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;
+  StabilityReport report_proto;
   CollectionStatus status = Collect(file, &report_proto);
   if (status != SUCCESS) {
     // The file was empty, or there was an error collecting the data. Detailed
@@ -237,7 +112,6 @@ PostmortemReportCollector::CollectAndSubmit(
       DLOG(ERROR) << "Failed to delete " << file.value();
     return status;
   }
-  DCHECK_NE(nullptr, report_proto.get());
 
   // Prepare a crashpad report.
   CrashReportDatabase::NewReport* new_report = nullptr;
@@ -252,7 +126,7 @@ PostmortemReportCollector::CollectAndSubmit(
       call_error_writing_crash_report(report_database, new_report);
 
   // Write the report to a minidump.
-  if (!WriteReportToMinidump(report_proto.get(), client_id, new_report->uuid,
+  if (!WriteReportToMinidump(&report_proto, client_id, new_report->uuid,
                              reinterpret_cast<FILE*>(new_report->handle))) {
     // Assume this is not recoverable and delete the file.
     if (!base::DeleteFile(file, false))
@@ -285,42 +159,16 @@ PostmortemReportCollector::CollectAndSubmit(
   return SUCCESS;
 }
 
-PostmortemReportCollector::CollectionStatus PostmortemReportCollector::Collect(
-    const base::FilePath& debug_state_file,
-    std::unique_ptr<StabilityReport>* report) {
-  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();
-  ActivityUserData::Snapshot global_data_snapshot =
-      global_analyzer->GetGlobalUserDataSnapshot();
-  ThreadActivityAnalyzer* thread_analyzer = global_analyzer->GetFirstAnalyzer();
-  if (log_messages.empty() && global_data_snapshot.empty() &&
-      !thread_analyzer) {
-    return DEBUG_FILE_NO_DATA;
-  }
-
-  // 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);
-  }
+CollectionStatus PostmortemReportCollector::Collect(const base::FilePath& file,
+                                                    StabilityReport* report) {
+  DCHECK(report);
+  CollectionStatus status = Extract(file, report);
+  if (status != SUCCESS)
+    return status;
 
-  // Collect global user data.
+  // Add the reporter's details to the report.
   google::protobuf::Map<std::string, TypedValue>& global_data =
-      *(*report)->mutable_global_data();
-  CollectUserData(global_data_snapshot, &global_data, report->get());
-
-  // Add the reporting Chrome's details to the report.
+      *(report->mutable_global_data());
   global_data[kStabilityReporterChannel].set_string_value(channel_name());
 #if defined(ARCH_CPU_X86)
   global_data[kStabilityReporterPlatform].set_string_value(
@@ -332,80 +180,9 @@ PostmortemReportCollector::CollectionStatus PostmortemReportCollector::Collect(
   global_data[kStabilityReporterProduct].set_string_value(product_name());
   global_data[kStabilityReporterVersion].set_string_value(version_number());
 
-  // 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);
-    }
-    DCHECK_EQ(thread_analyzer->activity_snapshot().process_id,
-              process_state->process_id());
-
-    ThreadState* thread_state = process_state->add_threads();
-    CollectThread(thread_analyzer->activity_snapshot(), thread_state);
-  }
-
-  // Collect module information.
-  CollectModuleInformation(global_analyzer->GetModules(), process_state);
-
   return SUCCESS;
 }
 
-void PostmortemReportCollector::CollectThread(
-    const base::debug::ThreadActivityAnalyzer::Snapshot& snapshot,
-    ThreadState* thread_state) {
-  DCHECK(thread_state);
-
-  thread_state->set_thread_name(snapshot.thread_name);
-  thread_state->set_thread_id(snapshot.thread_id);
-  thread_state->set_activity_count(snapshot.activity_stack_depth);
-
-  for (size_t i = 0; i < snapshot.activity_stack.size(); ++i) {
-    const base::debug::Activity& recorded = snapshot.activity_stack[i];
-    Activity* collected = thread_state->add_activities();
-
-    // Collect activity
-    switch (recorded.activity_type) {
-      case base::debug::Activity::ACT_TASK_RUN:
-        collected->set_type(Activity::ACT_TASK_RUN);
-        collected->set_origin_address(recorded.origin_address);
-        collected->set_task_sequence_id(recorded.data.task.sequence_id);
-        break;
-      case base::debug::Activity::ACT_LOCK_ACQUIRE:
-        collected->set_type(Activity::ACT_LOCK_ACQUIRE);
-        collected->set_lock_address(recorded.data.lock.lock_address);
-        break;
-      case base::debug::Activity::ACT_EVENT_WAIT:
-        collected->set_type(Activity::ACT_EVENT_WAIT);
-        collected->set_event_address(recorded.data.event.event_address);
-        break;
-      case base::debug::Activity::ACT_THREAD_JOIN:
-        collected->set_type(Activity::ACT_THREAD_JOIN);
-        collected->set_thread_id(recorded.data.thread.thread_id);
-        break;
-      case base::debug::Activity::ACT_PROCESS_WAIT:
-        collected->set_type(Activity::ACT_PROCESS_WAIT);
-        collected->set_process_id(recorded.data.process.process_id);
-        break;
-      default:
-        break;
-    }
-
-    // Collect user data
-    if (i < snapshot.user_data_stack.size()) {
-      CollectUserData(snapshot.user_data_stack[i],
-                      collected->mutable_user_data(), nullptr);
-    }
-  }
-}
-
 bool PostmortemReportCollector::WriteReportToMinidump(
     StabilityReport* report,
     const crashpad::UUID& client_id,