Collect stability data from global and activity scopes

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"
 #include "base/files/file_util.h"
 #include "base/logging.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/path_service.h"
+#include "base/strings/string_piece.h"
 #include "base/strings/utf_string_conversions.h"
 #include "components/browser_watcher/postmortem_minidump_writer.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::ThreadActivityAnalyzer;
 using crashpad::CrashReportDatabase;
 
+namespace {
+
+// 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) {
+  DCHECK(collected_map);
+
+  for (const auto& name_and_value : recorded_map) {
+    const ActivityUserData::TypedValue& recorded_value = name_and_value.second;
+    TypedValue collected_value;
+
+    switch (recorded_value.get_type()) {
+      case ActivityUserData::END_OF_VALUES:
+        NOTREACHED();
+        break;
+      case ActivityUserData::RAW_VALUE:
+        collected_value.set_bytes_value(recorded_value.Get().as_string());
+        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;

[bcwhite, 2016/12/22 17:21:58]

I believe style is

} break;

At least, "git cl format" has done that for me in the past.

[manzagop (departed), 2017/01/10 14:11:22]

The example from the style guide has it inside. Maybe both are ok?
https://google.github.io/styleguide/cppguide.html#Loops_and_Switch_Statements

[bcwhite, 2017/01/10 18:00:38]

Acknowledged.

+      }
+      case ActivityUserData::STRING_VALUE:
+        collected_value.set_string_value(
+            recorded_value.GetString().as_string());
+        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:
+        collected_value.set_char_value(
+            std::string(1, recorded_value.GetChar()));
+        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)[name_and_value.first] = collected_value;

[bcwhite, 2016/12/22 17:21:58]

This will create an default value and then assign to it.  Using

->insert(make_pair(a, std::move(b)))

might be more efficient.  At the very least:

std::move(collected_value)

[manzagop (departed), 2017/01/10 14:11:22]

I don't think protos can be moved. I've switched to a swap to avoid the copy
though.

+  }
+}
+
+}  // namespace
+
 PostmortemReportCollector::PostmortemReportCollector(
     const std::string& product_name,
     const std::string& version_number,
     const std::string& channel_name)
     : product_name_(product_name),
       version_number_(version_number),
       channel_name_(channel_name) {}
 
 int PostmortemReportCollector::CollectAndSubmitForUpload(
     const base::FilePath& debug_info_dir,
@@ skipping 123 matching lines @@
   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() && !thread_analyzer) {
+  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);
   }
 
+  // Collect global user data.
+  CollectUserData(global_data_snapshot, (*report)->mutable_global_data());
+
   // 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()) {
@@ skipping 12 matching lines @@
 
 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 (const base::debug::Activity& recorded : snapshot.activity_stack) {
+  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());
+    }
   }
 }
 
 bool PostmortemReportCollector::WriteReportToMinidump(
     const StabilityReport& report,
     const crashpad::UUID& client_id,
     const crashpad::UUID& report_id,
     base::PlatformFile minidump_file) {
   MinidumpInfo minidump_info;
   minidump_info.client_id = client_id;
   minidump_info.report_id = report_id;
   // TODO(manzagop): replace this information, i.e. the reporter's attributes,
   // by that of the reportee. Doing so requires adding this information to the
   // stability report. In the meantime, there is a tolerable information
   // mismatch after upgrades.
   minidump_info.product_name = product_name();
   minidump_info.version_number = version_number();
   minidump_info.channel_name = channel_name();
 #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