Factor out common error-code conversion code for PlatformFileErrorToNetError.

Factor out common error-code conversion code for PlatformFileErrorToNetError.


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

[nhiroki (google), 2012-07-20 04:11:00]

Hi, willchan

Could you review this?
(Please see: crbug.com/136494)

Thanks,
nhiroki

[willchan no longer on Chromium, 2012-07-20 16:12:59]

LGTM with one comment I'd like you to fix.

https://chromiumcodereview.appspot.com/10808043/diff/1/net/base/net_errors.h
File net/base/net_errors.h (right):

https://chromiumcodereview.appspot.com/10808043/diff/1/net/base/net_errors.h#...
net/base/net_errors.h:55: int
PlatformFileErrorToNetError(base::PlatformFileError file_error);
How about returning a net::Error instead? I think we primarily use int because
we may return a positive # of bytes when reading from a socket or something.
Since this is already a base:: enum, converting to net::Error makes sense to me.

[nhiroki (google), 2012-07-24 13:07:57]

Thank you for your review.

I fixed based on it. PTAL.

http://codereview.chromium.org/10808043/diff/1/net/base/net_errors.h
File net/base/net_errors.h (right):

http://codereview.chromium.org/10808043/diff/1/net/base/net_errors.h#newcode55
net/base/net_errors.h:55: int
PlatformFileErrorToNetError(base::PlatformFileError file_error);
On 2012/07/20 16:12:59, willchan wrote:
> How about returning a net::Error instead? I think we primarily use int because
> we may return a positive # of bytes when reading from a socket or something.
> Since this is already a base:: enum, converting to net::Error makes sense to
me.

Done.

[willchan no longer on Chromium, 2012-07-24 14:45:57]

lgtm

http://codereview.chromium.org/10808043/diff/4001/net/base/net_errors.cc
File net/base/net_errors.cc (right):

http://codereview.chromium.org/10808043/diff/4001/net/base/net_errors.cc#newc...
net/base/net_errors.cc:47: net::Error PlatformFileErrorToNetError(
ditto on no need for net::

http://codereview.chromium.org/10808043/diff/4001/net/base/net_errors.h
File net/base/net_errors.h (right):

http://codereview.chromium.org/10808043/diff/4001/net/base/net_errors.h#newco...
net/base/net_errors.h:55: net::Error
PlatformFileErrorToNetError(base::PlatformFileError file_error);
No need for net:: since we're in the net namespace already.

[nhiroki (google), 2012-07-24 16:23:02]

Thanks!

http://codereview.chromium.org/10808043/diff/4001/net/base/net_errors.cc
File net/base/net_errors.cc (right):

http://codereview.chromium.org/10808043/diff/4001/net/base/net_errors.cc#newc...
net/base/net_errors.cc:47: net::Error PlatformFileErrorToNetError(
On 2012/07/24 14:45:57, willchan wrote:
> ditto on no need for net::

Done.

http://codereview.chromium.org/10808043/diff/4001/net/base/net_errors.h
File net/base/net_errors.h (right):

http://codereview.chromium.org/10808043/diff/4001/net/base/net_errors.h#newco...
net/base/net_errors.h:55: net::Error
PlatformFileErrorToNetError(base::PlatformFileError file_error);
On 2012/07/24 14:45:57, willchan wrote:
> No need for net:: since we're in the net namespace already.

Done.

[nhiroki (google), 2012-07-24 16:24:15]

+kinuko@

Hi, kinuko@

Could you take a look for owner LGTM?:
  webkit/blob
  webkit/fileapi
  webkit/chromeos

[kinuko, 2012-07-24 17:59:28]

Thanks for doing this!  lgtm

On 2012/07/24 16:24:15, nhiroki wrote:
> +kinuko@
> 
> Hi, kinuko@
> 
> Could you take a look for owner LGTM?:
>   webkit/blob
>   webkit/fileapi
>   webkit/chromeos

[commit-bot: I haz the power, 2012-07-24 18:35:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nhiroki@google.com/10808043/10001

[commit-bot: I haz the power, 2012-07-24 19:13:53]

Try job failure for 10808043-10001 (retry) on linux_clang for step "compile"
(clobber build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_clan...

[commit-bot: I haz the power, 2012-07-24 22:36:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nhiroki@google.com/10808043/5008

[commit-bot: I haz the power, 2012-07-25 00:10:53]

Change committed as 148255