Cherry pick PR #1182: [Watchdog] Add clients lists as an optional par…

…ameter to GetWatchdogViolations() (#1195) Refer to the original PR: #1182 b/287302949 Co-authored-by: Sherry Zhou <[email protected]>
youtube · Aug 9, 2023 · 5b4f70e · 5b4f70e
1 parent 7189506
commit 5b4f70e
Show file tree

Hide file tree

Showing 6 changed files with 152 additions and 23 deletions.
diff --git a/cobalt/h5vcc/h5vcc_crash_log.cc b/cobalt/h5vcc/h5vcc_crash_log.cc
@@ -15,7 +15,9 @@
 #include "cobalt/h5vcc/h5vcc_crash_log.h"
 
 #include <map>
+#include <memory>
 #include <string>
+#include <vector>
 
 #include "base/atomicops.h"
 #include "base/memory/singleton.h"
@@ -171,9 +173,21 @@ bool H5vccCrashLog::Ping(const std::string& name,
   return false;
 }
 
-std::string H5vccCrashLog::GetWatchdogViolations() {
+std::string H5vccCrashLog::GetWatchdogViolations(
+    const script::Sequence<std::string>& 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::vector<std::string> client_names;
+    for (script::Sequence<std::string>::size_type i = 0; i < clients.size();
+         ++i) {
+      client_names.push_back(clients.at(i).c_str());
+    }
+    return watchdog->GetWatchdogViolations(client_names);
+  }
   return "";
 }
 

diff --git 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<std::string>& clients = {});
 
   bool GetPersistentSettingWatchdogEnable();
 

diff --git 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<DOMString> 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
@@ -517,39 +517,87 @@ 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 std::vector<std::string>& clients, 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_fetched = "";
 
   starboard::ScopedLock scoped_lock(mutex_);
 
   if (pending_write_) WriteWatchdogViolations();
 
-  starboard::ScopedFile read_file(GetWatchdogFilePath().c_str(),
-                                  kSbFileOpenOnly | kSbFileRead);
-  if (read_file.IsValid()) {
-    int64_t kFileSize = read_file.GetSize();
+  starboard::ScopedFile file(GetWatchdogFilePath().c_str(),
+                             kSbFileOpenOnly | kSbFileRead | kSbFileWrite);
+  if (file.IsValid()) {
+    int64_t kFileSize = file.GetSize();
     std::vector<char> buffer(kFileSize + 1, 0);
-    read_file.ReadAll(buffer.data(), kFileSize);
+    file.ReadAll(buffer.data(), kFileSize);
     watchdog_json = std::string(buffer.data());
 
-    // Removes all Watchdog violations.
-    if (clear) {
-      if (violations_map_) {
-        static_cast<base::DictionaryValue*>(violations_map_.get())->Clear();
-        violations_count_ = 0;
+    // If clients is empty we will fetch all clients.
+    if (clients.empty()) {
+      if (clear) {
+        if (violations_map_) {
+          static_cast<base::DictionaryValue*>(violations_map_.get())->Clear();
+          violations_count_ = 0;
+        }
+        starboard::SbFileDeleteRecursive(GetWatchdogFilePath().c_str(), true);
+      }
+      watchdog_json_fetched = watchdog_json;
+    } else {
+      std::string watchdog_json_not_read = "";
+      std::unique_ptr<base::Value> violations_map =
+          base::JSONReader::Read(watchdog_json);
+      base::Value filtered_client_data(base::Value::Type::DICTIONARY);
+      for (int i = 0; i < clients.size(); 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) {
+            base::Value* violations = violation_dict->FindKey("violations");
+            int violations_count = violations->GetList().size();
+            violations_map->RemoveKey(clients[i]);
+            if (violations_map_) {
+              bool result =
+                  static_cast<base::DictionaryValue*>(violations_map_.get())
+                      ->RemoveKey(clients[i]);
+              if (result) {
+                violations_count_ -= violations_count;
+              }
+              if (violations_count_ == 0) {
+                static_cast<base::DictionaryValue*>(violations_map_.get())
+                    ->Clear();
+              }
+            }
+          }
+        }
+      }
+      if (!filtered_client_data.DictEmpty()) {
+        base::JSONWriter::Write(filtered_client_data, &watchdog_json_fetched);
+      }
+      if (clear) {
+        // If all data is fetched, delete the violation file.
+        if (violations_map->DictEmpty()) {
+          starboard::SbFileDeleteRecursive(GetWatchdogFilePath().c_str(), true);
+        } else {
+          base::JSONWriter::Write(*violations_map, &watchdog_json_not_read);
+          file.Seek(kSbFileFromBegin, 0);
+          file.WriteAll(watchdog_json_not_read.c_str(),
+                        static_cast<int>(watchdog_json_not_read.size()));
+          file.Truncate(static_cast<int>(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_fetched;
   } else {
     SB_LOG(INFO) << "[Watchdog] No violations.";
   }
-  return watchdog_json;
+  return watchdog_json_fetched;
 }
 
 bool Watchdog::GetPersistentSettingWatchdogEnable() {

diff --git a/cobalt/watchdog/watchdog.h b/cobalt/watchdog/watchdog.h
@@ -18,6 +18,7 @@
 #include <memory>
 #include <string>
 #include <unordered_map>
+#include <vector>
 
 #include "base/values.h"
 #include "cobalt/base/application_state.h"
@@ -89,7 +90,8 @@ class Watchdog : public Singleton<Watchdog> {
   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 std::vector<std::string>& clients = {}, 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
@@ -235,7 +235,7 @@ TEST_F(WatchdogTest, RedundantViolationsShouldStack) {
                                   base::kApplicationStateStarted,
                                   kWatchdogMonitorFrequency));
   SbThreadSleep(kWatchdogSleepDuration);
-  std::string json = watchdog_->GetWatchdogViolations(false);
+  std::string json = watchdog_->GetWatchdogViolations({}, false);
   ASSERT_NE(json, "");
   std::unique_ptr<base::Value> uncleared_violations_map =
       base::JSONReader::Read(json);
@@ -252,7 +252,7 @@ TEST_F(WatchdogTest, RedundantViolationsShouldStack) {
                      .FindKey("violationDurationMilliseconds")
                      ->GetString());
   SbThreadSleep(kWatchdogSleepDuration);
-  json = watchdog_->GetWatchdogViolations(false);
+  json = watchdog_->GetWatchdogViolations({}, false);
   ASSERT_NE(json, "");
   std::unique_ptr<base::Value> violations_map = base::JSONReader::Read(json);
   ASSERT_NE(violations_map, nullptr);
@@ -332,7 +332,7 @@ TEST_F(WatchdogTest, ViolationsAreEvictedAfterMax) {
                                   kWatchdogMonitorFrequency));
   SbThreadSleep(kWatchdogSleepDuration);
 
-  json = watchdog_->GetWatchdogViolations(false);
+  json = watchdog_->GetWatchdogViolations({}, false);
   ASSERT_NE(json, "");
   std::unique_ptr<base::Value> uncleared_violations_map =
       base::JSONReader::Read(json);
@@ -487,5 +487,69 @@ 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 std::vector<std::string> clients = {"test-name-1"};
+  std::string json = watchdog_->GetWatchdogViolations(clients);
+  ASSERT_NE(json, "");
+  std::unique_ptr<base::Value> 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<char> buffer(kFileSize + 1, 0);
+    read_file.ReadAll(buffer.data(), kFileSize);
+    file_json = std::string(buffer.data());
+  }
+  ASSERT_NE(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);
+  ASSERT_EQ(json, "");
+
+  const std::vector<std::string> clients2 = {"test-name-2", "test-name-3"};
+  json = watchdog_->GetWatchdogViolations(clients2);
+  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