Implementing uploading of a WebRTC diagnostic log.

components/webrtc_log_uploader/webrtc_log_uploader.cc

Index: components/webrtc_log_uploader/webrtc_log_uploader.cc
diff --git a/components/webrtc_log_uploader/webrtc_log_uploader.cc b/components/webrtc_log_uploader/webrtc_log_uploader.cc
new file mode 100644
index 0000000000000000000000000000000000000000..fbba960d1b481262788636cc241578fb30e823ab
--- /dev/null
+++ b/components/webrtc_log_uploader/webrtc_log_uploader.cc
@@ -0,0 +1,299 @@
+// Copyright 2013 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 "components/webrtc_log_uploader/webrtc_log_uploader.h"
+
+#if defined(USE_SYSTEM_LIBBZ2)

[Jói, 2013/05/14 21:30:29]

nit: I think conditional includes usually come after the normal includes

[Henrik Grunell, 2013/05/15 20:17:53]

I've replaced bzlib with zlib; the former has been removed from Chromium.

+#include <bzlib.h>
+#else
+#include "third_party/bzip2/bzlib.h"
+#endif
+
+#include "base/file_util.h"
+#include "base/logging.h"
+#include "base/partial_circular_buffer.h"
+#include "base/shared_memory.h"
+#include "base/stringprintf.h"
+#include "content/public/browser/browser_thread.h"
+#include "net/base/network_delegate.h"
+#include "net/proxy/proxy_config.h"
+#include "net/proxy/proxy_config_service.h"
+#include "net/url_request/url_fetcher.h"
+#include "net/url_request/url_request_context.h"
+#include "net/url_request/url_request_context_builder.h"
+#include "net/url_request/url_request_context_getter.h"
+
+namespace components {
+
+namespace {
+
+const int kLogCountLimit = 5;
+const uint32 kIntermediateCompressionBufferSize = 256 * 1024;  // 256 KB

[Jói, 2013/05/14 21:30:29]

Suggest renaming to kIntermediateCompressionBufferBytes to make the unit obvious
from the name.

[Henrik Grunell, 2013/05/15 20:17:53]

Done.

+
+const char kUploadURL[] = "https://clients2.google.com/cr/report";
+const char kUploadContentType[] = "multipart/form-data";
+const char kMultipartBoundary[] =
+    "----**--yradnuoBgoLtrapitluMklaTelgooG--**----";
+
+// Config getter that always returns direct settings.

[Jói, 2013/05/14 21:30:29]

Can you document the motivation for this? I see it's used only for Linux and
Android below.

[Henrik Grunell, 2013/05/15 20:17:53]

I'll double check with someone who knows net, background is I got a DCHECK fail
in ProxyService::CreateUsingSystemProxyResolver (when starting the fetcher I
think), and the function to set the proxy config service only exists on linux
and android. I'll update comment (or code) after getting clearer on this.

[Henrik Grunell, 2013/05/24 13:01:50]

I've changed it so that it uses the system context request getter. (I checked
with chrome-network-stack@ about this.)

+class ProxyConfigServiceDirect : public net::ProxyConfigService {
+ public:
+  // ProxyConfigService implementation.
+  virtual void AddObserver(Observer* observer) OVERRIDE {}
+  virtual void RemoveObserver(Observer* observer) OVERRIDE {}
+  virtual ConfigAvailability GetLatestProxyConfig(
+      net::ProxyConfig* config) OVERRIDE {
+    *config = net::ProxyConfig::CreateDirect();
+    return CONFIG_VALID;
+  }
+};
+
+}  // namespace
+
+class WebRtcLogURLRequestContextGetter : public net::URLRequestContextGetter {
+ public:
+  WebRtcLogURLRequestContextGetter() {}
+
+  // net::URLRequestContextGetter implementation.
+  virtual net::URLRequestContext* GetURLRequestContext() OVERRIDE {
+    DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
+
+    if (!url_request_context_) {
+      net::URLRequestContextBuilder builder;
+#if defined(OS_LINUX) || defined(OS_ANDROID)
+      builder.set_proxy_config_service(new ProxyConfigServiceDirect());
+#endif
+      url_request_context_.reset(builder.Build());
+      CHECK(url_request_context_.get());
+    }
+
+    return url_request_context_.get();
+  }
+
+  virtual scoped_refptr<base::SingleThreadTaskRunner>
+      GetNetworkTaskRunner() const OVERRIDE {
+    return content::BrowserThread::GetMessageLoopProxyForThread(
+        content::BrowserThread::IO);
+  }
+
+ private:
+  virtual ~WebRtcLogURLRequestContextGetter() {}
+
+  // NULL if not yet initialized. Otherwise, it is the URLRequestContext
+  // instance that was lazily created by GetURLRequestContext().
+  // Access only from the IO thread.
+  scoped_ptr<net::URLRequestContext> url_request_context_;
+
+  DISALLOW_COPY_AND_ASSIGN(WebRtcLogURLRequestContextGetter);
+};
+
+
+WebRtcLogUploader::WebRtcLogUploader()
+    : log_count_(0) {
+}
+
+WebRtcLogUploader::~WebRtcLogUploader() {
+}
+
+void WebRtcLogUploader::OnURLFetchComplete(
+    const net::URLFetcher* source) {
+  std::vector<net::URLFetcher*>::iterator it;
+  for (it = url_fetchers_.begin(); it != url_fetchers_.end(); ++it) {
+    if (*it == source) {
+      url_fetchers_.erase(it);
+      break;
+    }
+  }
+  delete source;

[Jói, 2013/05/14 21:30:29]

Why are you deleting the object?  I don't think ownership has been passed to
this method, at least I can't find documentation of that in URLFetcherDelegate.

[Henrik Grunell, 2013/05/15 20:17:53]

You're right, I think I got that from a unit test in a weak moment. Removed that
and also |url_fetchers_| which I used (or planned to use) for something before
but not anymore.

+}
+
+void WebRtcLogUploader::OnURLFetchUploadProgress(
+    const net::URLFetcher* source, int64 current, int64 total) {
+}
+
+bool WebRtcLogUploader::ApplyForStartLogging() {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+  if (log_count_ < kLogCountLimit) {
+    ++log_count_;
+    return true;
+  }
+  return false;
+}
+
+void WebRtcLogUploader::UploadLog(scoped_ptr<base::SharedMemory> shared_memory,
+                                  uint32 length) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::FILE));
+  DCHECK(shared_memory);
+  DCHECK(shared_memory->memory());
+
+  base::FilePath upload_file_path;
+  if (file_util::CreateTemporaryFile(&upload_file_path)) {
+    SetupMultipartFile(reinterpret_cast<uint8*>(shared_memory->memory()),
+                       length, upload_file_path);
+
+    if (!request_context_getter_)
+      request_context_getter_ = new WebRtcLogURLRequestContextGetter();
+
+    std::string content_type = kUploadContentType;
+    content_type.append("; boundary=");
+    content_type.append(kMultipartBoundary);
+
+    net::URLFetcher* url_fetcher =
+        net::URLFetcher::Create(GURL(kUploadURL), net::URLFetcher::POST, this);
+    url_fetcher->SetRequestContext(request_context_getter_);
+    url_fetcher->SetUploadFilePath(
+        content_type, upload_file_path, 0, kuint64max,
+        content::BrowserThread::GetMessageLoopProxyForThread(
+            content::BrowserThread::FILE));
+    url_fetcher->Start();
+    url_fetchers_.push_back(url_fetcher);
+  } else {
+    DLOG(ERROR) << "Could not create temporary file";

[Jói, 2013/05/14 21:30:29]

Suggest a more informative message, i.e. that it's the WebRtcLogUploader and
perhaps the path you tried to create.

[Henrik Grunell, 2013/05/15 20:17:53]

Done.

+  }
+
+  content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE,
+      base::Bind(&WebRtcLogUploader::DecreaseLogCount, base::Unretained(this)));
+}
+
+void WebRtcLogUploader::SetupMultipartFile(
+    uint8* log_buffer, uint32 log_buffer_length,
+    const base::FilePath& upload_file_path) {
+  int flags = base::PLATFORM_FILE_CREATE_ALWAYS |
+              base::PLATFORM_FILE_WRITE;
+  bool created = false;
+  base::PlatformFileError error = base::PLATFORM_FILE_OK;
+  base::PlatformFile multipart_file =
+      CreatePlatformFile(upload_file_path, flags, &created, &error);
+  DCHECK(created);
+  DCHECK(error == base::PLATFORM_FILE_OK);

[Jói, 2013/05/14 21:30:29]

DCHECK_EQ

[Henrik Grunell, 2013/05/15 20:17:53]

Done.

+
+  // TODO(grunell): Correct product name.

[Jói, 2013/05/14 21:30:29]

Is this TODO (and the one 2 lines down) for the current change, or a follow-up?

[Henrik Grunell, 2013/05/15 20:17:53]

I was thinking for the current change, but it can be made in a follow-up CL if
that would make reviewers' life easier.

[Henrik Grunell, 2013/05/24 13:01:50]

It's done now.

+  AddPairString(multipart_file, "prod", "Chrome");
+  // TODO(grunell): Correct version.
+  AddPairString(multipart_file, "ver", "0.0.1.1-dev-test");
+  AddPairString(multipart_file, "guid", "0");
+  AddPairString(multipart_file, "type", "log");
+
+  AddUrlChunks();
+
+  AddLogData(multipart_file, log_buffer, log_buffer_length);
+
+  std::string str = "--";
+  str.append(kMultipartBoundary);
+  str.append("--\r\n");
+  base::WritePlatformFileAtCurrentPos(multipart_file, str.c_str(), str.size());
+
+  DCHECK(base::ClosePlatformFile(multipart_file));
+}
+
+void WebRtcLogUploader::AddPairString(base::PlatformFile multipart_file,
+                                      const std::string& key,
+                                      const std::string& value) {
+  std::string str;
+  AddMultipartValueForUpload(key, value, kMultipartBoundary, "", &str);
+  base::WritePlatformFileAtCurrentPos(multipart_file, str.c_str(), str.size());
+}
+
+void WebRtcLogUploader::AddUrlChunks() {
+  // TODO(grunell): Implement.

[Jói, 2013/05/14 21:30:29]

I'm assuming this is needed for the current change?

[Henrik Grunell, 2013/05/15 20:17:53]

Same as reply above. That's my intention but could be made in follow-up.

[Henrik Grunell, 2013/05/24 13:01:50]

This functions has been removed; not needed.

+}
+
+void WebRtcLogUploader::AddLogData(base::PlatformFile multipart_file,
+                                   uint8* log_buffer,
+                                   uint32 log_buffer_length) {
+  std::string str = "--";
+  str.append(kMultipartBoundary);
+  str.append("\r\n");
+  str.append("Content-Disposition: form-data; name=\"log\"");
+  str.append("; filename=\"log.bz2\"\r\n");
+  str.append("Content-Type: application/x-bzip\r\n\r\n");
+  base::WritePlatformFileAtCurrentPos(multipart_file, str.c_str(), str.size());

[Jói, 2013/05/14 21:30:29]

I have to wonder, since the log_buffer is in memory and makes up the bulk of the
file that is uploaded, do you really need to write to a file and then upload the
file? Why not just do everything in memory and then use the std::string version
of URLFetcher::SetUploadData? That seems slightly less prone to error and the
code would possibly be a little bit simpler.

[Henrik Grunell, 2013/05/15 20:17:53]

Very valid point. Here's why I chose a file: after this CL, I will add the
possibility for the user to upload logs manually afterwards, for the case where
the user setting doesn't allow automatic upload. Last x logs will be kept on the
disk. So my plan was to prepare for that. I could use memory now and change
later if you wish, but it would mean double work.

[Henrik Grunell, 2013/05/24 13:01:50]

Changed to writing to memory instead.

+
+  CompressLog(log_buffer, log_buffer_length, multipart_file);
+
+  std::string end_str = "\r\n";
+  base::WritePlatformFileAtCurrentPos(
+      multipart_file, end_str.c_str(), end_str.size());
+}
+
+// TODO(grunell): This is copied from cloud_print_helpers.cc. Break out to its
+// own file.
+void WebRtcLogUploader::AddMultipartValueForUpload(
+    const std::string& value_name, const std::string& value,
+    const std::string& mime_boundary, const std::string& content_type,
+    std::string* post_data) {
+  DCHECK(post_data);
+  // First line is the boundary
+  post_data->append("--" + mime_boundary + "\r\n");
+  // Next line is the Content-disposition
+  post_data->append(base::StringPrintf("Content-Disposition: form-data; "
+                   "name=\"%s\"\r\n", value_name.c_str()));
+  if (!content_type.empty()) {
+    // If Content-type is specified, the next line is that
+    post_data->append(base::StringPrintf("Content-Type: %s\r\n",
+                      content_type.c_str()));
+  }
+  // Leave an empty line and append the value.
+  post_data->append(base::StringPrintf("\r\n%s\r\n", value.c_str()));
+}
+
+void WebRtcLogUploader::CompressLog(uint8* input,
+                                    uint32 input_size,
+                                    base::PlatformFile output_file) {
+  base::PartialCircularBuffer read_pcb(input, input_size);
+
+  bz_stream stream = {0};
+  int result = BZ2_bzCompressInit(&stream, 3, 0, 0);
+  DCHECK(result == BZ_OK);
+
+  scoped_ptr<uint8[]> intermediate_buffer(
+      new uint8[kIntermediateCompressionBufferSize]);
+  scoped_ptr<uint8[]> compressed(
+      new uint8[kIntermediateCompressionBufferSize]);
+  memset(intermediate_buffer.get(), 0, kIntermediateCompressionBufferSize);
+  memset(compressed.get(), 0, kIntermediateCompressionBufferSize);
+  uint32 read = read_pcb.Read(intermediate_buffer.get(),
+                              kIntermediateCompressionBufferSize);
+
+  stream.next_in = reinterpret_cast<char*>(&intermediate_buffer.get()[0]);
+  stream.avail_in = read;
+  char* compressed_char_ptr = reinterpret_cast<char*>(&compressed.get()[0]);
+
+  // Keeps track of written output.
+  unsigned int last_total_out = stream.total_out_lo32;
+
+  while (read == kIntermediateCompressionBufferSize) {
+    stream.next_out = compressed_char_ptr;
+    stream.avail_out = kIntermediateCompressionBufferSize;
+    result = BZ2_bzCompress(&stream, BZ_RUN);
+    DCHECK(result == BZ_RUN_OK);
+    base::WritePlatformFileAtCurrentPos(output_file, compressed_char_ptr,
+                                        stream.total_out_lo32 - last_total_out);
+    last_total_out = stream.total_out_lo32;
+    read = read_pcb.Read(intermediate_buffer.get(),
+                         kIntermediateCompressionBufferSize);
+    stream.next_in = reinterpret_cast<char*>(&intermediate_buffer.get()[0]);
+    stream.avail_in = read;
+  }
+
+  do {
+    stream.next_out = compressed_char_ptr;
+    stream.avail_out = kIntermediateCompressionBufferSize;
+    result = BZ2_bzCompress(&stream, BZ_FINISH);
+    base::WritePlatformFileAtCurrentPos(output_file, compressed_char_ptr,
+                                        stream.total_out_lo32 - last_total_out);
+    last_total_out = stream.total_out_lo32;
+  } while (result == BZ_FINISH_OK);
+  DCHECK(result == BZ_STREAM_END);
+
+  result = BZ2_bzCompressEnd(&stream);
+  DCHECK(result == BZ_OK);
+}
+
+void WebRtcLogUploader::DecreaseLogCount() {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+  --log_count_;
+}
+
+}  // namespace components