Reduce CPU usage of download shelf UI. (both MD and pre-MD shelves)

content/browser/download/download_item_impl.cc

Index: content/browser/download/download_item_impl.cc
diff --git a/content/browser/download/download_item_impl.cc b/content/browser/download/download_item_impl.cc
index cf4294485a94276b72f032e45cb5f579e315cf18..f36b4b086c392b99038bbc65bccd5220ed6ed230 100644
--- a/content/browser/download/download_item_impl.cc
+++ b/content/browser/download/download_item_impl.cc
@@ -1038,6 +1038,8 @@ void DownloadItemImpl::DestinationUpdate(int64 bytes_so_far,
     // reporting.
     return;
   }
+
+  int previous_progress = PercentComplete();
   bytes_per_sec_ = bytes_per_sec;
   hash_state_ = hash_state;
   received_bytes_ = bytes_so_far;
@@ -1053,7 +1055,10 @@ void DownloadItemImpl::DestinationUpdate(int64 bytes_so_far,
         net::NetLog::Int64Callback("bytes_so_far", received_bytes_));
   }
 
-  UpdateObservers();
+  // Don't bother to update observers unless the progress percentage has
+  // actually changed.
+  if (previous_progress != -1 && PercentComplete() != previous_progress)

[asanka, 2015/12/18 03:39:16]

This change places the responsibility of throttling on the download guts which
are supposed to be agnostic of any specific UI. In this case, the
DownloadItemImpl will suppress OnDownloadUpdated() calls despite there being an
observable change (i.e. received_bytes_).

Shall we make the UI code ignore OnDownloadUpdated() notifications which don't
result in any noticeable changes?

If the updates are too chatty, we should throttle them at the source, which is
DownloadFileImpl. The updates are currently sent every 500ms, which I don't
think is excessive.

[Evan Stade, 2015/12/18 19:01:38]

My justification for putting the responsibility here is that it's the same class
that implements PercentComplete(), which provides an obscured view of the amount
of progress (you can't see received_bytes_ from the UI). So if the state of
DownloadItemImpl hasn't changed in any way that's observable from the outside
(modulo bytes_per_sec_), why call UpdateObservers?

The problem with moving this logic to the UI code is that there aren't enough
details in OnDownloadUpdated to easily know whether we really need to repaint.
It would be difficult for the UI to take a snapshot of the download item on
every paint and compare it to the new state of the download item in
OnDownloadUpdated. An alternative would be to break out OnDownloadUpdated into
more distinct methods (OnDownloadProgressed, etc.). I thought this solution was
the best way to minimize complexity.

All that said, this optimization is less important than the others because it
only saves us work on large/slow downloads (where it takes more than 500ms to
get 1% further along). We can probably skip this change and still get the vast
majority of the improvements noted in the CL description, the only reason I
added it was for thoroughness (I noticed the issue and it /felt/ wasteful).

[Evan Stade, 2015/12/29 18:57:03]

reverted this change

+    UpdateObservers();
 }
 
 void DownloadItemImpl::DestinationError(DownloadInterruptReason reason) {