Cherry pick PR #580: Add application state to crash reports. (#691)

* Add application state to crash reports. (#580) Add application state to crash reports by using the crash handler extension and falling back to the crash log dictionary if the extension is not supported. Refactors the h5vcc crash log dictionary singleton to support this. b/261773222 (cherry picked from commit 4eb9bbc) * Update application.cc --------- Co-authored-by: Brian Ting <[email protected]>
youtube · Jul 10, 2023 · 76a3a23 · 76a3a23
1 parent cca954c
commit 76a3a23
Show file tree

Hide file tree

Showing 3 changed files with 82 additions and 70 deletions.
diff --git a/cobalt/browser/application.cc b/cobalt/browser/application.cc
@@ -65,6 +65,7 @@
 #include "cobalt/configuration/configuration.h"
 #include "cobalt/extension/crash_handler.h"
 #include "cobalt/extension/installation_manager.h"
+#include "cobalt/h5vcc/h5vcc_crash_log.h"
 #include "cobalt/loader/image/image_decoder.h"
 #include "cobalt/math/size.h"
 #include "cobalt/script/javascript_engine.h"
@@ -589,26 +590,24 @@ void AddCrashHandlerAnnotations() {
   }
 }
 
-void AddCrashHandlerApplicationState(base::ApplicationState state) {
+void AddCrashLogApplicationState(base::ApplicationState state) {
+  std::string application_state = std::string(GetApplicationStateString(state));
+  application_state.push_back('\0');
+
   auto crash_handler_extension =
       static_cast<const CobaltExtensionCrashHandlerApi*>(
           SbSystemGetExtension(kCobaltExtensionCrashHandlerName));
-  if (!crash_handler_extension) {
-    DLOG(INFO) << "No crash handler extension, not sending application state.";
+  if (crash_handler_extension && crash_handler_extension->version >= 2) {
+    if (!crash_handler_extension->SetString("application_state",
+                                            application_state.c_str())) {
+      LOG(ERROR) << "Could not send application state to crash handler.";
+    }
     return;
   }
 
-  std::string application_state = std::string(GetApplicationStateString(state));
-  application_state.push_back('\0');
-
-  if (crash_handler_extension->version > 1) {
-    if (crash_handler_extension->SetString("application_state",
-                                           application_state.c_str())) {
-      DLOG(INFO) << "Sent application state to crash handler.";
-      return;
-    }
-  }
-  DLOG(ERROR) << "Could not send application state to crash handler.";
+  // Crash handler is not supported, fallback to crash log dictionary.
+  h5vcc::CrashLogDictionary::GetInstance()->SetString("application_state",
+                                                      application_state);
 }
 
 }  // namespace
@@ -1220,7 +1219,7 @@ void Application::OnApplicationEvent(SbEventType event_type,
     case kSbEventTypeStop:
       LOG(INFO) << "Got quit event.";
       if (watchdog) watchdog->UpdateState(base::kApplicationStateStopped);
-      AddCrashHandlerApplicationState(base::kApplicationStateStopped);
+      AddCrashLogApplicationState(base::kApplicationStateStopped);
       Quit();
       LOG(INFO) << "Finished quitting.";
       break;
@@ -1339,7 +1338,7 @@ void Application::OnApplicationEvent(SbEventType event_type,
       return;
   }
   if (watchdog) watchdog->UpdateState(browser_module_->GetApplicationState());
-  AddCrashHandlerApplicationState(browser_module_->GetApplicationState());
+  AddCrashLogApplicationState(browser_module_->GetApplicationState());
 }
 
 void Application::OnWindowSizeChangedEvent(const base::Event* event) {

diff --git a/cobalt/h5vcc/h5vcc_crash_log.cc b/cobalt/h5vcc/h5vcc_crash_log.cc
@@ -31,70 +31,49 @@
 namespace cobalt {
 namespace h5vcc {
 
-// We keep a global mapping of all registered logs.  When a crash occurs, we
-// will iterate through the mapping in this dictionary to extract all
-// logged entries and write them to the system's crash logger via Starboard.
-class CrashLogDictionary {
- public:
-  static CrashLogDictionary* GetInstance() {
-    return base::Singleton<CrashLogDictionary, base::DefaultSingletonTraits<
-                                                   CrashLogDictionary> >::get();
-  }
+CrashLogDictionary* CrashLogDictionary::GetInstance() {
+  return base::Singleton<CrashLogDictionary, base::DefaultSingletonTraits<
+                                                 CrashLogDictionary>>::get();
+}
 
-  void SetString(const std::string& key, const std::string& value) {
-    base::AutoLock lock(mutex_);
-    // While the lock prevents contention between other calls to SetString(),
-    // the atomics guard against OnCrash(), which doesn't acquire |mutex_|, from
-    // accessing the data at the same time.  In the case that OnCrash() is
-    // being called, we give up and skip adding the data.
-    if (base::subtle::Acquire_CompareAndSwap(&accessing_log_data_, 0, 1) == 0) {
-      string_log_map_[key] = value;
-      base::subtle::Release_Store(&accessing_log_data_, 0);
-    }
+void CrashLogDictionary::SetString(const std::string& key,
+                                   const std::string& value) {
+  base::AutoLock lock(mutex_);
+  // While the lock prevents contention between other calls to SetString(),
+  // the atomics guard against OnCrash(), which doesn't acquire |mutex_|, from
+  // accessing the data at the same time.  In the case that OnCrash() is
+  // being called, we give up and skip adding the data.
+  if (base::subtle::Acquire_CompareAndSwap(&accessing_log_data_, 0, 1) == 0) {
+    string_log_map_[key] = value;
+    base::subtle::Release_Store(&accessing_log_data_, 0);
   }
+}
 
- private:
-  CrashLogDictionary() : accessing_log_data_(0) {
+CrashLogDictionary::CrashLogDictionary() : accessing_log_data_(0) {
 #if SB_HAS(CORE_DUMP_HANDLER_SUPPORT)
-    SbCoreDumpRegisterHandler(&CoreDumpHandler, this);
+  SbCoreDumpRegisterHandler(&CoreDumpHandler, this);
 #endif
-  }
+}
 
-  static void CoreDumpHandler(void* context) {
-    CrashLogDictionary* crash_log_dictionary =
-        static_cast<CrashLogDictionary*>(context);
-    crash_log_dictionary->OnCrash();
-  }
+void CrashLogDictionary::CoreDumpHandler(void* context) {
+  CrashLogDictionary* crash_log_dictionary =
+      static_cast<CrashLogDictionary*>(context);
+  crash_log_dictionary->OnCrash();
+}
 
-  void OnCrash() {
+void CrashLogDictionary::OnCrash() {
 #if SB_HAS(CORE_DUMP_HANDLER_SUPPORT)
-    // Check that we're not already updating log data.  If we are, we just
-    // give up and skip recording any crash data, but hopefully this is rare.
-    if (base::subtle::Acquire_CompareAndSwap(&accessing_log_data_, 0, 1) == 0) {
-      for (StringMap::const_iterator iter = string_log_map_.begin();
-           iter != string_log_map_.end(); ++iter) {
-        SbCoreDumpLogString(iter->first.c_str(), iter->second.c_str());
-      }
-      base::subtle::Release_Store(&accessing_log_data_, 0);
+  // Check that we're not already updating log data.  If we are, we just
+  // give up and skip recording any crash data, but hopefully this is rare.
+  if (base::subtle::Acquire_CompareAndSwap(&accessing_log_data_, 0, 1) == 0) {
+    for (StringMap::const_iterator iter = string_log_map_.begin();
+         iter != string_log_map_.end(); ++iter) {
+      SbCoreDumpLogString(iter->first.c_str(), iter->second.c_str());
     }
-#endif
+    base::subtle::Release_Store(&accessing_log_data_, 0);
   }
-
-  friend struct base::DefaultSingletonTraits<CrashLogDictionary>;
-
-  base::subtle::Atomic32 accessing_log_data_;
-
-  // It is possible for multiple threads to call the H5VCC interface at the
-  // same time, and they will all forward to this global singleton, so we
-  // use this mutex to protect against concurrent access.
-  base::Lock mutex_;
-
-  typedef std::map<std::string, std::string> StringMap;
-  // Keeps track of all string values to be logged when a crash occurs.
-  StringMap string_log_map_;
-
-  DISALLOW_COPY_AND_ASSIGN(CrashLogDictionary);
-};
+#endif
+}
 
 bool H5vccCrashLog::SetString(const std::string& key,
                               const std::string& value) {

diff --git a/cobalt/h5vcc/h5vcc_crash_log.h b/cobalt/h5vcc/h5vcc_crash_log.h
@@ -15,8 +15,10 @@
 #ifndef COBALT_H5VCC_H5VCC_CRASH_LOG_H_
 #define COBALT_H5VCC_H5VCC_CRASH_LOG_H_
 
+#include <map>
 #include <string>
 
+#include "base/memory/singleton.h"
 #include "cobalt/h5vcc/h5vcc_crash_type.h"
 #include "cobalt/h5vcc/watchdog_replace.h"
 #include "cobalt/h5vcc/watchdog_state.h"
@@ -26,6 +28,38 @@
 namespace cobalt {
 namespace h5vcc {
 
+// We keep a global mapping of all registered logs.  When a crash occurs, we
+// will iterate through the mapping in this dictionary to extract all
+// logged entries and write them to the system's crash logger via Starboard.
+class CrashLogDictionary {
+ public:
+  static CrashLogDictionary* GetInstance();
+
+  void SetString(const std::string& key, const std::string& value);
+
+ private:
+  CrashLogDictionary();
+
+  static void CoreDumpHandler(void* context);
+
+  void OnCrash();
+
+  friend struct base::DefaultSingletonTraits<CrashLogDictionary>;
+
+  base::subtle::Atomic32 accessing_log_data_;
+
+  // It is possible for multiple threads to call the H5VCC interface at the
+  // same time, and they will all forward to this global singleton, so we
+  // use this mutex to protect against concurrent access.
+  base::Lock mutex_;
+
+  typedef std::map<std::string, std::string> StringMap;
+  // Keeps track of all string values to be logged when a crash occurs.
+  StringMap string_log_map_;
+
+  DISALLOW_COPY_AND_ASSIGN(CrashLogDictionary);
+};
+
 class H5vccCrashLog : public script::Wrappable {
  public:
   H5vccCrashLog() {}