[chromedriver] Prevent GetLog from returning more than 100,000 at a time.

chrome/test/chromedriver/logging.cc

Index: chrome/test/chromedriver/logging.cc
diff --git a/chrome/test/chromedriver/logging.cc b/chrome/test/chromedriver/logging.cc
index afd73754ca19963be1f6f74e75d15fdb070a88a6..c3453e55093d907765bde3c7336cb10a3c161207 100644
--- a/chrome/test/chromedriver/logging.cc
+++ b/chrome/test/chromedriver/logging.cc
@@ -130,6 +130,7 @@ bool HandleLogMessage(int severity,
 const char WebDriverLog::kBrowserType[] = "browser";
 const char WebDriverLog::kDriverType[] = "driver";
 const char WebDriverLog::kPerformanceType[] = "performance";
+const size_t WebDriverLog::kMaxReturnedEntries = 100000;
 
 bool WebDriverLog::NameToLevel(const std::string& name, Log::Level* out_level) {
   for (size_t i = 0; i < arraysize(kNameToLevel); ++i) {
@@ -151,8 +152,22 @@ WebDriverLog::~WebDriverLog() {
 }
 
 std::unique_ptr<base::ListValue> WebDriverLog::GetAndClearEntries() {
-  std::unique_ptr<base::ListValue> ret(entries_.release());
-  entries_.reset(new base::ListValue());
+  std::unique_ptr<base::ListValue> ret(new base::ListValue());
+  if (entries_->GetSize() <= WebDriverLog::kMaxReturnedEntries) {
+    std::swap(entries_, ret);
+  } else {
+    // net::HttpServer has a maximum buffer size of 100MB. If there are too many
+    // log entries, we'll fail to send them all back in one go. So we only
+    // return |kMaxReturnedEntries| entries at a time.
+    // TODO(crbug.com/681892): Add streaming support to net::HttpServer, so that
+    // we don't have to worry about its buffer size.
+    for (size_t i = 0; i < kMaxReturnedEntries; i++) {

[stgao, 2017/01/20 03:40:12]

Maybe leave a TODO to revisit whether it is possible to avoid a for loop like
this by modifying std::vector-based ListValue.

[samuong, 2017/01/23 18:09:24]

Done.

+      std::unique_ptr<base::Value> entry;
+      if (!entries_->Remove(0, &entry))

[klm, 2017/01/19 20:47:30]

I wonder if there is a race between this and the event listener that adds events
as they come in from the browser... This loop could be relatively long-running,
increasing the chance of an actual race. Or is there an implicit sequencing
between this method and the code paths that can add, e.g. because they always
run on the same thread? If they may run on different threads, is ListValue
thread safe?

[samuong, 2017/01/19 21:15:36]

I don't think there is a race condition - the WebDriverLogs are owned by the
Session object, which is thread local. Shuotao can correct me if I'm wrong.

[stgao, 2017/01/20 03:40:12]

IIRC, each session has its own thread.

[samuong, 2017/01/23 18:09:24]

Done.

+        break;
+      ret->Append(std::move(entry));
+    }
+  }
   return ret;
 }