Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(202)

Issue 9363001: Eliminate DownloadProgressUpdated. (Closed)

Created:
8 years, 10 months ago by Avi (use Gerrit)
Modified:
8 years, 10 months ago
CC:
chromium-reviews, joi+watch-content_chromium.org, darin-cc_chromium.org, jam, rdsmith+dwatch_chromium.org
Visibility:
Public.

Description

Eliminate DownloadProgressUpdated. content::DownloadManagerDelegate::DownloadProgressUpdated was called out of sync with the content::DownloadItem::Observer calls. Eliminate it, as the notification that it provides is anti-useful. BUG=112841 TEST=as in bug Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=121031 Reverted: https://src.chromium.org/viewvc/chrome?view=rev&revision=121041 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=121134

Patch Set 1 #

Patch Set 2 : tests work #

Unified diffs Side-by-side diffs Delta from patch set Stats (+27 lines, -31 lines) Patch
M chrome/browser/download/chrome_download_manager_delegate.h View 1 2 chunks +1 line, -2 lines 0 comments Download
M chrome/browser/download/chrome_download_manager_delegate.cc View 1 chunk +0 lines, -13 lines 0 comments Download
M chrome/browser/download/download_status_updater.h View 1 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/browser/download/download_status_updater.cc View 3 chunks +13 lines, -0 lines 0 comments Download
M chrome/browser/download/download_status_updater_unittest.cc View 1 1 chunk +8 lines, -1 line 0 comments Download
M content/browser/download/download_manager_impl.h View 1 chunk +0 lines, -3 lines 0 comments Download
M content/browser/download/download_manager_impl.cc View 1 5 chunks +0 lines, -8 lines 0 comments Download
M content/public/browser/download_manager_delegate.h View 1 2 chunks +1 line, -4 lines 0 comments Download

Messages

Total messages: 9 (0 generated)
Avi (use Gerrit)
8 years, 10 months ago (2012-02-07 23:10:30 UTC) #1
Randy Smith (Not in Mondays)
L(V)GTM. Thanks very much for doing this--sorry to break things, but the system's a bit ...
8 years, 10 months ago (2012-02-08 18:49:12 UTC) #2
Randy Smith (Not in Mondays)
On 2012/02/08 18:49:12, rdsmith wrote: > L(V)GTM. Thanks very much for doing this--sorry to break ...
8 years, 10 months ago (2012-02-08 18:49:28 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/avi@chromium.org/9363001/1
8 years, 10 months ago (2012-02-08 18:53:07 UTC) #4
commit-bot: I haz the power
Try job failure for 9363001-1 (retry) on win_rel for step "compile" (clobber build). It's a ...
8 years, 10 months ago (2012-02-08 19:25:10 UTC) #5
Avi (use Gerrit)
Now it passes the tests.
8 years, 10 months ago (2012-02-08 21:43:53 UTC) #6
Randy Smith (Not in Mondays)
Nice hack; I'll remember it. LGTM.
8 years, 10 months ago (2012-02-08 21:45:49 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/avi@chromium.org/9363001/6003
8 years, 10 months ago (2012-02-08 21:46:52 UTC) #8
commit-bot: I haz the power
8 years, 10 months ago (2012-02-09 01:45:41 UTC) #9
Change committed as 121134

Powered by Google App Engine
This is Rietveld 408576698