From eccfac1483018d7c8c3ff6ba469f9c4b3e6e904d Mon Sep 17 00:00:00 2001 From: Sherry Zhou Date: Mon, 7 Aug 2023 16:54:28 -0700 Subject: [PATCH] Add clients lists as an optional parameter to GetWatchdogViolations() b/287302949 --- cobalt/h5vcc/h5vcc_crash_log.cc | 17 +++++++- cobalt/h5vcc/h5vcc_crash_log.h | 3 +- cobalt/h5vcc/h5vcc_crash_log.idl | 2 +- cobalt/watchdog/watchdog.cc | 67 +++++++++++++++++++++++++----- cobalt/watchdog/watchdog.h | 3 +- cobalt/watchdog/watchdog_test.cc | 71 ++++++++++++++++++++++++++++++-- 6 files changed, 145 insertions(+), 18 deletions(-) diff --git a/cobalt/h5vcc/h5vcc_crash_log.cc b/cobalt/h5vcc/h5vcc_crash_log.cc index baccbac2ea0d..ebcba463a5f8 100644 --- a/cobalt/h5vcc/h5vcc_crash_log.cc +++ b/cobalt/h5vcc/h5vcc_crash_log.cc @@ -171,9 +171,22 @@ bool H5vccCrashLog::Ping(const std::string& name, return false; } -std::string H5vccCrashLog::GetWatchdogViolations() { +std::string H5vccCrashLog::GetWatchdogViolations(const script::Sequence& clients) { watchdog::Watchdog* watchdog = watchdog::Watchdog::GetInstance(); - if (watchdog) return watchdog->GetWatchdogViolations(); + if (watchdog) { + // If not clients name is given, return all clients' data. + if (clients.size() == 0) { + return watchdog->GetWatchdogViolations(); + } + std::unique_ptr client_names( + new const char*[clients.size()]); + for (script::Sequence::size_type i = 0; i < clients.size(); + ++i) { + client_names[i] = clients.at(i).c_str(); + } + return watchdog->GetWatchdogViolations(client_names.get(), + static_cast(clients.size())); + } return ""; } diff --git a/cobalt/h5vcc/h5vcc_crash_log.h b/cobalt/h5vcc/h5vcc_crash_log.h index bb20a48c07c7..ea06c88f7d10 100644 --- a/cobalt/h5vcc/h5vcc_crash_log.h +++ b/cobalt/h5vcc/h5vcc_crash_log.h @@ -78,7 +78,8 @@ class H5vccCrashLog : public script::Wrappable { bool Ping(const std::string& name, const std::string& ping_info); - std::string GetWatchdogViolations(); + std::string GetWatchdogViolations( + const script::Sequence& clients = {}); bool GetPersistentSettingWatchdogEnable(); diff --git a/cobalt/h5vcc/h5vcc_crash_log.idl b/cobalt/h5vcc/h5vcc_crash_log.idl index e93caac491e8..8d4a3baf3b1b 100644 --- a/cobalt/h5vcc/h5vcc_crash_log.idl +++ b/cobalt/h5vcc/h5vcc_crash_log.idl @@ -84,7 +84,7 @@ interface H5vccCrashLog { // ] // } // } - DOMString getWatchdogViolations(); + DOMString getWatchdogViolations(optional sequence clients); // Gets a persistent Watchdog setting that determines whether or not Watchdog // is enabled. When disabled, Watchdog behaves like a stub except that diff --git a/cobalt/watchdog/watchdog.cc b/cobalt/watchdog/watchdog.cc index 9faf98378f3a..be42ade265ab 100644 --- a/cobalt/watchdog/watchdog.cc +++ b/cobalt/watchdog/watchdog.cc @@ -518,13 +518,14 @@ bool Watchdog::Ping(const std::string& name, const std::string& info) { return client_exists; } -std::string Watchdog::GetWatchdogViolations(bool clear) { +std::string Watchdog::GetWatchdogViolations(const char* clients[], + int client_num, bool clear) { // Gets a json string containing the Watchdog violations since the last // call (up to the kWatchdogMaxViolations limit). - if (is_disabled_) return ""; std::string watchdog_json = ""; + std::string watchdog_json_filtered = ""; starboard::ScopedLock scoped_lock(mutex_); @@ -538,19 +539,65 @@ std::string Watchdog::GetWatchdogViolations(bool clear) { read_file.ReadAll(buffer.data(), kFileSize); watchdog_json = std::string(buffer.data()); - // Removes all Watchdog violations. - if (clear) { - if (violations_map_) { - static_cast(violations_map_.get())->Clear(); - violations_count_ = 0; + // Get all Watchdog violations in clients is given. + if (client_num == 0) { + // Removes all Watchdog violations. + if (clear) { + if (violations_map_) { + static_cast(violations_map_.get())->Clear(); + violations_count_ = 0; + } + starboard::SbFileDeleteRecursive(GetWatchdogFilePath().c_str(), true); + } + watchdog_json_filtered = watchdog_json; + } else { + std::string watchdog_json_not_read = ""; + std::unique_ptr violations_map = + base::JSONReader::Read(watchdog_json); + base::Value filtered_client_data(base::Value::Type::DICTIONARY); + for (int i = 0; i < client_num; i++) { + base::Value* violation_dict = violations_map->FindKey(clients[i]); + if (violation_dict != nullptr) { + filtered_client_data.SetKey(clients[i], (*violation_dict).Clone()); + if (clear) { + violations_map->RemoveKey(clients[i]); + if (violations_map_) { + bool result = + static_cast(violations_map_.get()) + ->RemoveKey(clients[i]); + if (result) { + violations_count_ -= 1; + } + if (violations_count_ == 0) { + static_cast(violations_map_.get()) + ->Clear(); + } + } + } + } + } + if (!filtered_client_data.DictEmpty()) { + base::JSONWriter::Write(filtered_client_data, &watchdog_json_filtered); + } + if (clear) { + starboard::SbFileDeleteRecursive(GetWatchdogFilePath().c_str(), true); + if (!violations_map->DictEmpty()) { + base::JSONWriter::Write(*violations_map, &watchdog_json_not_read); + + starboard::ScopedFile write_file(GetWatchdogFilePath().c_str(), + kSbFileCreateAlways | kSbFileWrite); + write_file.WriteAll(watchdog_json_not_read.c_str(), + static_cast(watchdog_json_not_read.size())); + time_last_written_microseconds_ = SbTimeGetMonotonicNow(); + } } - starboard::SbFileDeleteRecursive(GetWatchdogFilePath().c_str(), true); } - SB_LOG(INFO) << "[Watchdog] Reading violations:\n" << watchdog_json; + SB_LOG(INFO) << "[Watchdog] Reading violations:\n" + << watchdog_json_filtered; } else { SB_LOG(INFO) << "[Watchdog] No violations."; } - return watchdog_json; + return watchdog_json_filtered; } bool Watchdog::GetPersistentSettingWatchdogEnable() { diff --git a/cobalt/watchdog/watchdog.h b/cobalt/watchdog/watchdog.h index b689e867e0d0..2906d48e04ca 100644 --- a/cobalt/watchdog/watchdog.h +++ b/cobalt/watchdog/watchdog.h @@ -89,7 +89,8 @@ class Watchdog : public Singleton { bool Unregister(const std::string& name, bool lock = true); bool Ping(const std::string& name); bool Ping(const std::string& name, const std::string& info); - std::string GetWatchdogViolations(bool clear = true); + std::string GetWatchdogViolations(const char* clients[] = {}, int client_num = 0, + bool clear = true); bool GetPersistentSettingWatchdogEnable(); void SetPersistentSettingWatchdogEnable(bool enable_watchdog); bool GetPersistentSettingWatchdogCrash(); diff --git a/cobalt/watchdog/watchdog_test.cc b/cobalt/watchdog/watchdog_test.cc index a8ba9f540324..11c3f9435a6d 100644 --- a/cobalt/watchdog/watchdog_test.cc +++ b/cobalt/watchdog/watchdog_test.cc @@ -236,7 +236,7 @@ TEST_F(WatchdogTest, RedundantViolationsShouldStack) { base::kApplicationStateStarted, kWatchdogMonitorFrequency)); SbThreadSleep(kWatchdogSleepDuration); - std::string json = watchdog_->GetWatchdogViolations(false); + std::string json = watchdog_->GetWatchdogViolations({}, 0, false); ASSERT_NE(json, ""); std::unique_ptr uncleared_violations_map = base::JSONReader::Read(json); @@ -253,7 +253,7 @@ TEST_F(WatchdogTest, RedundantViolationsShouldStack) { .FindKey("violationDurationMilliseconds") ->GetString()); SbThreadSleep(kWatchdogSleepDuration); - json = watchdog_->GetWatchdogViolations(false); + json = watchdog_->GetWatchdogViolations({}, 0, false); ASSERT_NE(json, ""); std::unique_ptr violations_map = base::JSONReader::Read(json); ASSERT_NE(violations_map, nullptr); @@ -333,7 +333,7 @@ TEST_F(WatchdogTest, ViolationsAreEvictedAfterMax) { kWatchdogMonitorFrequency)); SbThreadSleep(kWatchdogSleepDuration); - json = watchdog_->GetWatchdogViolations(false); + json = watchdog_->GetWatchdogViolations({}, 0, false); ASSERT_NE(json, ""); std::unique_ptr uncleared_violations_map = base::JSONReader::Read(json); @@ -488,5 +488,70 @@ TEST_F(WatchdogTest, FrequentConsecutiveViolationsShouldNotWrite) { ASSERT_NE(write_json, json); } +TEST_F(WatchdogTest, GetPartialViolationsByClients) { + ASSERT_TRUE(watchdog_->Register("test-name-1", "test-desc-1", + base::kApplicationStateStarted, + kWatchdogMonitorFrequency)); + ASSERT_TRUE(watchdog_->Register("test-name-2", "test-desc-2", + base::kApplicationStateStarted, + kWatchdogMonitorFrequency)); + ASSERT_TRUE(watchdog_->Register("test-name-3", "test-desc-3", + base::kApplicationStateStarted, + kWatchdogMonitorFrequency)); + SbThreadSleep(kWatchdogSleepDuration); + ASSERT_TRUE(watchdog_->Unregister("test-name-1")); + ASSERT_TRUE(watchdog_->Unregister("test-name-2")); + ASSERT_TRUE(watchdog_->Unregister("test-name-3")); + const char* clients[] = {"test-name-1"}; + std::string json = watchdog_->GetWatchdogViolations(clients, 1); + ASSERT_NE(json, ""); + std::unique_ptr violations_map = base::JSONReader::Read(json); + ASSERT_NE(violations_map, nullptr); + base::Value* violation_dict = violations_map->FindKey("test-name-1"); + ASSERT_NE(violation_dict, nullptr); + violation_dict = violations_map->FindKey("test-name-2"); + ASSERT_EQ(violation_dict, nullptr); + violation_dict = violations_map->FindKey("test-name-3"); + ASSERT_EQ(violation_dict, nullptr); + + std::string file_json = ""; + starboard::ScopedFile read_file(watchdog_->GetWatchdogFilePath().c_str(), + kSbFileOpenOnly | kSbFileRead); + if (read_file.IsValid()) { + int64_t kFileSize = read_file.GetSize(); + std::vector buffer(kFileSize + 1, 0); + read_file.ReadAll(buffer.data(), kFileSize); + file_json = std::string(buffer.data()); + } + ASSERT_NE(file_json, ""); + LOG(INFO) << "file_json" << file_json; + violations_map = base::JSONReader::Read(file_json); + ASSERT_NE(violations_map, nullptr); + violation_dict = violations_map->FindKey("test-name-2"); + ASSERT_NE(violation_dict, nullptr); + violation_dict = violations_map->FindKey("test-name-3"); + ASSERT_NE(violation_dict, nullptr); + violation_dict = violations_map->FindKey("test-name-1"); + ASSERT_EQ(violation_dict, nullptr); + + json = watchdog_->GetWatchdogViolations(clients, 1); + ASSERT_EQ(json, ""); + + const char* clients2[] = {"test-name-2", "test-name-3"}; + json = watchdog_->GetWatchdogViolations(clients2, 2); + ASSERT_NE(json, ""); + violations_map = base::JSONReader::Read(json); + ASSERT_NE(violations_map, nullptr); + violation_dict = violations_map->FindKey("test-name-1"); + ASSERT_EQ(violation_dict, nullptr); + violation_dict = violations_map->FindKey("test-name-2"); + ASSERT_NE(violation_dict, nullptr); + violation_dict = violations_map->FindKey("test-name-3"); + ASSERT_NE(violation_dict, nullptr); + starboard::ScopedFile read_file_again(watchdog_->GetWatchdogFilePath().c_str(), + kSbFileOpenOnly | kSbFileRead); + ASSERT_EQ(read_file_again.IsValid(), false); +} + } // namespace watchdog } // namespace cobalt