Implement WebRtcEventLogUploaderImpl

This class is in charge of uploading a single WebRTC event log
to the Crash server.

Upcoming related work:
* Update chrome://webrtc-logs
* Fix the known issue where the log is read into memory in its
  entirety prior to initiating the upload.
* Compress the log on disk while writing.

Bug: 775415
Change-Id: Ie5713143b3bc532ac07dd830aecf927698577f18
Reviewed-on: https://chromium-review.googlesource.com/958511Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Tommi <tommi@chromium.org>
Commit-Queue: Elad Alon <eladalon@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543387}

chrome/browser/media/webrtc/webrtc_event_log_manager.cc

@@ -96,6 +96,7 @@ WebRtcEventLogManager::WebRtcEventLogManager()
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   if (base::FeatureList::IsEnabled(::features::kWebRtcRemoteEventLog)) {
+    VLOG(1) << "WebRTC remote-bound event logging enabled.";
     remote_logs_manager_ = std::make_unique<WebRtcRemoteEventLogManager>(this);
   }
 

chrome/browser/media/webrtc/webrtc_event_log_manager_remote.cc

@@ -20,14 +20,15 @@
 #include "base/threading/sequenced_task_runner_handle.h"
 #include "content/public/browser/browser_thread.h"
 
-// TODO(eladalon): Block remote-bound logging on mobile devices.
-// https://crbug.com/775415
+// TODO(crbug.com/775415): Block remote-bound logging on mobile devices.
 
 const size_t kMaxRemoteLogFileMetadataSizeBytes = 0xffffu;  // 65535
 static_assert(kMaxRemoteLogFileMetadataSizeBytes <= 0xFFFFFFu,
               "Only 24 bits available for encoding the metadata's length.");
 
-const size_t kMaxRemoteLogFileSizeBytes = (1u << 29);  // ~500MBs
+// TODO(crbug.com/775415): Change back to (1u << 29) after resolving the issue
+// where we read the entire file into memory.
+const size_t kMaxRemoteLogFileSizeBytes = 50000000u;
 
 namespace {
 const base::FilePath::CharType kRemoteBoundLogSubDirectory[] =
@@ -95,9 +96,9 @@ WebRtcRemoteEventLogManager::WebRtcRemoteEventLogManager(
 
 WebRtcRemoteEventLogManager::~WebRtcRemoteEventLogManager() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  // TODO(eladalon): Purge from disk files which were being uploaded  while
-  // destruction took place, thereby avoiding endless attempts to upload
-  // the same file. https://crbug.com/775415
+  // TODO(crbug.com/775415): Purge from disk files which were being uploaded
+  // while destruction took place, thereby avoiding endless attempts to upload
+  // the same file.
 }
 
 void WebRtcRemoteEventLogManager::EnableForBrowserContext(
@@ -119,7 +120,7 @@ void WebRtcRemoteEventLogManager::EnableForBrowserContext(
   enabled_browser_contexts_.insert(browser_context_id);
 }
 
-// TODO(eladalon): Add unit tests. https://crbug.com/775415
+// TODO(crbug.com/775415): Add unit tests.
 void WebRtcRemoteEventLogManager::DisableForBrowserContext(
     BrowserContextId browser_context_id) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(io_task_sequence_checker_);
@@ -282,9 +283,6 @@ void WebRtcRemoteEventLogManager::OnWebRtcEventLogUploadComplete(
       base::BindOnce(
           &WebRtcRemoteEventLogManager::OnWebRtcEventLogUploadCompleteInternal,
           base::Unretained(this)));
-
-  // TODO(eladalon): Send indication of success/failure back to JS.
-  // https://crbug.com/775415
 }
 
 void WebRtcRemoteEventLogManager::SetWebRtcEventLogUploaderFactoryForTesting(
@@ -344,7 +342,7 @@ bool WebRtcRemoteEventLogManager::MaybeCreateLogsDirectory(
     return false;
   }
 
-  // TODO(eladalon): Test for appropriate permissions. https://crbug.com/775415
+  // TODO(crbug.com/775415): Test for appropriate permissions.
 
   return true;
 }
@@ -389,7 +387,7 @@ bool WebRtcRemoteEventLogManager::StartWritingLog(
   // Randomize a new filename. In the highly unlikely event that this filename
   // is already taken, it will be treated the same way as any other failure
   // to start the log file.
-  // TODO(eladalon): Add a unit test for above comment. https://crbug.com/775415
+  // TODO(crbug.com/775415): Add a unit test for above comment.
   const std::string unique_filename =
       "event_log_" + std::to_string(base::RandUint64());
   const base::FilePath base_path = GetLogsDirectoryPath(browser_context_dir);
@@ -531,11 +529,13 @@ void WebRtcRemoteEventLogManager::MaybeStartUploading() {
 
   // The uploader takes ownership of the file; it's no longer considered to be
   // pending. (If the upload fails, the log will be deleted.)
-  // TODO(eladalon): Add more refined retry behavior, so that we would not
-  // delete the log permanently if the network is just down, on the one hand,
-  // but also would not be uploading unlimited data on endless retries on the
-  // other hand. https://crbug.com/775415
-  // TODO(eladalon): Delay the upload's start. https://crbug.com/814362
+  // TODO(crbug.com/775415): Add more refined retry behavior, so that we would
+  // not delete the log permanently if the network is just down, on the one
+  // hand, but also would not be uploading unlimited data on endless retries on
+  // the other hand.
+  // TODO(crbug.com/814362): Delay the upload's start.
+  // TODO(crbug.com/775415): Rename the file before uploading, so that we would
+  // not retry the upload after restarting Chrome, if the upload is interrupted.
   uploader_ = uploader_factory_->Create(pending_logs_.begin()->path, this);
   pending_logs_.erase(pending_logs_.begin());
 }

chrome/browser/media/webrtc/webrtc_event_log_uploader.cc

@@ -4,36 +4,280 @@
 
 #include "chrome/browser/media/webrtc/webrtc_event_log_uploader.h"
 
+#include "base/bind.h"
 #include "base/files/file_util.h"
 #include "base/logging.h"
+#include "base/threading/sequenced_task_runner_handle.h"
+#include "build/build_config.h"
+#include "chrome/browser/browser_process.h"
+#include "components/version_info/version_info.h"
+#include "content/public/browser/browser_thread.h"
+#include "net/base/load_flags.h"
+#include "net/base/mime_util.h"
+#include "net/http/http_status_code.h"
 
-WebRtcEventLogUploaderImpl::WebRtcEventLogUploaderImpl(
-    const base::FilePath& path,
+// Explanation about the life cycle of a WebRtcEventLogUploaderImpl object, and
+// about why its use of base::Unretained is safe:
+// * WebRtcEventLogUploaderImpl objects are owned (indirectly) by
+//   WebRtcEventLogManager, which is a singleton object that is not destroyed
+//   during Chrome shutdown, but rather, is allowed to leak.
+// * Therefore, objects of type WebRtcEventLogUploaderImpl will only be
+//   destroyed when their owner explicitly decides to do so.
+// * The direct owner, WebRtcRemoteEventLogManager, only deletes a
+//   WebRtcEventLogUploaderImpl after it receives a notification
+//   of type OnWebRtcEventLogUploadComplete.
+// * OnWebRtcEventLogUploadComplete() is only ever called as the last step,
+//   there are no tasks pending which have a reference to this object.
+// * The previous point follows from OnURLFetchComplete being guaranteed to
+//   be the last callback called on a URLFetcherDelegate.
+
+namespace {
+// TODO(crbug.com/817495): Eliminate the duplication with other uploaders.
+const char kUploadContentType[] = "multipart/form-data";
+const char kBoundary[] = "----**--yradnuoBgoLtrapitluMklaTelgooG--**----";
+
+const char kLogFilename[] = "webrtc_event_log";
+const char kLogExtension[] = "log";
+
+constexpr size_t kExpectedMimeOverheadBytes = 1000;  // Intentional overshot.
+
+// TODO(crbug.com/817495): Eliminate the duplication with other uploaders.
+#if defined(OS_WIN)
+const char kProduct[] = "Chrome";
+#elif defined(OS_MACOSX)
+const char kProduct[] = "Chrome_Mac";
+#elif defined(OS_LINUX)
+const char kProduct[] = "Chrome_Linux";
+#elif defined(OS_ANDROID)
+const char kProduct[] = "Chrome_Android";
+#elif defined(OS_CHROMEOS)
+const char kProduct[] = "Chrome_ChromeOS";
+#else
+#error Platform not supported.
+#endif
+
+// TODO(crbug.com/775415): Update comment to reflect new policy when discarding
+// the command line flag.
+constexpr net::NetworkTrafficAnnotationTag kTrafficAnnotation =
+    net::DefineNetworkTrafficAnnotation("webrtc_event_log_uploader", R"(
+      semantics {
+        sender: "WebRTC Event Log uploader module"
+        description:
+          "Uploads a WebRTC event log to a server called Crash. These logs "
+          "will not contain private information. They will be used to "
+          "improve WebRTC (fix bugs, tune performance, etc.)."
+        trigger:
+          "A privileged JS application (Hangouts/Meet) has requested a peer "
+          "connection to be logged, and the resulting event log to be "
+          "uploaded at a time deemed to cause the least interference to the "
+          "user (i.e., when the user is not busy making other VoIP calls)."
+        data:
+          "WebRTC events such as the timing of audio playout (but not the "
+          "content), timing and size of RTP packets sent/received, etc."
+        destination: GOOGLE_OWNED_SERVICE
+      }
+      policy {
+        cookies_allowed: NO
+        setting: "This feature is only enabled if the user launches Chrome "
+                 "with a specific command line flag: "
+                 "--enable-features=WebRtcRemoteEventLog"
+        policy_exception_justification:
+          "Not applicable."
+      })");
+
+void AddFileContents(const std::string& file_contents,
+                     const std::string& content_type,
+                     std::string* post_data) {
+  // net::AddMultipartValueForUpload does almost what we want to do here, except
+  // that it does not add the "filename" attribute. We hack it to force it to.
+  std::string mime_value_name = base::StringPrintf(
+      "%s\"; filename=\"%s.%s\"", kLogFilename, kLogFilename, kLogExtension);
+  net::AddMultipartValueForUpload(mime_value_name, file_contents, kBoundary,
+                                  content_type, post_data);
+}
+
+std::string MimeContentType() {
+  const char kBoundaryKeywordAndMisc[] = "; boundary=";
+
+  std::string content_type;
+  content_type.reserve(sizeof(content_type) + sizeof(kBoundaryKeywordAndMisc) +
+                       sizeof(kBoundary));
+
+  content_type.append(kUploadContentType);
+  content_type.append(kBoundaryKeywordAndMisc);
+  content_type.append(kBoundary);
+
+  return content_type;
+}
+}  // namespace
+
+const char WebRtcEventLogUploaderImpl::kUploadURL[] =
+    "https://clients2.google.com/cr/report";
+
+std::unique_ptr<WebRtcEventLogUploader>
+WebRtcEventLogUploaderImpl::Factory::Create(
+    const base::FilePath& log_file,
     WebRtcEventLogUploaderObserver* observer) {
   DCHECK(observer);
+  return std::make_unique<WebRtcEventLogUploaderImpl>(
+      log_file, observer, kMaxRemoteLogFileSizeBytes);
+}
 
-  // TODO(eladalon): Provide an actual implementation; really upload the file.
-  // https://crbug.com/775415
+std::unique_ptr<WebRtcEventLogUploader>
+WebRtcEventLogUploaderImpl::Factory::CreateWithCurstomMaxSizeForTesting(
+    const base::FilePath& log_file,
+    WebRtcEventLogUploaderObserver* observer,
+    size_t max_log_file_size_bytes) {
+  DCHECK(observer);
+  return std::make_unique<WebRtcEventLogUploaderImpl>(log_file, observer,
+                                                      max_log_file_size_bytes);
+}
+
+WebRtcEventLogUploaderImpl::Delegate::Delegate(
+    WebRtcEventLogUploaderImpl* owner)
+    : owner_(owner) {}
+
+void WebRtcEventLogUploaderImpl::Delegate::OnURLFetchComplete(
+    const net::URLFetcher* source) {
+  owner_->OnURLFetchComplete(source);
+}
+
+WebRtcEventLogUploaderImpl::WebRtcEventLogUploaderImpl(
+    const base::FilePath& log_file,
+    WebRtcEventLogUploaderObserver* observer,
+    size_t max_log_file_size_bytes)
+    : delegate_(this),
+      log_file_(log_file),
+      observer_(observer),
+      max_log_file_size_bytes_(max_log_file_size_bytes),
+      request_context_getter_(nullptr),
+      io_task_runner_(base::SequencedTaskRunnerHandle::Get()) {
+  DCHECK(observer);
+
+  if (!PrepareUploadData()) {
+    ReportResult(false);
+    return;
+  }
+
+  // See the comment at the beginning of this file for an explanation about why
+  // base::Unretained is safe to use here.
+  content::BrowserThread::PostTask(
+      content::BrowserThread::UI, FROM_HERE,
+      base::BindOnce(&WebRtcEventLogUploaderImpl::PrepareRequestContext,
+                     base::Unretained(this)));
+}
+
+WebRtcEventLogUploaderImpl::~WebRtcEventLogUploaderImpl() {
+  DCHECK(io_task_runner_->RunsTasksInCurrentSequence());
+  // WebRtcEventLogUploaderImpl objects only deleted if either:
+  // 1. The upload was never started, meaning |url_fetcher_| was never set.
+  // 2. Upload started and finished.
+  // Therefore, we can be sure that when we destroy this object, there are no
+  // tasks pending that still hold a base::Unretained() reference to it.
+  DCHECK(!url_fetcher_);
+}
+
+bool WebRtcEventLogUploaderImpl::PrepareUploadData() {
+  DCHECK(io_task_runner_->RunsTasksInCurrentSequence());
+
+  // TODO(crbug.com/775415): Avoid reading the entire file into memory.
+  std::string log_file_contents;
+  if (!base::ReadFileToStringWithMaxSize(log_file_, &log_file_contents,
+                                         max_log_file_size_bytes_)) {
+    LOG(WARNING) << "Couldn't read event log file, or max file size exceeded.";
+    return false;
+  }
+
+  DCHECK(post_data_.empty());
+  post_data_.reserve(log_file_contents.size() + kExpectedMimeOverheadBytes);
+  net::AddMultipartValueForUpload("prod", kProduct, kBoundary, "", &post_data_);
+  net::AddMultipartValueForUpload("ver",
+                                  version_info::GetVersionNumber() + "-webrtc",
+                                  kBoundary, "", &post_data_);
+  net::AddMultipartValueForUpload("guid", "0", kBoundary, "", &post_data_);
+  net::AddMultipartValueForUpload("type", kLogFilename, kBoundary, "",
+                                  &post_data_);
+  AddFileContents(log_file_contents, "application/log", &post_data_);
+  net::AddMultipartFinalDelimiterForUpload(kBoundary, &post_data_);
+
+  return true;
+}
+
+void WebRtcEventLogUploaderImpl::PrepareRequestContext() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  // system_request_context() can only be gotten on the UI thread, but can
+  // then be used by any thread.
+  DCHECK(!request_context_getter_);
+  request_context_getter_ = g_browser_process->system_request_context();
+  // In unit tests, request_context_getter_ will remain null.
+
+  // See the comment at the beginning of this file for an explanation about why
+  // base::Unretained is safe to use here.
+  io_task_runner_->PostTask(
+      FROM_HERE, base::BindOnce(&WebRtcEventLogUploaderImpl::StartUpload,
+                                base::Unretained(this)));
+}
+
+void WebRtcEventLogUploaderImpl::StartUpload() {
+  DCHECK(io_task_runner_->RunsTasksInCurrentSequence());
+
+  url_fetcher_ = net::URLFetcher::Create(
+      GURL(kUploadURL), net::URLFetcher::POST, &delegate_, kTrafficAnnotation);
+  url_fetcher_->SetRequestContext(request_context_getter_);
+  url_fetcher_->SetLoadFlags(net::LOAD_DO_NOT_SAVE_COOKIES |
+                             net::LOAD_DO_NOT_SEND_COOKIES);
+  url_fetcher_->SetUploadData(MimeContentType(), post_data_);
+  url_fetcher_->Start();  // Delegat::OnURLFetchComplete called when finished.
+}
+
+void WebRtcEventLogUploaderImpl::OnURLFetchComplete(
+    const net::URLFetcher* source) {
+  DCHECK(io_task_runner_->RunsTasksInCurrentSequence());
+  DCHECK_EQ(source, url_fetcher_.get());
+
+  const bool upload_successful =
+      (source->GetStatus().status() == net::URLRequestStatus::SUCCESS &&
+       source->GetResponseCode() == net::HTTP_OK);
+
+  if (upload_successful) {
+    // TODO(crbug.com/775415): Update chrome://webrtc-logs.
+    std::string report_id;
+    if (!url_fetcher_->GetResponseAsString(&report_id)) {
+      LOG(WARNING) << "WebRTC event log completed, but report ID unknown.";
+    } else {
+      // TODO(crbug.com/775415): Remove this when chrome://webrtc-logs updated.
+      VLOG(1) << "WebRTC event log successfully uploaded: " << report_id;
+    }
+  } else {
+    LOG(WARNING) << "WebRTC event log upload failed.";
+  }
+
+  ReportResult(upload_successful);
+
+  url_fetcher_.reset();  // Explicitly maintain determinant.
+}
+
+void WebRtcEventLogUploaderImpl::ReportResult(bool result) {
+  DCHECK(io_task_runner_->RunsTasksInCurrentSequence());
 
   // If the upload was successful, the file is no longer needed.
   // If the upload failed, we don't want to retry, because we run the risk of
   // uploading significant amounts of data once again, only for the upload to
   // fail again after (as an example) wasting 50MBs of upload bandwidth.
-  const bool deletion_successful = base::DeleteFile(path, /*recursive=*/false);
-  if (!deletion_successful) {
-    // This is a somewhat serious (though unlikely) error, because now we'll try
-    // to upload this file again next time Chrome launches.
-    LOG(ERROR) << "Could not delete pending log file.";
-  }
+  // TODO(crbug.com/775415): Provide refined retrial behavior.
+  DeleteLogFile();
 
-  // TODO(eladalon): Provide actual success/failure of upload.
-  // https://crbug.com/775415
-  observer->OnWebRtcEventLogUploadComplete(path, true);
+  observer_->OnWebRtcEventLogUploadComplete(log_file_, result);
 }
 
-std::unique_ptr<WebRtcEventLogUploader>
-WebRtcEventLogUploaderImpl::Factory::Create(
-    const base::FilePath& log_file,
-    WebRtcEventLogUploaderObserver* observer) {
-  return std::make_unique<WebRtcEventLogUploaderImpl>(log_file, observer);
+void WebRtcEventLogUploaderImpl::DeleteLogFile() {
+  DCHECK(io_task_runner_->RunsTasksInCurrentSequence());
+  const bool deletion_successful =
+      base::DeleteFile(log_file_, /*recursive=*/false);
+  if (!deletion_successful) {
+    // This is a somewhat serious (though unlikely) error, because now we'll
+    // try to upload this file again next time Chrome launches.
+    LOG(ERROR) << "Could not delete pending WebRTC event log file.";
+  }
 }

chrome/browser/media/webrtc/webrtc_event_log_uploader.h

@@ -6,14 +6,23 @@
 #define CHROME_BROWSER_MEDIA_WEBRTC_WEBRTC_EVENT_LOG_UPLOADER_H_
 
 #include <memory>
+#include <string>
 
 #include "base/files/file_path.h"
+#include "base/sequenced_task_runner.h"
+#include "chrome/browser/media/webrtc/webrtc_event_log_manager_common.h"
+#include "net/url_request/url_fetcher.h"
+#include "net/url_request/url_fetcher_delegate.h"
+
+namespace net {
+class URLRequestContextGetter;
+}  // namespace net
 
 // A class implementing this interace can register for notification of an
 // upload's eventual result (success/failure).
 class WebRtcEventLogUploaderObserver {
  public:
-  virtual void OnWebRtcEventLogUploadComplete(const base::FilePath& file_path,
+  virtual void OnWebRtcEventLogUploadComplete(const base::FilePath& log_file,
                                               bool upload_successful) = 0;
 
  protected:
@@ -27,8 +36,6 @@ class WebRtcEventLogUploaderObserver {
 // of the upload.
 class WebRtcEventLogUploader {
  public:
-  virtual ~WebRtcEventLogUploader() = default;
-
   // Since we'll need more than one instance of the abstract
   // WebRtcEventLogUploader, we'll need an abstract factory for it.
   class Factory {
@@ -39,18 +46,20 @@ class WebRtcEventLogUploader {
     // rather than be memorized by the factory's constructor, because factories
     // created by unit tests have no visibility into the real implementation's
     // observer (WebRtcRemoteEventLogManager).
+    // This takes ownership of the file. The caller must not attempt to access
+    // the file after invoking Create().
     virtual std::unique_ptr<WebRtcEventLogUploader> Create(
         const base::FilePath& log_file,
         WebRtcEventLogUploaderObserver* observer) = 0;
   };
+
+  virtual ~WebRtcEventLogUploader() = default;
 };
 
+// Primary implementation of WebRtcEventLogUploader. Uploads log files to crash.
+// Deletes log files whether they were successfully uploaded or not.
 class WebRtcEventLogUploaderImpl : public WebRtcEventLogUploader {
  public:
-  WebRtcEventLogUploaderImpl(const base::FilePath& path,
-                             WebRtcEventLogUploaderObserver* observer);
-  ~WebRtcEventLogUploaderImpl() override = default;
-
   class Factory : public WebRtcEventLogUploader::Factory {
    public:
     ~Factory() override = default;
@@ -58,7 +67,90 @@ class WebRtcEventLogUploaderImpl : public WebRtcEventLogUploader {
     std::unique_ptr<WebRtcEventLogUploader> Create(
         const base::FilePath& log_file,
         WebRtcEventLogUploaderObserver* observer) override;
+
+   protected:
+    friend class WebRtcEventLogUploaderImplTest;
+
+    std::unique_ptr<WebRtcEventLogUploader> CreateWithCurstomMaxSizeForTesting(
+        const base::FilePath& log_file,
+        WebRtcEventLogUploaderObserver* observer,
+        size_t max_remote_log_file_size_bytes);
   };
+
+  WebRtcEventLogUploaderImpl(const base::FilePath& log_file,
+                             WebRtcEventLogUploaderObserver* observer,
+                             size_t max_remote_log_file_size_bytes);
+  ~WebRtcEventLogUploaderImpl() override;
+
+ protected:
+  friend class WebRtcEventLogUploaderImplTest;
+
+  // Prepare the data that will be uploaded. Runs on io_task_runner_.
+  bool PrepareUploadData();
+
+  // Prepares the URLRequestContextGetter. This has to run on the UI thread,
+  // but once complete, the URLRequestContextGetter it produces, which is
+  // stored in request_context_getter_, may be used on any task context.
+  void PrepareRequestContext();
+
+  // Initiates the upload. Runs on io_task_runner_, so that the callback will
+  // also be called on io_task_runner_.
+  void StartUpload();
+
+  // Called on io_task_runner_.  Before this is called, other methods of the
+  // URLFetcherDelegate API may be called, but this is guaranteed to be the
+  // last call, so deleting |this| is permissible afterwards.
+  void OnURLFetchComplete(const net::URLFetcher* source);
+
+  // Cleanup and reporting to |observer_|.
+  void ReportResult(bool result);
+
+  // Remove the log file which is owned by |this|.
+  void DeleteLogFile();
+
+  // Allows testing the behavior for excessively large files.
+  void SetMaxRemoteLogFileSizeBytesForTesting(size_t max_size_bytes);
+
+  // The URL used for uploading the logs.
+  static const char kUploadURL[];
+
+ private:
+  class Delegate : public net::URLFetcherDelegate {
+   public:
+    explicit Delegate(WebRtcEventLogUploaderImpl* owner);
+    ~Delegate() override = default;
+
+    // net::URLFetcherDelegate implementation.
+    void OnURLFetchComplete(const net::URLFetcher* source) override;
+
+   private:
+    WebRtcEventLogUploaderImpl* const owner_;
+  } delegate_;
+
+  // The path to the WebRTC event log file that this uploader is in charge of.
+  const base::FilePath log_file_;
+
+  // The observer to be notified when this upload succeeds or fails.
+  WebRtcEventLogUploaderObserver* const observer_;
+
+  // Maximum allowed file size. In production code, this is a hard-coded,
+  // but unit tests may set other values.
+  const size_t max_log_file_size_bytes_;
+
+  // This is written to on the UI thread, but used on the IO thread. It allows
+  // the creation of URLFetcher objects.
+  net::URLRequestContextGetter* request_context_getter_;
+
+  // This object is in charge of the actual upload.
+  std::unique_ptr<net::URLFetcher> url_fetcher_;
+
+  // To avoid an unnecessary hop to the UI thread when something is amiss with
+  // the data we wish to upload, PrepareUploadData() is called first, and saves
+  // the data here. When back from the UI thread, StartUpload will read this.
+  std::string post_data_;
+
+  // The object lives on this IO-capable task runner.
+  scoped_refptr<base::SequencedTaskRunner> io_task_runner_;
 };
 
 #endif  // CHROME_BROWSER_MEDIA_WEBRTC_WEBRTC_EVENT_LOG_UPLOADER_H_

chrome/browser/media/webrtc/webrtc_event_log_uploader_impl_unittest.cc

+// Copyright (c) 2018 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/browser/media/webrtc/webrtc_event_log_uploader.h"
+
+#include <memory>
+#include <string>
+
+#include "base/callback_forward.h"
+#include "base/files/file_path.h"
+#include "base/files/file_util.h"
+#include "base/files/scoped_temp_dir.h"
+#include "base/run_loop.h"
+#include "base/sequenced_task_runner.h"
+#include "base/task_scheduler/post_task.h"
+#include "build/build_config.h"
+#include "content/public/test/test_browser_thread_bundle.h"
+#include "net/http/http_status_code.h"
+#include "net/url_request/test_url_fetcher_factory.h"
+#include "net/url_request/url_request_status.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using ::testing::StrictMock;
+
+namespace {
+class MockWebRtcEventLogUploaderObserver
+    : public WebRtcEventLogUploaderObserver {
+ public:
+  explicit MockWebRtcEventLogUploaderObserver(
+      base::OnceClosure completion_closure)
+      : completion_closure_(std::move(completion_closure)) {}
+
+  ~MockWebRtcEventLogUploaderObserver() override = default;
+
+  // Combines the mock functionality via a helper (CompletionCallback), as well
+  // as calls the completion closure.
+  void OnWebRtcEventLogUploadComplete(const base::FilePath& log_file,
+                                      bool upload_successful) {
+    CompletionCallback(log_file, upload_successful);
+    std::move(completion_closure_).Run();
+  }
+
+  MOCK_METHOD2(CompletionCallback, void(const base::FilePath&, bool));
+
+ private:
+  base::OnceClosure completion_closure_;
+};
+
+#if defined(OS_POSIX) && !defined(OS_FUCHSIA)
+void RemovePermissions(const base::FilePath& path, int removed_permissions) {
+  int permissions;
+  ASSERT_TRUE(base::GetPosixFilePermissions(path, &permissions));
+  permissions &= ~removed_permissions;
+  ASSERT_TRUE(base::SetPosixFilePermissions(path, permissions));
+}
+
+void RemoveReadPermissions(const base::FilePath& path) {
+  constexpr int read_permissions = base::FILE_PERMISSION_READ_BY_USER |
+                                   base::FILE_PERMISSION_READ_BY_GROUP |
+                                   base::FILE_PERMISSION_READ_BY_OTHERS;
+  RemovePermissions(path, read_permissions);
+}
+
+void RemoveWritePermissions(const base::FilePath& path) {
+  constexpr int write_permissions = base::FILE_PERMISSION_WRITE_BY_USER |
+                                    base::FILE_PERMISSION_WRITE_BY_GROUP |
+                                    base::FILE_PERMISSION_WRITE_BY_OTHERS;
+  RemovePermissions(path, write_permissions);
+}
+#endif  // defined(OS_POSIX) && !defined(OS_FUCHSIA)
+}  // namespace
+
+class WebRtcEventLogUploaderImplTest : public ::testing::Test {
+ public:
+  WebRtcEventLogUploaderImplTest()
+      : observer_run_loop_(),
+        url_fetcher_factory_(nullptr),
+        observer_(observer_run_loop_.QuitWhenIdleClosure()),
+        task_runner_(base::CreateSequencedTaskRunnerWithTraits(
+            {base::MayBlock(), base::TaskPriority::BACKGROUND,
+             base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN})) {}
+  ~WebRtcEventLogUploaderImplTest() override = default;
+
+  void SetUp() override {
+    ASSERT_TRUE(log_dir_.CreateUniqueTempDir());
+    ASSERT_TRUE(base::CreateTemporaryFileInDir(log_dir_.GetPath(), &log_file_));
+    constexpr size_t kLogFileSizeBytes = 100u;
+    const std::string file_contents(kLogFileSizeBytes, 'A');
+    ASSERT_EQ(
+        base::WriteFile(log_file_, file_contents.c_str(), file_contents.size()),
+        static_cast<int>(file_contents.size()));
+  }
+
+  void TearDown() override {
+    base::RunLoop tear_down_run_loop;
+    task_runner_->PostTask(
+        FROM_HERE,
+        base::BindOnce(
+            [](WebRtcEventLogUploaderImplTest* test,
+               base::OnceClosure quit_closure) {
+              test->uploader_.reset();
+              std::move(quit_closure).Run();
+            },
+            base::Unretained(this), tear_down_run_loop.QuitWhenIdleClosure()));
+    tear_down_run_loop.Run();
+  }
+
+  void SetUrlFetcherResponse(net::HttpStatusCode http_code,
+                             net::URLRequestStatus::Status request_status) {
+    const std::string kResponseId = "ec1ed029734b8f7e";  // Arbitrary.
+    url_fetcher_factory_.SetFakeResponse(
+        GURL(WebRtcEventLogUploaderImpl::kUploadURL), kResponseId, http_code,
+        request_status);
+  }
+
+  void StartAndWaitForUpload() {
+    task_runner_->PostTask(
+        FROM_HERE, base::BindOnce(
+                       [](WebRtcEventLogUploaderImplTest* test) {
+                         test->uploader_ = test->uploader_factory_.Create(
+                             test->log_file_, &test->observer_);
+                       },
+                       base::Unretained(this)));
+    observer_run_loop_.Run();  // Observer was given quit-closure by ctor.
+  }
+
+  void StartAndWaitForUploadWithCustomMaxSize(size_t max_log_size_bytes) {
+    task_runner_->PostTask(
+        FROM_HERE,
+        base::BindOnce(
+            [](WebRtcEventLogUploaderImplTest* test,
+               size_t max_log_size_bytes) {
+              test->uploader_ =
+                  test->uploader_factory_.CreateWithCurstomMaxSizeForTesting(
+                      test->log_file_, &test->observer_, max_log_size_bytes);
+            },
+            base::Unretained(this), max_log_size_bytes));
+    observer_run_loop_.Run();  // Observer was given quit-closure by ctor.
+  }
+
+  content::TestBrowserThreadBundle test_browser_thread_bundle_;
+  base::RunLoop observer_run_loop_;
+
+  base::ScopedTempDir log_dir_;
+  base::FilePath log_file_;
+
+  net::FakeURLFetcherFactory url_fetcher_factory_;
+
+  StrictMock<MockWebRtcEventLogUploaderObserver> observer_;
+
+  // These (uploader-factory and uploader) are the units under test.
+  WebRtcEventLogUploaderImpl::Factory uploader_factory_;
+  std::unique_ptr<WebRtcEventLogUploader> uploader_;
+
+  scoped_refptr<base::SequencedTaskRunner> task_runner_;
+};
+
+TEST_F(WebRtcEventLogUploaderImplTest, SuccessfulUploadReportedToObserver) {
+  // Main test.
+  SetUrlFetcherResponse(net::HTTP_OK, net::URLRequestStatus::SUCCESS);
+  EXPECT_CALL(observer_, CompletionCallback(log_file_, true)).Times(1);
+  StartAndWaitForUpload();
+  EXPECT_FALSE(base::PathExists(log_file_));
+}
+
+// Version #1 - request reported as successful, but got an error (404) as the
+// HTTP return code.
+// Due to the simplicitly of both tests, this also tests the scenario
+// FileDeletedAfterUnsuccessfulUpload, rather than giving each its own test.
+TEST_F(WebRtcEventLogUploaderImplTest, UnsuccessfulUploadReportedToObserver1) {
+  SetUrlFetcherResponse(net::HTTP_NOT_FOUND, net::URLRequestStatus::SUCCESS);
+  EXPECT_CALL(observer_, CompletionCallback(log_file_, false)).Times(1);
+  StartAndWaitForUpload();
+  EXPECT_FALSE(base::PathExists(log_file_));
+}
+
+// Version #2 - request reported as failed; HTTP return code ignored, even
+// if it's a purported success.
+TEST_F(WebRtcEventLogUploaderImplTest, UnsuccessfulUploadReportedToObserver2) {
+  SetUrlFetcherResponse(net::HTTP_OK, net::URLRequestStatus::FAILED);
+  EXPECT_CALL(observer_, CompletionCallback(log_file_, false)).Times(1);
+  StartAndWaitForUpload();
+  EXPECT_FALSE(base::PathExists(log_file_));
+}
+
+#if defined(OS_POSIX) && !defined(OS_FUCHSIA)
+TEST_F(WebRtcEventLogUploaderImplTest, FailureToReadFileReportedToObserver) {
+  // Show the failure was independent of the URLFetcher's primed return value.
+  SetUrlFetcherResponse(net::HTTP_OK, net::URLRequestStatus::SUCCESS);
+
+  RemoveReadPermissions(log_file_);
+  EXPECT_CALL(observer_, CompletionCallback(log_file_, false)).Times(1);
+  StartAndWaitForUpload();
+}
+
+TEST_F(WebRtcEventLogUploaderImplTest, FailureToDeleteFileHandledGracefully) {
+  // Prepare for end of test cleanup.
+  int permissions;
+  ASSERT_TRUE(base::GetPosixFilePermissions(log_dir_.GetPath(), &permissions));
+
+  // The uploader won't be able to delete the file, but it would be able to
+  // read and upload it.
+  RemoveWritePermissions(log_dir_.GetPath());
+  SetUrlFetcherResponse(net::HTTP_OK, net::URLRequestStatus::SUCCESS);
+  EXPECT_CALL(observer_, CompletionCallback(log_file_, true)).Times(1);
+  StartAndWaitForUpload();
+
+  // Sanity over the test itself - the file really could not be deleted.
+  ASSERT_TRUE(base::PathExists(log_file_));
+
+  // Cleaup
+  ASSERT_TRUE(base::SetPosixFilePermissions(log_dir_.GetPath(), permissions));
+}
+#endif  // defined(OS_POSIX) && !defined(OS_FUCHSIA)
+
+TEST_F(WebRtcEventLogUploaderImplTest, FilesUpToMaxSizeUploaded) {
+  int64_t log_file_size_bytes;
+  ASSERT_TRUE(base::GetFileSize(log_file_, &log_file_size_bytes));
+
+  SetUrlFetcherResponse(net::HTTP_OK, net::URLRequestStatus::SUCCESS);
+  EXPECT_CALL(observer_, CompletionCallback(log_file_, true)).Times(1);
+  StartAndWaitForUploadWithCustomMaxSize(log_file_size_bytes);
+  EXPECT_FALSE(base::PathExists(log_file_));
+}
+
+TEST_F(WebRtcEventLogUploaderImplTest, ExcessivelyLargeFilesNotUploaded) {
+  int64_t log_file_size_bytes;
+  ASSERT_TRUE(base::GetFileSize(log_file_, &log_file_size_bytes));
+
+  SetUrlFetcherResponse(net::HTTP_OK, net::URLRequestStatus::SUCCESS);
+  EXPECT_CALL(observer_, CompletionCallback(log_file_, false)).Times(1);
+  StartAndWaitForUploadWithCustomMaxSize(log_file_size_bytes - 1);
+  EXPECT_FALSE(base::PathExists(log_file_));
+}

chrome/test/BUILD.gn

@@ -2851,6 +2851,7 @@ test("unit_tests") {
       "../browser/media/router/discovery/discovery_network_monitor_unittest.cc",
       "../browser/media/webrtc/tab_desktop_media_list_unittest.cc",
       "../browser/media/webrtc/webrtc_event_log_manager_unittest.cc",
+      "../browser/media/webrtc/webrtc_event_log_uploader_impl_unittest.cc",
       "../browser/media_galleries/fileapi/native_media_file_util_unittest.cc",
       "../browser/media_galleries/gallery_watch_manager_unittest.cc",
       "../browser/media_galleries/mac/mtp_device_delegate_impl_mac_unittest.mm",

tools/traffic_annotation/summary/annotations.xml

@@ -252,6 +252,7 @@ Refer to README.md for content description and update process.
  <item id="web_history_expire_between_dates" hash_code="126122632" type="1" second_id="110307337" content_hash_code="34304787" os_list="linux,windows" semantics_fields="2,3,4" policy_fields="4" file_path="components/history/core/browser/history_service.cc"/>
  <item id="web_history_query" hash_code="17400350" type="1" second_id="110307337" content_hash_code="36075147" os_list="linux,windows" semantics_fields="2,3,4" policy_fields="4" file_path="components/history/core/browser/browsing_history_service.cc"/>
  <item id="web_history_service" hash_code="110307337" type="2" content_hash_code="16140045" os_list="linux,windows" semantics_fields="1,5" policy_fields="-1,3" file_path="components/history/core/browser/web_history_service.cc"/>
+ <item id="webrtc_event_log_uploader" hash_code="24186190" type="0" content_hash_code="107123816" os_list="linux,windows" file_path="chrome/browser/media/webrtc/webrtc_event_log_uploader.cc"/>
  <item id="webrtc_log_upload" hash_code="62443804" type="0" content_hash_code="33661169" os_list="linux,windows" file_path="chrome/browser/media/webrtc/webrtc_log_uploader.cc"/>
  <item id="webrtc_peer_connection" hash_code="63497370" type="0" content_hash_code="60553259" os_list="linux,windows" file_path="content/renderer/media/webrtc/peer_connection_dependency_factory.cc"/>
  <item id="websocket_basic_stream" hash_code="51586722" type="0" content_hash_code="68121427" os_list="linux,windows" file_path="net/websockets/websocket_basic_stream.cc"/>