Bulletproof download status update code.

Bulletproof download status update code.

BUG=142966
TEST=dunno repro steps; hope we crash less


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=151955

[Robert Sesek, 2012-08-16 15:30:29]

LGTM

https://chromiumcodereview.appspot.com/10831340/diff/1/chrome/browser/downloa...
File chrome/browser/download/download_status_updater_mac.mm (right):

https://chromiumcodereview.appspot.com/10831340/diff/1/chrome/browser/downloa...
chrome/browser/download/download_status_updater_mac.mm:82: result = [result
substringToIndex:[result length] - 3];
Pull @"Key" into a local so you don't have to count this manually.

https://chromiumcodereview.appspot.com/10831340/diff/1/chrome/browser/downloa...
chrome/browser/download/download_status_updater_mac.mm:198:
ProgressString(kNSProgressFileOperationKindDownloading),
nit: this should be indented 2 more

[Avi (use Gerrit), 2012-08-16 15:41:42]

https://chromiumcodereview.appspot.com/10831340/diff/1/chrome/browser/downloa...
File chrome/browser/download/download_status_updater_mac.mm (right):

https://chromiumcodereview.appspot.com/10831340/diff/1/chrome/browser/downloa...
chrome/browser/download/download_status_updater_mac.mm:82: result = [result
substringToIndex:[result length] - 3];
On 2012/08/16 15:30:29, rsesek wrote:
> Pull @"Key" into a local so you don't have to count this manually.

Done.

https://chromiumcodereview.appspot.com/10831340/diff/1/chrome/browser/downloa...
chrome/browser/download/download_status_updater_mac.mm:198:
ProgressString(kNSProgressFileOperationKindDownloading),
On 2012/08/16 15:30:29, rsesek wrote:
> nit: this should be indented 2 more

Done.

[commit-bot: I haz the power, 2012-08-16 15:42:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/avi@chromium.org/10831340/5001

[commit-bot: I haz the power, 2012-08-16 15:42:12]

Presubmit check for 10831340-5001 failed and returned exit status 1.


Running presubmit commit checks ...

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    chrome/browser/download

[benjhayden, 2012-08-16 15:52:03]

I don't know C#, but the basic idea LGTM.

https://chromiumcodereview.appspot.com/10831340/diff/5001/chrome/browser/down...
File chrome/browser/download/download_status_updater_mac.mm (right):

https://chromiumcodereview.appspot.com/10831340/diff/5001/chrome/browser/down...
chrome/browser/download/download_status_updater_mac.mm:210: if (source_url) {
Could you do this right after making |user_info| before |progress|? Seems like
that might be clearer.

[Avi (use Gerrit), 2012-08-16 17:05:30]

> I don't know C#, but the basic idea LGTM.

No problem, as this isn't C#. (It's Objective-C.)

https://chromiumcodereview.appspot.com/10831340/diff/5001/chrome/browser/down...
File chrome/browser/download/download_status_updater_mac.mm (right):

https://chromiumcodereview.appspot.com/10831340/diff/5001/chrome/browser/down...
chrome/browser/download/download_status_updater_mac.mm:210: if (source_url) {
On 2012/08/16 15:52:03, benjhayden_chromium wrote:
> Could you do this right after making |user_info| before |progress|? Seems like
> that might be clearer.

Unfortunately, NSDictionaries are not mutable so I can't easily set it there
without changing up the definition of the dictionary.

[commit-bot: I haz the power, 2012-08-16 17:05:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/avi@chromium.org/10831340/5001

[commit-bot: I haz the power, 2012-08-16 20:19:01]

Change committed as 151955