TelemetryLogWriter cleanups

remoting/base/telemetry_log_writer.cc

Index: remoting/base/telemetry_log_writer.cc
diff --git a/remoting/signaling/telemetry_log_writer.cc b/remoting/base/telemetry_log_writer.cc
similarity index 77%
rename from remoting/signaling/telemetry_log_writer.cc
rename to remoting/base/telemetry_log_writer.cc
index 39f93c94d3e8f333d88927fd167c04728cb50f8d..a441ec162075a84bdd1a0e6cacf11cde2881dfcb 100644
--- a/remoting/signaling/telemetry_log_writer.cc
+++ b/remoting/base/telemetry_log_writer.cc
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "remoting/signaling/telemetry_log_writer.h"
+#include "remoting/base/telemetry_log_writer.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ -12,7 +12,7 @@
 
 namespace remoting {
 
-const int kMaxTries = 5;
+const int kMaxSendAttempts = 5;
 
 TelemetryLogWriter::TelemetryLogWriter(
     const std::string& telemetry_base_url,
@@ -22,38 +22,38 @@ TelemetryLogWriter::TelemetryLogWriter(
 TelemetryLogWriter::~TelemetryLogWriter() {}
 
 void TelemetryLogWriter::SetAuthToken(const std::string& auth_token) {
-  DCHECK(CalledOnValidThread());
+  DCHECK(thread_checker_.CalledOnValidThread());
   auth_token_ = auth_token;
   SendPendingEntries();
 }
 
 void TelemetryLogWriter::SetAuthClosure(const base::Closure& closure) {
-  DCHECK(CalledOnValidThread());
+  DCHECK(thread_checker_.CalledOnValidThread());
   auth_closure_ = closure;
 }
 
 void TelemetryLogWriter::Log(const ChromotingEvent& entry) {
-  DCHECK(CalledOnValidThread());
+  DCHECK(thread_checker_.CalledOnValidThread());
   pending_entries_.push_back(entry);
   SendPendingEntries();
 }
 
 void TelemetryLogWriter::SendPendingEntries() {
-  DCHECK(CalledOnValidThread());
+  DCHECK(thread_checker_.CalledOnValidThread());
   if (request_ || pending_entries_.empty()) {
     return;
   }
 
-  base::ListValue* events = new base::ListValue();
+  std::unique_ptr<base::ListValue> events(new base::ListValue());
   while (!pending_entries_.empty()) {
     ChromotingEvent& entry = pending_entries_.front();
     events->Append(entry.CopyDictionaryValue());
-    entry.IncrementTryCount();
+    entry.IncrementSendAttempts();
     sending_entries_.push_back(std::move(entry));
     pending_entries_.pop_front();
   }
   base::DictionaryValue log_dictionary;
-  log_dictionary.Set("event", std::unique_ptr<base::Value>(events));
+  log_dictionary.Set("event", std::move(events));
 
   std::string json;
   JSONStringValueSerializer serializer(&json);
@@ -65,7 +65,7 @@ void TelemetryLogWriter::SendPendingEntries() {
 }
 
 void TelemetryLogWriter::PostJsonToServer(const std::string& json) {
-  DCHECK(CalledOnValidThread());
+  DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(!request_);
   request_ = request_factory_->CreateUrlRequest(UrlRequest::Type::POST,
                                                 telemetry_base_url_);
@@ -82,25 +82,26 @@ void TelemetryLogWriter::PostJsonToServer(const std::string& json) {
 
 void TelemetryLogWriter::OnSendLogResult(
     const remoting::UrlRequest::Result& result) {
-  DCHECK(CalledOnValidThread());
+  DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(request_);
   if (!result.success || result.status != net::HTTP_OK) {
     LOG(WARNING) << "Error occur when sending logs to the telemetry server, "
                  << "status: " << result.status;
     VLOG(1) << "Response body: \n"
             << "body: " << result.response_body;
+
+    // Reverse iterating + push_front in order to restore the order of logs.
+    for (auto i = sending_entries_.rbegin(); i < sending_entries_.rend(); i++) {
+      ChromotingEvent& entry = *i;

[Sergey Ulanov, 2016/05/12 22:48:05]

Don't need this. Just use *i below.

[Yuwei, 2016/05/13 00:40:45]

Done.

+      if (entry.send_attempts() < kMaxSendAttempts) {

[Sergey Ulanov, 2016/05/12 22:48:04]

replace this with
 if (entry.send_attempts() >= kMaxSendAttempts)
   break;
The log entries are in chronological order, so everything before current one can
be dropped as well.

[Yuwei, 2016/05/13 00:40:45]

Done. Good catch :)

+        pending_entries_.push_front(std::move(entry));
+      }
+    }
   } else {
     VLOG(1) << "Successfully sent " << sending_entries_.size()
             << " log(s) to telemetry server.";
-    sending_entries_.clear();
-  }
-  while (!sending_entries_.empty()) {
-    ChromotingEvent& entry = sending_entries_.front();
-    if (entry.try_count() < kMaxTries) {
-      pending_entries_.push_back(std::move(entry));
-    }
-    sending_entries_.pop_front();
   }
+  sending_entries_.clear();
   bool should_call_auth_closure =
       result.status == net::HTTP_UNAUTHORIZED && !auth_closure_.is_null();
   request_.reset();  // This may also destroy the result.