Don't share ResourceDispatcherHostImpl's timer for reporting upload progress.

content/browser/loader/resource_loader.cc

Index: content/browser/loader/resource_loader.cc
diff --git a/content/browser/loader/resource_loader.cc b/content/browser/loader/resource_loader.cc
index 6855cf61d2051bd1cb45c37d93262bb53ee6f720..8bc51930cdcf67dfba283e8091ddba75b09dfe7d 100644
--- a/content/browser/loader/resource_loader.cc
+++ b/content/browser/loader/resource_loader.cc
@@ -40,6 +40,9 @@ using base::TimeTicks;
 namespace content {
 namespace {
 
+// The interval for calls to ResourceLoader::ReportUploadProgress.
+const int kUploadProgressIntervalMsec = 100;
+
 void PopulateResourceResponse(ResourceRequestInfoImpl* info,
                               net::URLRequest* request,
                               ResourceResponse* response) {
@@ -161,11 +164,8 @@ void ResourceLoader::ReportUploadProgress() {
   bool too_much_time_passed = time_since_last > kOneSecond;
 
   if (is_finished || enough_new_progress || too_much_time_passed) {
-    ResourceRequestInfoImpl* info = GetRequestInfo();
-    if (info->is_upload_progress_enabled()) {
-      handler_->OnUploadProgress(progress.position(), progress.size());
-      waiting_for_upload_progress_ack_ = true;
-    }
+    handler_->OnUploadProgress(progress.position(), progress.size());
+    waiting_for_upload_progress_ack_ = true;
     last_upload_ticks_ = TimeTicks::Now();
     last_upload_position_ = progress.position();
   }
@@ -334,6 +334,8 @@ void ResourceLoader::OnResponseStarted(net::URLRequest* unused) {
 
   VLOG(1) << "OnResponseStarted: " << request_->url().spec();
 
+  progress_timer_.Stop();
+
   // The CanLoadPage check should take place after any server redirects have
   // finished, at the point in time that we know a page will commit in the
   // renderer process.
@@ -355,8 +357,10 @@ void ResourceLoader::OnResponseStarted(net::URLRequest* unused) {
   // We want to send a final upload progress message prior to sending the
   // response complete message even if we're waiting for an ack to to a
   // previous upload progress message.
-  waiting_for_upload_progress_ack_ = false;
-  ReportUploadProgress();
+  if (info->is_upload_progress_enabled()) {

[Andre, 2015/05/20 22:36:10]

I think we need to add this check.

+    waiting_for_upload_progress_ack_ = false;
+    ReportUploadProgress();
+  }
 
   CompleteResponseStarted();
 
@@ -496,6 +500,14 @@ void ResourceLoader::StartRequestInternal() {
   request_->Start();
 
   delegate_->DidStartRequest(this);
+
+  if (GetRequestInfo()->is_upload_progress_enabled()) {

[mmenke, 2015/05/21 15:41:57]

I'm paranoid.  Maybe add "&& request_->get_upload()".  Otherwise, if some change
in blink ends up in having is_upload_progress_enabled() set for non-uploads,
we'll be running a ton of timers.

(Note that we never attach an upload body after a request starts...Well,
extensions may be able to do it during a redirect or something).

[Andre, 2015/05/21 20:59:31]

Done.

+    progress_timer_.Start(
+        FROM_HERE,
+        base::TimeDelta::FromMilliseconds(kUploadProgressIntervalMsec),
+        this,
+        &ResourceLoader::ReportUploadProgress);
+  }
 }
 
 void ResourceLoader::CancelRequestInternal(int error, bool from_renderer) {