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..8bf621e0cc0fc1dade1e30b00b753629137b406b
--- /dev/null
+++ b/components/rappor/log_uploader.h
@@ -0,0 +1,92 @@
+// Copyright 2014 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/scoped_ptr.h"
+#include "base/time/time.h"
+#include "base/timer/timer.h"
+#include "net/url_request/url_fetcher_delegate.h"
+#include "net/url_request/url_request_context_getter.h"
+#include "url/gurl.h"
+
+namespace net {
+class URLFetcher;
+}
+
+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
+  // with.
+  LogUploader(const GURL& server_url,
+              const std::string& mime_type,

[Ilya Sherman, 2014/02/13 01:39:03]

Why aren't the server_url and especially the mime_type simply constants defined
in the implementation file?

[Steven Holte, 2014/02/13 05:11:12]

Server URL at least, must be a parameter for now because we will be setting it
through a Finch trial.  Since the log uploader does not otherwise make any
assumptions about the format of it's payload, it would be nice to reuse it for
other things, so that's why mime_type is a parameter.  We aren't quite able to
reuse it for UMA, yet but that's something I would like to expand it to, later.

[Ilya Sherman, 2014/02/13 23:23:08]

Hmm, why are we setting the URL via a Finch trial?  Seems like there ought to
only be one RAPPOR server, and possibly a Finch trial to control whether RAPPOR
metrics are reported at all.  I think there's actually value to not controlling
the string via Finch, as it increases transparency to users about where exactly
their data is being reported to.

[Steven Holte, 2014/02/14 02:53:28]

It gives us a little more flexibility while we are setting up the server.  Once
the server is running this can hard coded.

+              net::URLRequestContextGetter* request_context);
+
+  ~LogUploader();
+
+  // Adds an entry to the queue of logs to be uploaded to the server.  The
+  // uploader makes no assumptions about the format of |log| and simply sends
+  // it verbatim to the server.
+  void QueueLog(const std::string& log);
+
+  // Increases the upload interval each time it's called, to handle the case
+  // where the server is having issues.
+  static base::TimeDelta BackOffUploadInterval(base::TimeDelta);

[Ilya Sherman, 2014/02/13 01:39:03]

nit: This doesn't seem like it needs to be public.

[Steven Holte, 2014/02/13 05:11:12]

Changed to protected and marked exposed for tests.

+
+ protected:
+  // Schedules a future call to StartScheduledUpload if one isn't already
+  // pending.  Can be overridden for testing.
+  virtual void ScheduleNextUpload(base::TimeDelta interval);
+
+  // Check if an upload has been scheduled.
+  virtual bool IsUploadScheduled() const;
+
+  // Starts transmission of the next log. Exposed for tests.
+  void StartScheduledUpload();
+
+ private:
+  // 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 OnUploadFinished(bool server_is_healthy, bool more_logs_remaining);
+
+  // The server URL to upload logs to.
+  const GURL server_url_;
+
+  // The mime type to specify on uploaded logs.
+  const std::string mime_type_;
+
+  scoped_refptr<net::URLRequestContextGetter> request_context_;

[Ilya Sherman, 2014/02/13 01:39:03]

nit: Docs, please.

[Steven Holte, 2014/02/13 05:11:12]

Done.

+
+  // The outstanding transmission appears as a URL Fetch operation.

[Ilya Sherman, 2014/02/13 01:39:03]

nit: Should "The outstanding transmission appears" instead be "The outstanding
transmission that appears"?

[Steven Holte, 2014/02/13 05:11:12]

Done.

+  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_;

[Ilya Sherman, 2014/02/13 01:39:03]

nit: Docs, please.

[Steven Holte, 2014/02/13 05:11:12]

Done.

+
+  // Indicates that the last triggered upload hasn't resolved yet.
+  bool has_callback_pending_;
+
+  // The interval to wait after an upload's URLFetcher completion before
+  // starting the next upload attempt.
+  base::TimeDelta upload_interval_;
+
+  DISALLOW_COPY_AND_ASSIGN(LogUploader);
+};
+
+}  // namespace rappor
+
+#endif  // COMPONENTS_RAPPOR_LOG_UPLOADER_H_