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

chrome/test/chromedriver/logging.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/test/chromedriver/logging.h"
 
 #include <stddef.h>
 #include <stdint.h>
 #include <stdio.h>
 
@@ skipping 112 matching lines @@
     session_log->AddEntry(level, message);
 
   return true;
 }
 
 }  // namespace
 
 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) {
     if (name == kNameToLevel[i].name) {
       *out_level = kNameToLevel[i].level;
       return true;
     }
   }
   return false;
 }
 
 WebDriverLog::WebDriverLog(const std::string& type, Log::Level min_level)
     : type_(type), min_level_(min_level), entries_(new base::ListValue()) {
 }
 
 WebDriverLog::~WebDriverLog() {
   VLOG(1) << "Log type '" << type_ << "' lost "
           << entries_->GetSize() << " entries on destruction";
 }
 
 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;
 }
 
 std::string WebDriverLog::GetFirstErrorMessage() const {
   for (base::ListValue::iterator it = entries_->begin();
        it != entries_->end();
        ++it) {
     base::DictionaryValue* log_entry = NULL;
     (*it)->GetAsDictionary(&log_entry);
     if (log_entry != NULL) {
@@ skipping 126 matching lines @@
   logs.push_back(browser_log);
   // If the level is OFF, don't even bother listening for DevTools events.
   if (browser_log_level != Log::kOff)
     devtools_listeners.push_back(new ConsoleLogger(browser_log));
 
   out_logs->swap(logs);
   out_devtools_listeners->swap(devtools_listeners);
   out_command_listeners->swap(command_listeners);
   return Status(kOk);
 }