RAPPOR implementation

components/rappor/log_uploader.h

Index: components/rappor/log_uploader.h
diff --git a/components/rappor/log_uploader.h b/components/rappor/log_uploader.h
new file mode 100644
index 0000000000000000000000000000000000000000..cb31c920101830201ba37440b08a1420ddfc2f73
--- /dev/null
+++ b/components/rappor/log_uploader.h
@@ -0,0 +1,81 @@
+// Copyright (c) 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.
+
+#ifndef COMPONENTS_RAPPOR_LOG_UPLOADER_H_
+#define COMPONENTS_RAPPOR_LOG_UPLOADER_H_
+
+#include <queue>
+#include <string>
+
+#include "base/memory/weak_ptr.h"
+#include "base/time/time.h"
+#include "base/timer/timer.h"
+#include "net/url_request/url_fetcher.h"
+#include "net/url_request/url_fetcher_delegate.h"
+#include "net/url_request/url_request_context_getter.h"
+
+namespace rappor {
+
+// Handles uploading logs to an external server.
+class LogUploader : public net::URLFetcherDelegate {
+ public:
+  // Constructor takes the server_url that logs should be uploaded to, the
+  // mime_type of the uploaded data, and request context to create uploads

[Ilya Sherman, 2014/01/10 11:00:32]

nit: "request context" -> "request_context"

[Steven Holte, 2014/01/14 00:47:54]

Done.

+  // with.
+  LogUploader(const char* server_url, const char* mime_type,

[Ilya Sherman, 2014/01/10 11:00:32]

nit: Please pass "const std::string&" rather than "const char*" unless you have
some really compelling reason to use C-strings.

[Steven Holte, 2014/01/14 00:47:54]

Done.

+    net::URLRequestContextGetter* request_context);

[Ilya Sherman, 2014/01/10 11:00:32]

nit: In header files, please wrap parameters so that there is one per line, and
all params are aligned with one another.

[Steven Holte, 2014/01/14 00:47:54]

Done.

+
+  ~LogUploader();
+
+  // Adds an entry to the queue of logs to be uploaded to the server.
+  void QueueLog(const std::string& log);
+
+ private:
+  // Starts transmission of the next log.
+  void StartScheduledUpload();
+
+  // Schedules a future call to StartScheduledUpload if one isn't already
+  // pending.
+  void ScheduleNextUpload();
+
+  // Increases the upload interval each time it's called, to handle the case
+  // where the server is having issues.
+  void BackOffUploadInterval();
+
+  // Implementation of net::URLFetcherDelegate. Called after transmission
+  // completes (either successfully or with failure).
+  virtual void OnURLFetchComplete(const net::URLFetcher* source) OVERRIDE;
+
+  // Called when the upload is completed.
+  void UploadFinished(bool server_is_healthy, bool more_logs_remaining);
+
+  // The server URL to upload logs to.
+  const char* server_url_;

[Ilya Sherman, 2014/01/10 11:00:32]

nit: This isn't const enough ;)  If you want to store const C-strings, they
should be stored as const char arrays, or as "const char* const"s.

[Steven Holte, 2014/01/14 00:47:54]

changed to const std::string

+
+  // The mime type to specify on uploaded logs.
+  const char* mime_type_;
+
+  net::URLRequestContextGetter* request_context_;
+
+  // The outstanding transmission appears as a URL Fetch operation.
+  scoped_ptr<net::URLFetcher> current_fetch_;
+
+  // The logs that still need to be uploaded.
+  std::queue<std::string> queued_logs_;
+
+  base::OneShotTimer<LogUploader> upload_timer_;
+
+  // Indicates that the last triggered upload hasn't resolved yet.
+  bool callback_pending_;

[Ilya Sherman, 2014/01/10 11:00:32]

nit: I'd recommend adding a prefix like "has_" to emphasize that this is a
boolean value.

[Steven Holte, 2014/01/14 00:47:54]

Done.

+
+  // The interval between being told an upload is done and starting the next

[Ilya Sherman, 2014/01/10 11:00:32]

nit: "being told" -> "receiving confirmation" or something like that

[Steven Holte, 2014/01/14 00:47:54]

Done.

+  // upload.
+  base::TimeDelta upload_interval_;
+
+  DISALLOW_COPY_AND_ASSIGN(LogUploader);
+};

[Ilya Sherman, 2014/01/10 11:00:32]

How much of this code is fairly directly copied from the
//chrome/browser/metrics version of this class?  Could much of the code be
shared?

[Steven Holte, 2014/01/14 00:47:54]

Almost all of this code has some corresponding code in either MetricsService or
MetricsReportingScheduler.  I tried to write it in such a way that it could be
reused by the metrics code, but I didn't want to introduce significant UMA
changes in this CL, so this class would need to be extended a bit to work there,
and I'd prefer to defer that until a later change.  Some of the changes from
UMA: No serializing unsent logs to prefs, no prioritizing uploading initial
logs, disconnected log rotation time from upload success/failure.

+
+}  // namespace rappor
+
+#endif  // COMPONENTS_RAPPOR_LOG_UPLOADER_H_