Write a log list for uploaded WebRTC logs.

chrome/browser/media/webrtc_log_uploader.cc

Index: chrome/browser/media/webrtc_log_uploader.cc
diff --git a/chrome/browser/media/webrtc_log_uploader.cc b/chrome/browser/media/webrtc_log_uploader.cc
index f88e3821ada4ca34a3d27a3a5bc6fed882169957..4d2f533738d1aa9ad533e177e8a9fed8bbc84496 100644
--- a/chrome/browser/media/webrtc_log_uploader.cc
+++ b/chrome/browser/media/webrtc_log_uploader.cc
@@ -4,9 +4,16 @@
 
 #include "chrome/browser/media/webrtc_log_uploader.h"
 
+#include "base/files/file_path.h"
 #include "base/logging.h"
+#include "base/path_service.h"
 #include "base/shared_memory.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/strings/string_split.h"
 #include "base/strings/stringprintf.h"
+#include "base/time/time.h"
+#include "chrome/browser/media/webrtc_log_upload_list.h"
+#include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_version_info.h"
 #include "chrome/common/partial_circular_buffer.h"
 #include "content/public/browser/browser_thread.h"
@@ -24,6 +31,7 @@ namespace {
 
 const int kLogCountLimit = 5;
 const uint32 kIntermediateCompressionBufferBytes = 256 * 1024;  // 256 KB
+const int kLogListLimitLines = 50;
 
 const char kUploadURL[] = "https://clients2.google.com/cr/report";
 const char kUploadContentType[] = "multipart/form-data";
@@ -32,8 +40,9 @@ const char kMultipartBoundary[] =
 
 }  // namespace
 
-WebRtcLogUploader::WebRtcLogUploader()
-    : log_count_(0) {
+WebRtcLogUploader::WebRtcLogUploader(base::FilePath& upload_list_path)
+    : log_count_(0),
+      upload_list_path_(upload_list_path) {
 }
 
 WebRtcLogUploader::~WebRtcLogUploader() {
@@ -41,6 +50,10 @@ WebRtcLogUploader::~WebRtcLogUploader() {
 
 void WebRtcLogUploader::OnURLFetchComplete(
     const net::URLFetcher* source) {
+  int response_code = source->GetResponseCode();
+  std::string report_id;
+  if (response_code == 200 && source->GetResponseAsString(&report_id))
+    AddUploadedLogInfoToUploadListFile(report_id);
 }
 
 void WebRtcLogUploader::OnURLFetchUploadProgress(
@@ -196,3 +209,66 @@ void WebRtcLogUploader::DecreaseLogCount() {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   --log_count_;
 }
+
+void WebRtcLogUploader::AddUploadedLogInfoToUploadListFile(

[tommi (sloooow) - chröme, 2013/07/01 12:01:01]

Since there's no locking in this class can we add thread checks that all of
these calls happen on the same thread?

[Henrik Grunell, 2013/07/01 13:26:22]

As discussed offline, not done. Problems for test. Only data to protect is
upload_list_path_ which is accessed in ctor and here.

+    const std::string& report_id) {
+  if (upload_list_path_.empty()) {
+    base::FilePath log_dir_path;
+    PathService::Get(chrome::DIR_USER_DATA, &log_dir_path);
+    upload_list_path_ =
+        log_dir_path.AppendASCII(WebRtcLogUploadList::kWebRtcLogListFilename);
+  }
+
+  int flags = base::PLATFORM_FILE_OPEN_ALWAYS |
+              base::PLATFORM_FILE_READ |
+              base::PLATFORM_FILE_WRITE;
+  base::PlatformFileError error = base::PLATFORM_FILE_OK;
+  base::PlatformFile logs_uploaded_file =
+      base::CreatePlatformFile(upload_list_path_, flags, NULL, &error);
+  if (error != base::PLATFORM_FILE_OK) {
+    LOG(WARNING) << "Could not open WebRTC log list file.";

[tommi (sloooow) - chröme, 2013/07/01 12:01:01]

nit: also include the error value

[Henrik Grunell, 2013/07/01 13:26:22]

Done.

+    return;
+  }
+
+  // Read the log list file. Each line is ~35 chars (bytes). Use a buffer size
+  // of 3 times the estimated max size to have a margin.
+  std::string contents(3 * kLogListLimitLines * 35, '\0');
+  size_t read = base::ReadPlatformFileAtCurrentPos(logs_uploaded_file,
+                                                   &contents[0],
+                                                   contents.size());
+  DCHECK_LT(read, contents.size());

[tommi (sloooow) - chröme, 2013/07/01 12:01:01]

isn't it possible that read can be equal to content.size()?

[Henrik Grunell, 2013/07/01 13:26:22]

Is should be ~ a factor 3 smaller, but I can check for LE instead if you think?
Equal is OK in terms of not writing out of bounds.

+  contents.resize(read);
+
+  // Limit the number of log entries to |kLogListLimitLines| - 1, to make room
+  // for the new entry. Each line including the last ends with a '\n', so hit
+  // n will be before line n-1 (from the back).
+  if (!contents.empty()) {

[tommi (sloooow) - chröme, 2013/07/01 12:01:01]

nit: this condition isn't really necessary

[Henrik Grunell, 2013/07/01 13:26:22]

That's right. Removed.

+    int lf_count = 0;
+    int i = contents.size() - 1;
+    for (; i >= 0 && lf_count < kLogListLimitLines; --i) {
+      if (contents[i] == '\n')
+        ++lf_count;
+    }
+    if (lf_count <= kLogListLimitLines) {

[tommi (sloooow) - chröme, 2013/07/01 12:01:01]

shouldn't this be >= ?  Right now it looks like you'll always delete everything
otherwise and if the file is too big you don't trim it.

[Henrik Grunell, 2013/07/01 13:26:22]

Yes it should, thanks.

+      // + 1 to compensate for the for loop decrease before the conditional
+      // check and + 1 to get the length.
+      contents.erase(0, i + 2);
+    }
+  }
+
+  // Write the Unix time and report ID to the log list file.
+  base::Time time_now = base::Time::Now();
+  contents += base::DoubleToString(time_now.ToDoubleT()) +
+              "," + report_id + '\n';
+
+  base::SeekPlatformFile(logs_uploaded_file, base::PLATFORM_FILE_FROM_BEGIN, 0);
+  size_t written = base::WritePlatformFileAtCurrentPos(logs_uploaded_file,
+                                                       contents.c_str(),
+                                                       contents.size());
+  DCHECK_EQ(written, contents.size());
+  bool ret = base::TruncatePlatformFile(logs_uploaded_file, written);
+  DPCHECK(ret);
+
+  ret = base::ClosePlatformFile(logs_uploaded_file);
+  DPCHECK(ret);
+}