Add UploadProgressTracker unittest

content/browser/loader/upload_progress_tracker.h

Index: content/browser/loader/upload_progress_tracker.h
diff --git a/content/browser/loader/upload_progress_tracker.h b/content/browser/loader/upload_progress_tracker.h
index 4172490b35795d0a6051ace5ac8d9f3a01285435..2c563870aa041c9a17d759c34d68ee76c40dc640 100644
--- a/content/browser/loader/upload_progress_tracker.h
+++ b/content/browser/loader/upload_progress_tracker.h
@@ -7,48 +7,66 @@
 
 #include <stdint.h>
 
-#include "base/callback.h"
 #include "base/macros.h"
+#include "base/memory/ref_counted.h"
 #include "base/time/time.h"
 #include "base/timer/timer.h"
+#include "content/common/content_export.h"
+
+namespace base {
+class SingleThreadTaskRunner;
+}
 
 namespace tracked_objects {
 class Location;
 }
 
 namespace net {
-class URLRequest;
+class UploadProgress;
 }
 
 namespace content {
 
 // UploadProgressTracker watches the upload progress of a URL loading, and sends
 // the progress to the client in a suitable granularity and frequency.
-class UploadProgressTracker final {
+class CONTENT_EXPORT UploadProgressTracker {
  public:
-  using UploadProgressReportCallback =
-      base::RepeatingCallback<void(int64_t, int64_t)>;
-
-  UploadProgressTracker(const tracked_objects::Location& location,
-                        UploadProgressReportCallback report_progress,
-                        net::URLRequest* request);
+  class CONTENT_EXPORT Client {
+   public:
+    virtual net::UploadProgress GetUploadProgress() = 0;

[mmenke, 2017/01/11 19:31:13]

Since we're returning this (And not just by ref or by value), should be be
including the header instead of forward declaring it?

[tzik, 2017/01/12 13:00:57]

Done.

+    virtual void ReportUploadProgress(int64_t current_position,
+                                      int64_t total_size) = 0;

[mmenke, 2017/01/11 19:31:13]

Should document these.  Also, why doesn't ReporterUploadProgress just take a
net::UploadProgress?

[tzik, 2017/01/12 13:00:56]

Done.

Updated the reporting function to take a net::UploadProgress.

+
+   protected:
+    Client();
+    virtual ~Client();
+
+   private:
+    DISALLOW_COPY_AND_ASSIGN(Client);
+  };
+
+  UploadProgressTracker(
+      const tracked_objects::Location& location,
+      Client* client,
+      scoped_refptr<base::SingleThreadTaskRunner> task_runner);
   ~UploadProgressTracker();
 
   void OnAckReceived();
   void OnUploadCompleted();
 
+ protected:
+  virtual base::TimeTicks GetCurrentTime();

[mmenke, 2017/01/11 19:31:13]

Maybe add a comment that this can be overridden for testing?

[mmenke, 2017/01/11 19:31:13]

const?

[mmenke, 2017/01/11 19:31:13]

nit:  Can be private (Subclasses can override private functions, they just can't
call them (Unless they also override them, though not relevant here, anyways))

[tzik, 2017/01/12 13:00:56]

Done.

[tzik, 2017/01/12 13:00:56]

Done.

[tzik, 2017/01/12 13:00:57]

Done.

+
  private:
   void ReportUploadProgressIfNeeded();
 
-  net::URLRequest* request_;  // Not owned.
+  Client* client_;  // Not owned.
 
   uint64_t last_upload_position_ = 0;
   bool waiting_for_upload_progress_ack_ = false;
   base::TimeTicks last_upload_ticks_;
   base::RepeatingTimer progress_timer_;
 
-  UploadProgressReportCallback report_progress_;
-
   DISALLOW_COPY_AND_ASSIGN(UploadProgressTracker);
 };