Refactor WebRtcLoggingHandlerHost in preparation of automatic upload of WebRtcEventLogs.

chrome/browser/media/webrtc_logging_handler_host.cc

Index: chrome/browser/media/webrtc_logging_handler_host.cc
diff --git a/chrome/browser/media/webrtc_logging_handler_host.cc b/chrome/browser/media/webrtc_logging_handler_host.cc
index d05496073a1eb9ab228743d47253b0a3eb103f70..670e2fc7799b352db727f0e49387ce042a4e6c8c 100644
--- a/chrome/browser/media/webrtc_logging_handler_host.cc
+++ b/chrome/browser/media/webrtc_logging_handler_host.cc
@@ -9,149 +9,39 @@
 
 #include "base/bind.h"
 #include "base/command_line.h"
-#include "base/cpu.h"
 #include "base/files/file_util.h"
 #include "base/logging.h"
-#include "base/strings/string_number_conversions.h"
-#include "base/sys_info.h"
-#include "base/time/time.h"
 #include "build/build_config.h"
 #include "chrome/browser/bad_message.h"
 #include "chrome/browser/browser_process.h"
-#include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chrome/browser/media/webrtc_log_list.h"
 #include "chrome/browser/media/webrtc_log_uploader.h"
 #include "chrome/browser/media/webrtc_rtp_dump_handler.h"
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/common/channel_info.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/media/webrtc_logging_messages.h"
-#include "chromeos/settings/cros_settings_names.h"
-#include "components/prefs/pref_service.h"
-#include "components/version_info/version_info.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
-#include "content/public/browser/gpu_data_manager.h"
 #include "content/public/browser/render_process_host.h"
-#include "gpu/config/gpu_info.h"
-#include "net/base/ip_address.h"
-#include "net/url_request/url_request_context_getter.h"
 
-#if defined(OS_LINUX)
-#include "base/linux_util.h"
-#endif
-
-#if defined(OS_MACOSX)
-#include "base/mac/mac_util.h"
-#endif
-
-#if defined(OS_CHROMEOS)
-#include "chromeos/system/statistics_provider.h"
-#endif
-
-using base::IntToString;
 using content::BrowserThread;
 
 // Key used to attach the handler to the RenderProcessHost.
 const char WebRtcLoggingHandlerHost::kWebRtcLoggingHandlerHostKey[] =
     "kWebRtcLoggingHandlerHostKey";
 
-namespace {
-
-const char kLogNotStoppedOrNoLogOpen[] =
-    "Logging not stopped or no log open.";
-
-// For privacy reasons when logging IP addresses. The returned "sensitive
-// string" is for release builds a string with the end stripped away. Last
-// octet for IPv4 and last 80 bits (5 groups) for IPv6. String will be
-// "1.2.3.x" and "1.2.3::" respectively. For debug builds, the string is
-// not stripped.
-std::string IPAddressToSensitiveString(const net::IPAddress& address) {
-#if defined(NDEBUG)
-  std::string sensitive_address;
-  switch (address.size()) {
-    case net::IPAddress::kIPv4AddressSize: {
-      sensitive_address = address.ToString();
-      size_t find_pos = sensitive_address.rfind('.');
-      if (find_pos == std::string::npos)
-        return std::string();
-      sensitive_address.resize(find_pos);
-      sensitive_address += ".x";
-      break;
-    }
-    case net::IPAddress::kIPv6AddressSize: {
-      // TODO(grunell): Create a string of format "1:2:3:x:x:x:x:x" to clarify
-      // that the end has been stripped out.
-      std::vector<uint8_t> bytes = address.bytes();
-      std::fill(bytes.begin() + 6, bytes.end(), 0);
-      net::IPAddress stripped_address(bytes);
-      sensitive_address = stripped_address.ToString();
-      break;
-    }
-    default: { break; }
-  }
-  return sensitive_address;
-#else
-  return address.ToString();
-#endif
-}
-
-void FormatMetaDataAsLogMessage(
-    const MetaDataMap& meta_data,
-    std::string* message) {
-  for (MetaDataMap::const_iterator it = meta_data.begin();
-       it != meta_data.end(); ++it) {
-    *message += it->first + ": " + it->second + '\n';
-  }
-  // Remove last '\n'.
-  message->resize(message->size() - 1);
-}
-
-}  // namespace
-
-WebRtcLogBuffer::WebRtcLogBuffer()
-    : buffer_(),
-      circular_(&buffer_[0], sizeof(buffer_), sizeof(buffer_) / 2, false),
-      read_only_(false) {
-}
-
-WebRtcLogBuffer::~WebRtcLogBuffer() {
-  DCHECK(read_only_ || thread_checker_.CalledOnValidThread());
-}
-
-void WebRtcLogBuffer::Log(const std::string& message) {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  DCHECK(!read_only_);
-  circular_.Write(message.c_str(), message.length());
-  const char eol = '\n';
-  circular_.Write(&eol, 1);
-}
-
-PartialCircularBuffer WebRtcLogBuffer::Read() {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  DCHECK(read_only_);
-  return PartialCircularBuffer(&buffer_[0], sizeof(buffer_));
-}
-
-void WebRtcLogBuffer::SetComplete() {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  DCHECK(!read_only_) << "Already set? (programmer error)";
-  read_only_ = true;
-  // Detach from the current thread so that we can check reads on a different
-  // thread.  This is to make sure that Read()s still happen on one thread only.
-  thread_checker_.DetachFromThread();
-}
-
 WebRtcLoggingHandlerHost::WebRtcLoggingHandlerHost(
     int render_process_id,
     Profile* profile,
     WebRtcLogUploader* log_uploader)
     : BrowserMessageFilter(WebRtcLoggingMsgStart),
+      render_process_id_(render_process_id),
       profile_(profile),
-      logging_state_(CLOSED),
       upload_log_on_render_close_(false),
-      log_uploader_(log_uploader),
-      render_process_id_(render_process_id) {
+      text_log_handler_(new WebRtcTextLogHandler(render_process_id)),
+      rtp_dump_handler_(),
+      stop_rtp_dump_callback_(),
+      log_uploader_(log_uploader) {
   DCHECK(profile_);
   DCHECK(log_uploader_);
 }
@@ -159,7 +49,7 @@ WebRtcLoggingHandlerHost::WebRtcLoggingHandlerHost(
 WebRtcLoggingHandlerHost::~WebRtcLoggingHandlerHost() {
   // If we hit this, then we might be leaking a log reference count (see
   // ApplyForStartLogging).
-  DCHECK_EQ(CLOSED, logging_state_);
+  DCHECK_EQ(WebRtcTextLogHandler::CLOSED, text_log_handler_->GetState());
 }
 
 void WebRtcLoggingHandlerHost::SetMetaData(
@@ -168,25 +58,7 @@ void WebRtcLoggingHandlerHost::SetMetaData(
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(!callback.is_null());
 
-  std::string error_message;
-  if (logging_state_ == CLOSED) {
-    if (!meta_data_.get())
-      meta_data_ = std::move(meta_data);
-  } else if (logging_state_ == STARTED) {
-    std::string meta_data_message;
-    FormatMetaDataAsLogMessage(*meta_data.get(), &meta_data_message);
-    LogToCircularBuffer(meta_data_message);
-  } else {
-    error_message = "Meta data must be set before stop or upload.";
-  }
-
-  if (error_message.empty() && meta_data.get()) {
-    // Keep the meta data around for uploading separately from the log.
-    for (const auto& it : *meta_data.get())
-      (*meta_data_.get())[it.first] = it.second;
-  }
-
-  FireGenericDoneCallback(callback, error_message.empty(), error_message);
+  text_log_handler_->SetMetaData(std::move(meta_data), callback);
 }
 
 void WebRtcLoggingHandlerHost::StartLogging(
@@ -194,27 +66,12 @@ void WebRtcLoggingHandlerHost::StartLogging(
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(!callback.is_null());
 
-  if (logging_state_ != CLOSED) {
-    FireGenericDoneCallback(callback, false, "A log is already open.");
-    return;
-  }
-
-  if (!log_uploader_->ApplyForStartLogging()) {
-    FireGenericDoneCallback(callback, false,
-        "Cannot start, maybe the maximum number of "
-        "simultaneuos logs has been reached.");
-    return;
+  // Request a log_slot from the LogUploader and start logging.
+  if (text_log_handler_->StartLogging(log_uploader_, callback)) {
+    // Start logging in the renderer. The callback has already been fired since
+    // there is no acknowledgement when the renderer actually starts.
+    Send(new WebRtcLoggingMsg_StartLogging());
   }
-
-  logging_state_ = STARTING;
-
-  DCHECK(!log_buffer_.get());
-  log_buffer_.reset(new WebRtcLogBuffer());
-  if (!meta_data_.get())
-    meta_data_.reset(new MetaDataMap());
-
-  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE, base::Bind(
-      &WebRtcLoggingHandlerHost::LogInitialInfoOnFileThread, this, callback));
 }
 
 void WebRtcLoggingHandlerHost::StopLogging(
@@ -222,38 +79,28 @@ void WebRtcLoggingHandlerHost::StopLogging(
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(!callback.is_null());
 
-  if (logging_state_ != STARTED) {
-    FireGenericDoneCallback(callback, false, "Logging not started.");
-    return;
+  // Change the state to STOPPING and disable logging in the browser.
+  if (text_log_handler_->StopLogging(callback)) {
+    // Stop logging in the renderer. OnLoggingStoppedInRenderer will be called
+    // when this is done to change the state from STOPPING to STOPPED and fire
+    // the callback.
+    Send(new WebRtcLoggingMsg_StopLogging());
   }
-
-  stop_callback_ = callback;
-  logging_state_ = STOPPING;
-
-  Send(new WebRtcLoggingMsg_StopLogging());
-
-  BrowserThread::PostTask(
-      BrowserThread::UI, FROM_HERE,
-      base::Bind(
-          &WebRtcLoggingHandlerHost::DisableBrowserProcessLoggingOnUIThread,
-          this));
 }
 
 void WebRtcLoggingHandlerHost::UploadLog(const UploadDoneCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(!callback.is_null());
 
-  if (logging_state_ != STOPPED) {
-    if (!callback.is_null()) {
-      content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE,
-          base::Bind(callback, false, "", kLogNotStoppedOrNoLogOpen));
-    }
-    return;
-  }
+  // This functions uploads both text logs (mandatory) and RTP dumps (optional).
+  // TODO(terelius): If there's no text log available (either because it hasn't
+  // been started or because it hasn't been stopped), the current implementation
+  // will fire an error callback and leave any RTP dumps in a local directory.
+  // Would it be better to upload whatever logs we have, or would the lack of
+  // an error callback make it harder to debug potential errors?
 
-  content::BrowserThread::PostTaskAndReplyWithResult(
-      content::BrowserThread::FILE,
-      FROM_HERE,
+  BrowserThread::PostTaskAndReplyWithResult(
+      content::BrowserThread::FILE, FROM_HERE,
       base::Bind(&WebRtcLoggingHandlerHost::GetLogDirectoryAndEnsureExists,
                  this),
       base::Bind(&WebRtcLoggingHandlerHost::TriggerUpload, this, callback));
@@ -287,21 +134,22 @@ void WebRtcLoggingHandlerHost::UploadStoredLogOnFileThread(
 
 void WebRtcLoggingHandlerHost::UploadLogDone() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  logging_state_ = CLOSED;
+  // The logging state changed to CLOSED when we released the logs prior to
+  // uploading. We can't check the state because a new log might have started
+  // already, so there is nothing for us to do here. In the future, we might
+  // want to use this function to clean up files stored on disc.
 }
 
 void WebRtcLoggingHandlerHost::DiscardLog(const GenericDoneCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(!callback.is_null());
 
-  if (logging_state_ != STOPPED) {
-    FireGenericDoneCallback(callback, false, kLogNotStoppedOrNoLogOpen);
+  if (!text_log_handler_->ExpectLoggingStateStopped(callback)) {
+    // The callback is fired with an error message by ExpectLoggingStateStopped.
     return;
   }
   log_uploader_->LoggingStoppedDontUpload();
-  log_buffer_.reset();
-  meta_data_.reset();
-  logging_state_ = CLOSED;
+  text_log_handler_->DiscardLog();
   rtp_dump_handler_.reset();
   stop_rtp_dump_callback_.Reset();
   FireGenericDoneCallback(callback, true, "");
@@ -314,8 +162,8 @@ void WebRtcLoggingHandlerHost::StoreLog(
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(!callback.is_null());
 
-  if (logging_state_ != STOPPED) {
-    FireGenericDoneCallback(callback, false, kLogNotStoppedOrNoLogOpen);
+  if (!text_log_handler_->ExpectLoggingStateStopped(callback)) {
+    // The callback is fired with an error message by ExpectLoggingStateStopped.
     return;
   }
 
@@ -351,13 +199,6 @@ void WebRtcLoggingHandlerHost::StoreLogContinue(
                  base::Passed(&log_paths), callback));
 }
 
-void WebRtcLoggingHandlerHost::LogMessage(const std::string& message) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  if (logging_state_ == STARTED) {
-    LogToCircularBuffer(WebRtcLoggingMessageData::Format(
-        message, base::Time::Now(), logging_started_time_));
-  }
-}
 
 void WebRtcLoggingHandlerHost::StartRtpDump(
     RtpDumpType type,
@@ -444,21 +285,31 @@ void WebRtcLoggingHandlerHost::DumpRtpPacketOnIOThread(
 
 void WebRtcLoggingHandlerHost::OnChannelClosing() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  if (logging_state_ == STARTED || logging_state_ == STOPPED) {
-    if (upload_log_on_render_close_) {
-      logging_started_time_ = base::Time();
-
-      content::BrowserThread::PostTaskAndReplyWithResult(
-          content::BrowserThread::FILE,
-          FROM_HERE,
-          base::Bind(&WebRtcLoggingHandlerHost::GetLogDirectoryAndEnsureExists,
-                     this),
-          base::Bind(&WebRtcLoggingHandlerHost::TriggerUpload, this,
-                     UploadDoneCallback()));
-    } else {
-      log_uploader_->LoggingStoppedDontUpload();
-      logging_state_ = CLOSED;
-    }
+  switch (text_log_handler_->GetState()) {
+    case WebRtcTextLogHandler::STARTING:
+    case WebRtcTextLogHandler::STARTED:
+    case WebRtcTextLogHandler::STOPPING:
+    case WebRtcTextLogHandler::STOPPED:
+      text_log_handler_->ChannelClosing();
+      if (upload_log_on_render_close_) {
+        content::BrowserThread::PostTaskAndReplyWithResult(
+            content::BrowserThread::FILE, FROM_HERE,
+            base::Bind(
+                &WebRtcLoggingHandlerHost::GetLogDirectoryAndEnsureExists,
+                this),
+            base::Bind(&WebRtcLoggingHandlerHost::TriggerUpload, this,
+                       UploadDoneCallback()));
+      } else {
+        log_uploader_->LoggingStoppedDontUpload();
+        text_log_handler_->DiscardLog();
+      }
+      break;
+    case WebRtcTextLogHandler::CLOSED:
+    case WebRtcTextLogHandler::CHANNEL_CLOSING:
+      // Do nothing
+      break;
+    default:
+      NOTREACHED();
   }
   content::BrowserMessageFilter::OnChannelClosing();
 }
@@ -483,166 +334,26 @@ bool WebRtcLoggingHandlerHost::OnMessageReceived(const IPC::Message& message) {
 void WebRtcLoggingHandlerHost::OnAddLogMessages(
     const std::vector<WebRtcLoggingMessageData>& messages) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  if (logging_state_ == STARTED || logging_state_ == STOPPING) {
+  if (text_log_handler_->GetState() == WebRtcTextLogHandler::STARTED ||
+      text_log_handler_->GetState() == WebRtcTextLogHandler::STOPPING) {
     for (size_t i = 0; i < messages.size(); ++i) {

[magjed_chromium, 2016/07/22 09:30:31]

A range-based for loop would be a little bit cleaner.

[terelius-chromium, 2016/07/26 09:51:41]

Done.

-      LogToCircularBuffer(messages[i].Format(logging_started_time_));
+      text_log_handler_->LogWebRtcLoggingMessageData(messages[i]);
     }
   }
 }
 
 void WebRtcLoggingHandlerHost::OnLoggingStoppedInRenderer() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  if (logging_state_ != STOPPING) {
+  if (text_log_handler_->GetState() != WebRtcTextLogHandler::STOPPING) {
     // If an out-of-order response is received, stop_callback_ may be invalid,
     // and must not be invoked.
     DLOG(ERROR) << "OnLoggingStoppedInRenderer invoked in state "
-                << logging_state_;
+                << text_log_handler_->GetState();
     bad_message::ReceivedBadMessage(
         this, bad_message::WRLHH_LOGGING_STOPPED_BAD_STATE);
     return;
   }
-  logging_started_time_ = base::Time();
-  logging_state_ = STOPPED;
-  FireGenericDoneCallback(stop_callback_, true, "");
-  stop_callback_.Reset();
-}
-
-void WebRtcLoggingHandlerHost::LogInitialInfoOnFileThread(
-    const GenericDoneCallback& callback) {
-  DCHECK_CURRENTLY_ON(BrowserThread::FILE);
-
-  net::NetworkInterfaceList network_list;
-  net::GetNetworkList(&network_list,
-                      net::EXCLUDE_HOST_SCOPE_VIRTUAL_INTERFACES);
-
-  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE, base::Bind(
-      &WebRtcLoggingHandlerHost::LogInitialInfoOnIOThread, this, network_list,
-      callback));
-}
-
-void WebRtcLoggingHandlerHost::LogInitialInfoOnIOThread(
-    const net::NetworkInterfaceList& network_list,
-    const GenericDoneCallback& callback) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  if (logging_state_ != STARTING) {
-    FireGenericDoneCallback(callback, false, "Logging cancelled.");
-    return;
-  }
-
-  // Tell the renderer and the browser to enable logging. Log messages are
-  // recevied on the IO thread, so the initial info will finish to be written
-  // first.
-  Send(new WebRtcLoggingMsg_StartLogging());
-  BrowserThread::PostTask(
-      BrowserThread::UI, FROM_HERE,
-      base::Bind(
-          &WebRtcLoggingHandlerHost::EnableBrowserProcessLoggingOnUIThread,
-          this));
-
-  // Log start time (current time). We don't use base/i18n/time_formatting.h
-  // here because we don't want the format of the current locale.
-  base::Time::Exploded now = {0};
-  base::Time::Now().LocalExplode(&now);
-  LogToCircularBuffer(base::StringPrintf(
-      "Start %d-%02d-%02d %02d:%02d:%02d", now.year, now.month,
-      now.day_of_month, now.hour, now.minute, now.second));
-
-  // Write metadata if received before logging started.
-  if (meta_data_.get() && !meta_data_->empty()) {
-    std::string info;
-    FormatMetaDataAsLogMessage(*meta_data_.get(), &info);
-    LogToCircularBuffer(info);
-  }
-
-  // Chrome version
-  LogToCircularBuffer("Chrome version: " + version_info::GetVersionNumber() +
-                      " " + chrome::GetChannelString());
-
-  // OS
-  LogToCircularBuffer(base::SysInfo::OperatingSystemName() + " " +
-                      base::SysInfo::OperatingSystemVersion() + " " +
-                      base::SysInfo::OperatingSystemArchitecture());
-#if defined(OS_LINUX)
-  LogToCircularBuffer("Linux distribution: " + base::GetLinuxDistro());
-#endif
-
-  // CPU
-  base::CPU cpu;
-  LogToCircularBuffer(
-      "Cpu: " + IntToString(cpu.family()) + "." + IntToString(cpu.model()) +
-      "." + IntToString(cpu.stepping()) + ", x" +
-      IntToString(base::SysInfo::NumberOfProcessors()) + ", " +
-      IntToString(base::SysInfo::AmountOfPhysicalMemoryMB()) + "MB");
-  std::string cpu_brand = cpu.cpu_brand();
-  // Workaround for crbug.com/249713.
-  // TODO(grunell): Remove workaround when bug is fixed.
-  size_t null_pos = cpu_brand.find('\0');
-  if (null_pos != std::string::npos)
-    cpu_brand.erase(null_pos);
-  LogToCircularBuffer("Cpu brand: " + cpu_brand);
-
-  // Computer model
-  std::string computer_model = "Not available";
-#if defined(OS_MACOSX)
-  computer_model = base::mac::GetModelIdentifier();
-#elif defined(OS_CHROMEOS)
-  chromeos::system::StatisticsProvider::GetInstance()->
-      GetMachineStatistic(chromeos::system::kHardwareClassKey, &computer_model);
-#endif
-  LogToCircularBuffer("Computer model: " + computer_model);
-
-  // GPU
-  gpu::GPUInfo gpu_info = content::GpuDataManager::GetInstance()->GetGPUInfo();
-  LogToCircularBuffer(
-      "Gpu: machine-model-name=" + gpu_info.machine_model_name +
-      ", machine-model-version=" + gpu_info.machine_model_version +
-      ", vendor-id=" + base::UintToString(gpu_info.gpu.vendor_id) +
-      ", device-id=" + base::UintToString(gpu_info.gpu.device_id) +
-      ", driver-vendor=" + gpu_info.driver_vendor +
-      ", driver-version=" + gpu_info.driver_version);
-  LogToCircularBuffer(
-      "OpenGL: gl-vendor=" + gpu_info.gl_vendor +
-      ", gl-renderer=" + gpu_info.gl_renderer +
-      ", gl-version=" + gpu_info.gl_version);
-
-  // Network interfaces
-  LogToCircularBuffer("Discovered " + base::SizeTToString(network_list.size()) +
-                      " network interfaces:");
-  for (net::NetworkInterfaceList::const_iterator it = network_list.begin();
-       it != network_list.end(); ++it) {
-    LogToCircularBuffer(
-        "Name: " + it->friendly_name + ", Address: " +
-        IPAddressToSensitiveString(it->address) + ", Type: " +
-        net::NetworkChangeNotifier::ConnectionTypeToString(it->type));
-  }
-
-  logging_started_time_ = base::Time::Now();
-  logging_state_ = STARTED;
-  FireGenericDoneCallback(callback, true, "");
-}
-
-void WebRtcLoggingHandlerHost::EnableBrowserProcessLoggingOnUIThread() {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  content::RenderProcessHost* host =
-      content::RenderProcessHost::FromID(render_process_id_);
-  if (host) {
-    host->SetWebRtcLogMessageCallback(
-        base::Bind(&WebRtcLoggingHandlerHost::LogMessage, this));
-  }
-}
-
-void WebRtcLoggingHandlerHost::DisableBrowserProcessLoggingOnUIThread() {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  content::RenderProcessHost* host =
-      content::RenderProcessHost::FromID(render_process_id_);
-  if (host)
-    host->ClearWebRtcLogMessageCallback();
-}
-
-void WebRtcLoggingHandlerHost::LogToCircularBuffer(const std::string& message) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  DCHECK_NE(logging_state_, CLOSED);
-  log_buffer_->Log(message);
+  text_log_handler_->StopDone();
 }
 
 base::FilePath WebRtcLoggingHandlerHost::GetLogDirectoryAndEnsureExists() {
@@ -687,15 +398,16 @@ void WebRtcLoggingHandlerHost::StoreLogInDirectory(
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   log_paths->log_path = directory;
 
-  log_buffer_->SetComplete();
+  std::unique_ptr<WebRtcLogBuffer> log_buffer;
+  std::unique_ptr<MetaDataMap> meta_data;
+  text_log_handler_->ReleaseLog(&log_buffer, &meta_data);
+
   BrowserThread::PostTask(
       BrowserThread::FILE, FROM_HERE,
       base::Bind(&WebRtcLogUploader::LoggingStoppedDoStore,
-                 base::Unretained(log_uploader_),
-                 *log_paths.get(), log_id, base::Passed(&log_buffer_),
-                 base::Passed(&meta_data_), done_callback));
-
-  logging_state_ = CLOSED;
+                 base::Unretained(log_uploader_), *log_paths.get(), log_id,
+                 base::Passed(&log_buffer), base::Passed(&meta_data),
+                 done_callback));
 }
 
 void WebRtcLoggingHandlerHost::DoUploadLogAndRtpDumps(
@@ -703,20 +415,29 @@ void WebRtcLoggingHandlerHost::DoUploadLogAndRtpDumps(
     const UploadDoneCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
+  if (text_log_handler_->GetState() != WebRtcTextLogHandler::STOPPED &&
+      text_log_handler_->GetState() != WebRtcTextLogHandler::CHANNEL_CLOSING) {
+    BrowserThread::PostTask(
+        content::BrowserThread::UI, FROM_HERE,
+        base::Bind(callback, false, "", "Logging not stopped or no log open."));
+    return;
+  }
+
   WebRtcLogUploadDoneData upload_done_data;
   upload_done_data.log_path = log_directory;
   upload_done_data.callback = callback;
   upload_done_data.host = this;
   ReleaseRtpDumps(&upload_done_data);
 
-  log_buffer_->SetComplete();
+  std::unique_ptr<WebRtcLogBuffer> log_buffer;
+  std::unique_ptr<MetaDataMap> meta_data;
+  text_log_handler_->ReleaseLog(&log_buffer, &meta_data);
+
   BrowserThread::PostTask(
       BrowserThread::FILE, FROM_HERE,
       base::Bind(&WebRtcLogUploader::LoggingStoppedDoUpload,
-                 base::Unretained(log_uploader_), base::Passed(&log_buffer_),
-                 base::Passed(&meta_data_), upload_done_data));
-
-  logging_state_ = CLOSED;
+                 base::Unretained(log_uploader_), base::Passed(&log_buffer),
+                 base::Passed(&meta_data), upload_done_data));
 }
 
 void WebRtcLoggingHandlerHost::CreateRtpDumpHandlerAndStart(
@@ -763,45 +484,13 @@ bool WebRtcLoggingHandlerHost::ReleaseRtpDumps(WebRtcLogPaths* log_paths) {
 }
 
 void WebRtcLoggingHandlerHost::FireGenericDoneCallback(
-    const WebRtcLoggingHandlerHost::GenericDoneCallback& callback,
+    const GenericDoneCallback& callback,
     bool success,
     const std::string& error_message) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(!callback.is_null());
+  DCHECK_EQ(success, error_message.empty());
 
-  if (error_message.empty()) {
-    DCHECK(success);
-    content::BrowserThread::PostTask(
-        content::BrowserThread::UI,
-        FROM_HERE,
-        base::Bind(callback, success, error_message));
-    return;
-  }
-
-  DCHECK(!success);
-
-  // Add current logging state to error message.
-  std::string error_message_with_state(error_message);
-  switch (logging_state_) {
-  case CLOSED:
-    error_message_with_state += " State=closed.";
-    break;
-  case STARTING:
-    error_message_with_state += " State=starting.";
-    break;
-  case STARTED:
-    error_message_with_state += " State=started.";
-    break;
-  case STOPPING:
-    error_message_with_state += " State=stopping.";
-    break;
-  case STOPPED:
-    error_message_with_state += " State=stopped.";
-    break;
-  }
-
-  content::BrowserThread::PostTask(
-      content::BrowserThread::UI,
-      FROM_HERE,
-      base::Bind(callback, success, error_message_with_state));
+  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+                          base::Bind(callback, success, error_message));
 }