Implementation of ResourceDispatcher::OnDownloadProgress

Implementation of ResourceDispatcher::OnDownloadProgress
Implemented ResourceDispatcher::OnDownloadProgress to handle
ViewMsg_DownloadProgress IPC message, implementation is a clone of
ResourceDispatcher::OnUploadProgress because they have identical behavior.

[Alpha Left Google, 2009-03-04 02:18:26]

Implementation of ResourceDispatcher::OnDownloadProgress copying
ResourceDispatcher::OnUploadProgress, this patch
is depending on my other patch that adds the IPC messages and handlers.

[Erik does not do reviews, 2009-03-04 17:55:44]

LGTM

http://codereview.chromium.org/39104/diff/1/2
File chrome/common/resource_dispatcher.cc (right):

http://codereview.chromium.org/39104/diff/1/2#newcode285
Line 285: DLOG(WARNING) << "Got download progress for a nonexistant or "
should this be a DCHECK?

http://codereview.chromium.org/39104/diff/1/2#newcode292
Line 292: RESOURCE_LOG("Dispatching download progress for " <<
this log macro looks different than our normal style.  I'd expect it to be
RESOURCE_LOG("msg") << extra; rather than RESOURCE_LOG("msg" << extra);  Is this
correct?

http://codereview.chromium.org/39104/diff/1/2#newcode294
Line 294: request_info.peer->OnDownloadProgress(position, size);
is request_info.peer guaranteed to be non-null?  DCHECK?

http://codereview.chromium.org/39104/diff/1/2#newcode298
Line 298: if (sender)
Since the contents of the if wrap to multiple lines, use {} to make it more
clear.

[Alpha Left Google, 2009-03-05 00:32:06]

http://codereview.chromium.org/39104/diff/1/2
File chrome/common/resource_dispatcher.cc (right):

http://codereview.chromium.org/39104/diff/1/2#newcode285
Line 285: DLOG(WARNING) << "Got download progress for a nonexistant or "
On 2009/03/04 17:55:44, Erik Kay wrote:
> should this be a DCHECK?

I guess browser process can accidentally send resource messages to renderer with
invalid request_id sometimes, it would be better log it than crash the whole
renderer.

http://codereview.chromium.org/39104/diff/1/2#newcode292
Line 292: RESOURCE_LOG("Dispatching download progress for " <<
On 2009/03/04 17:55:44, Erik Kay wrote:
> this log macro looks different than our normal style.  I'd expect it to be
> RESOURCE_LOG("msg") << extra; rather than RESOURCE_LOG("msg" << extra);  Is
this
> correct?

It looks strange to me too, but seems the whole file is using this style, I
would prefer making it coherent.

http://codereview.chromium.org/39104/diff/1/2#newcode294
Line 294: request_info.peer->OnDownloadProgress(position, size);
On 2009/03/04 17:55:44, Erik Kay wrote:
> is request_info.peer guaranteed to be non-null?  DCHECK?

This assumption is made throughout this file too.

http://codereview.chromium.org/39104/diff/1/2#newcode298
Line 298: if (sender)
On 2009/03/04 17:55:44, Erik Kay wrote:
> Since the contents of the if wrap to multiple lines, use {} to make it more
> clear.

Done.