Cherry pick PR #3005: Added persistent setting for logtrace killswitch.

cobalt/watchdog/watchdog.cc

@@ -53,19 +53,6 @@ const int kWatchdogMaxPingInfoLength = 1024;
 // The maximum number of milliseconds old of an unfetched Watchdog violation.
 const int64_t kWatchdogMaxViolationsAge = 86400000;
 
-// Persistent setting name and default setting for the boolean that controls
-// whether or not Watchdog is enabled. When disabled, Watchdog behaves like a
-// stub except that persistent settings can still be get/set. Requires a
-// restart to take effect.
-const char kPersistentSettingWatchdogEnable[] =
-    "kPersistentSettingWatchdogEnable";
-const bool kDefaultSettingWatchdogEnable = true;
-// Persistent setting name and default setting for the boolean that controls
-// whether or not a Watchdog violation will trigger a crash.
-const char kPersistentSettingWatchdogCrash[] =
-    "kPersistentSettingWatchdogCrash";
-const bool kDefaultSettingWatchdogCrash = false;
-
 }  // namespace
 
 bool Watchdog::Initialize(
@@ -80,6 +67,7 @@ bool Watchdog::InitializeCustom(
     std::string watchdog_file_name, int64_t watchdog_monitor_frequency) {
   persistent_settings_ = persistent_settings;
   is_disabled_ = !GetPersistentSettingWatchdogEnable();
+  is_logtrace_disabled_ = !GetPersistentSettingLogtraceEnable();
 
   if (is_disabled_) return true;
 
@@ -805,14 +793,45 @@ void Watchdog::SetPersistentSettingWatchdogCrash(bool can_trigger_crash) {
 }
 
 bool Watchdog::LogEvent(const std::string& event) {
+  if (is_logtrace_disabled_) {
+    return true;
+  }
+
   return instrumentation_log_.LogEvent(event);
 }
 
 std::vector<std::string> Watchdog::GetLogTrace() {
+  if (is_logtrace_disabled_) {
+    return {};
+  }
+
   return instrumentation_log_.GetLogTrace();
 }
 
-void Watchdog::ClearLog() { instrumentation_log_.ClearLog(); }
+void Watchdog::ClearLog() {
+  if (is_logtrace_disabled_) {
+    return;
+  }
+
+  instrumentation_log_.ClearLog();
+}
+
+bool Watchdog::GetPersistentSettingLogtraceEnable() {
+  if (!persistent_settings_) return kDefaultSettingLogtraceEnable;
+
+  // Gets the boolean that controls whether or not LogTrace is enabled.
+  return persistent_settings_->GetPersistentSettingAsBool(
+      kPersistentSettingLogtraceEnable, kDefaultSettingLogtraceEnable);
+}
+
+void Watchdog::SetPersistentSettingLogtraceEnable(bool enable_logtrace) {
+  if (!persistent_settings_) return;
+
+  // Sets the boolean that controls whether or not LogTrace is enabled.
+  persistent_settings_->SetPersistentSetting(
+      kPersistentSettingLogtraceEnable,
+      std::make_unique<base::Value>(enable_logtrace));
+}
 
 #if defined(_DEBUG)
 // Sleeps threads for Watchdog debugging.

cobalt/watchdog/watchdog.h

@@ -33,6 +33,28 @@
 namespace cobalt {
 namespace watchdog {
 
+// Persistent setting name and default setting for the boolean that controls
+// whether or not Watchdog is enabled. When disabled, Watchdog behaves like a
+// stub except that persistent settings can still be get/set. Requires a
+// restart to take effect.
+constexpr char kPersistentSettingWatchdogEnable[] =
+    "kPersistentSettingWatchdogEnable";
+constexpr bool kDefaultSettingWatchdogEnable = true;
+
+// Persistent setting name and default setting for the boolean that controls
+// whether or not a Watchdog violation will trigger a crash.
+constexpr char kPersistentSettingWatchdogCrash[] =
+    "kPersistentSettingWatchdogCrash";
+constexpr bool kDefaultSettingWatchdogCrash = false;
+
+// Persistent setting name and default setting for the boolean that controls
+// whether or not LogTrace API is enabled. When disabled, all LogTrace methods
+// behave like a stub except for persistent settings itself. Requires a
+// restart to take effect.
+constexpr char kPersistentSettingLogtraceEnable[] =
+    "kPersistentSettingLogtraceEnable";
+constexpr bool kDefaultSettingLogtraceEnable = true;
+
 // Client to monitor
 typedef struct Client {
   std::string name;
@@ -112,6 +134,8 @@ class Watchdog : public Singleton<Watchdog> {
   bool LogEvent(const std::string& event);
   std::vector<std::string> GetLogTrace();
   void ClearLog();
+  bool GetPersistentSettingLogtraceEnable();
+  void SetPersistentSettingLogtraceEnable(bool enable_logtrace);
 
 #if defined(_DEBUG)
   // Sleeps threads based off of environment variables for Watchdog debugging.
@@ -181,6 +205,9 @@ class Watchdog : public Singleton<Watchdog> {
   int64_t watchdog_monitor_frequency_;
   // Captures string events emitted from Kabuki via logEvent() h5vcc API.
   InstrumentationLog instrumentation_log_;
+  // Flag to disable LogTrace API. When disabled, all LogTrace methods behave
+  // like a stub except that the flag itself can still be get/set.
+  bool is_logtrace_disabled_;
 
 #if defined(_DEBUG)
   starboard::Mutex delay_mutex_;

cobalt/watchdog/watchdog_test.cc

@@ -22,12 +22,15 @@
 
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
+#include "base/test/task_environment.h"
 #include "starboard/common/file.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace cobalt {
 namespace watchdog {
 
+using persistent_storage::PersistentSettings;
+
 namespace {
 
 const char kWatchdogViolationsJson[] = "watchdog_test.json";
@@ -38,7 +41,9 @@ const int64_t kWatchdogSleepDuration = kWatchdogMonitorFrequency * 4;
 
 class WatchdogTest : public testing::Test {
  protected:
-  WatchdogTest() {}
+  WatchdogTest()
+      : task_environment_(
+            base::test::TaskEnvironment::MainThreadType::DEFAULT) {}
 
   void SetUp() final {
     watchdog_ = new watchdog::Watchdog();
@@ -51,6 +56,8 @@ class WatchdogTest : public testing::Test {
     watchdog_->Uninitialize();
     delete watchdog_;
     watchdog_ = nullptr;
+
+    DeletePersistentSettingsFile();
   }
 
   base::Value CreateDummyViolationDict(std::string desc, int begin, int end) {
@@ -82,7 +89,23 @@ class WatchdogTest : public testing::Test {
     return violation.Clone();
   }
 
+  void DeletePersistentSettingsFile() {
+    std::vector<char> storage_dir(kSbFileMaxPath + 1, 0);
+    SbSystemGetPath(kSbSystemPathCacheDirectory, storage_dir.data(),
+                    kSbFileMaxPath);
+    std::string path =
+        std::string(storage_dir.data()) + kSbFileSepString + kSettingsFileName;
+
+    starboard::SbFileDeleteRecursive(path.c_str(), true);
+  }
+
+  const std::string kSettingsFileName = "test-settings.json";
+
   watchdog::Watchdog* watchdog_;
+  base::test::TaskEnvironment task_environment_;
+  base::WaitableEvent task_done_ = {
+      base::WaitableEvent::ResetPolicy::MANUAL,
+      base::WaitableEvent::InitialState::NOT_SIGNALED};
 };
 
 TEST_F(WatchdogTest, RedundantRegistersShouldFail) {
@@ -719,5 +742,69 @@ TEST_F(WatchdogTest, ViolationContainsEmptyLogTrace) {
   ASSERT_EQ(logTrace->size(), 0);
 }
 
+TEST_F(WatchdogTest, WatchdogMethodsAreNoopWhenWatchdogIsDisabled) {
+  // init and destroy existing watchdog to re-initialize it later
+  watchdog_->Register("test-name", "test-desc", base::kApplicationStateStarted,
+                      kWatchdogMonitorFrequency);
+  TearDown();
+
+  // PersistentSettings doesn't have interface so it's not mockable
+  auto persistent_settings =
+      std::make_unique<PersistentSettings>(kSettingsFileName);
+  persistent_settings->ValidatePersistentSettings();
+
+  base::OnceClosure closure = base::BindOnce(
+      [](base::WaitableEvent* task_done) { task_done->Signal(); }, &task_done_);
+  persistent_settings->SetPersistentSetting(
+      kPersistentSettingWatchdogEnable, std::make_unique<base::Value>(false),
+      std::move(closure), true);
+  task_done_.Wait();
+
+  watchdog_ = new watchdog::Watchdog();
+  watchdog_->InitializeCustom(persistent_settings.get(),
+                              std::string(kWatchdogViolationsJson),
+                              kWatchdogMonitorFrequency);
+
+  ASSERT_TRUE(watchdog_->Register("test-name", "test-desc",
+                                  base::kApplicationStateStarted,
+                                  kWatchdogMonitorFrequency));
+  ASSERT_TRUE(watchdog_->Ping("test-name"));
+  ASSERT_TRUE(watchdog_->PingByClient(nullptr));
+
+  usleep(kWatchdogSleepDuration);
+
+  ASSERT_EQ(watchdog_->GetWatchdogViolations(), "");
+  ASSERT_TRUE(watchdog_->Unregister("test-name"));
+  ASSERT_TRUE(watchdog_->Unregister(""));
+}
+
+TEST_F(WatchdogTest, LogtraceMethodsAreNoopWhenLogtraceIsDisabled) {
+  // init and destroy existing watchdog to re-initialize it later
+  watchdog_->Register("test-name", "test-desc", base::kApplicationStateStarted,
+                      kWatchdogMonitorFrequency);
+  TearDown();
+
+  // PersistentSettings doesn't have interface so it's not mockable
+  auto persistent_settings =
+      std::make_unique<PersistentSettings>(kSettingsFileName);
+  persistent_settings->ValidatePersistentSettings();
+
+  base::OnceClosure closure = base::BindOnce(
+      [](base::WaitableEvent* task_done) { task_done->Signal(); }, &task_done_);
+  persistent_settings->SetPersistentSetting(
+      kPersistentSettingLogtraceEnable, std::make_unique<base::Value>(false),
+      std::move(closure), true);
+  task_done_.Wait();
+
+  watchdog_ = new watchdog::Watchdog();
+  watchdog_->InitializeCustom(persistent_settings.get(),
+                              std::string(kWatchdogViolationsJson),
+                              kWatchdogMonitorFrequency);
+
+  ASSERT_TRUE(watchdog_->LogEvent("foo"));
+  ASSERT_EQ(watchdog_->GetLogTrace().size(), 0);
+  ASSERT_NO_FATAL_FAILURE(watchdog_->ClearLog());
+}
+
 }  // namespace watchdog
 }  // namespace cobalt