Add base::HostToNetXX() & NetToHostXX(), and use them to replace htonX() & ntohX() in Chrome.

Add base::HostToNetXX() & NetToHostXX(), and use them to replace htonX() & ntohX() in Chrome.

This primarily addresses issues with code using the OS-provided htonX() & ntohX() functions from within the Chrome sandbox.  Under Windows these functions are provided by ws2_32.dll, which is no longer available within Chrome's sandbox.

The new base::HostToNetXX() and NetToHostXX() functions are safe for use by sandboxed code on Windows, and provide a single place where future fixes for other platforms can be made.

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

[Wez, 2012-03-21 16:44:34]

PTAL!

Most of this CL is a(n annoyingly manual) search-and-replace of
htons->HostToNet16 and so-on.  The meat is in base/sys_byteorder.h, which
follows the pattern of the htonll/ntohll functions that were already in there,
rather than the way the Talk code now does it.

[rvargas (doing something else), 2012-03-21 18:47:41]

http://codereview.chromium.org/9716020/diff/12001/sandbox/sandbox_poc/pocdll/...
File sandbox/sandbox_poc/pocdll/network.cc (right):

http://codereview.chromium.org/9716020/diff/12001/sandbox/sandbox_poc/pocdll/...
sandbox/sandbox_poc/pocdll/network.cc:38: service.sin_port =
base::HostToNet16(88);
I'm on the fence about this particular change. POC basically does not depend on
base, and it is used to showcase the sandbox functionality (in other words it is
not really used for anything on a regular Chrome build).

So in a way, this is not needed and it may mean more work in the future. Now,
philosophically, given that the point of the change is to double-fix an issue
with the sandbox access, it may be better not to touch a sandbox demo.

[Wez, 2012-03-21 19:22:05]

On 2012/03/21 18:47:41, rvargas wrote:
>
http://codereview.chromium.org/9716020/diff/12001/sandbox/sandbox_poc/pocdll/...
> File sandbox/sandbox_poc/pocdll/network.cc (right):
> 
>
http://codereview.chromium.org/9716020/diff/12001/sandbox/sandbox_poc/pocdll/...
> sandbox/sandbox_poc/pocdll/network.cc:38: service.sin_port =
> base::HostToNet16(88);
> I'm on the fence about this particular change. POC basically does not depend
on
> base, and it is used to showcase the sandbox functionality (in other words it
is
> not really used for anything on a regular Chrome build).
> 
> So in a way, this is not needed and it may mean more work in the future. Now,
> philosophically, given that the point of the change is to double-fix an issue
> with the sandbox access, it may be better not to touch a sandbox demo.

Didn't realise that was a demo. Happy to revert that bit of the patch.

[Wez, 2012-03-21 20:09:03]

http://codereview.chromium.org/9716020/diff/12001/sandbox/sandbox_poc/pocdll/...
File sandbox/sandbox_poc/pocdll/network.cc (right):

http://codereview.chromium.org/9716020/diff/12001/sandbox/sandbox_poc/pocdll/...
sandbox/sandbox_poc/pocdll/network.cc:38: service.sin_port =
base::HostToNet16(88);
On 2012/03/21 18:47:41, rvargas wrote:
> I'm on the fence about this particular change. POC basically does not depend
on
> base, and it is used to showcase the sandbox functionality (in other words it
is
> not really used for anything on a regular Chrome build).
> 
> So in a way, this is not needed and it may mean more work in the future. Now,
> philosophically, given that the point of the change is to double-fix an issue
> with the sandbox access, it may be better not to touch a sandbox demo.

Done.

[willchan no longer on Chromium, 2012-03-22 01:41:16]

Please undo changes to net/spdy/spdy_frame* and net/spdy/spdy_protocol.h as they
are pulled in from google3. We're not going to actually use SPDY from a
sandboxed process, so this should be OK, right? Sorry for being annoying, but
this will make it easier for us to integrate cleanly.

[Wez, 2012-03-22 05:09:23]

On 2012/03/22 01:41:16, willchan wrote:
> Please undo changes to net/spdy/spdy_frame* and net/spdy/spdy_protocol.h as
they
> are pulled in from google3. We're not going to actually use SPDY from a
> sandboxed process, so this should be OK, right? Sorry for being annoying, but
> this will make it easier for us to integrate cleanly.

Done.

If these mirror code maintained outside Chrome, don't they belong in a
third_party/ directory?

[willchan no longer on Chromium, 2012-03-22 05:32:19]

On Wed, Mar 21, 2012 at 10:09 PM, <wez@chromium.org> wrote:

> On 2012/03/22 01:41:16, willchan wrote:
>
>> Please undo changes to net/spdy/spdy_frame* and net/spdy/spdy_protocol.h
>> as
>>
> they
>
>> are pulled in from google3. We're not going to actually use SPDY from a
>> sandboxed process, so this should be OK, right? Sorry for being annoying,
>> but
>> this will make it easier for us to integrate cleanly.
>>
>
> Done.
>
> If these mirror code maintained outside Chrome, don't they belong in a
> third_party/ directory?
>

It's complicated :)


>
>
http://codereview.chromium.**org/9716020/<http://codereview.chromium.org/9716...
>

[willchan no longer on Chromium, 2012-03-22 21:33:40]

lgtm

[commit-bot: I haz the power, 2012-03-22 21:51:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wez@chromium.org/9716020/21001

[commit-bot: I haz the power, 2012-03-22 21:52:19]

Presubmit check for 9716020-21001 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Messages **
If this change requires manual test instructions to QA team, add
TEST=[instructions].

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    ppapi
    native_client_sdk/src
    sync
    crypto
    content/browser/renderer_host/p2p

Presubmit checks took 6.9s to calculate.

[Wez, 2012-03-22 22:13:44]

dmichael: OWNERS for ppapi/, plz
noelallan1: same for native_client_sdk/src, plz
akalin: ditto for sync/
rsleevi: guess what... OWNERS for crypto/, plz
sergeyu: last but not least, content/browser/renderer_host/p2p/ plz

:)

[Ryan Sleevi, 2012-03-22 22:29:10]

lgtm

[dmichael (off chromium), 2012-03-22 22:48:12]

ppapi lgtm

[akalin, 2012-03-22 23:02:03]

jingle/sync lgtm

[noelallen1, 2012-03-24 17:43:27]

lgtm

I do not believe the version in native_client_sdk is being used.  It probably
got pulled in when we moved into the Chrome repo.  However, I there IS version
in the native client repo, so I you may need to change it there and roll deps.

[Wez, 2012-03-27 21:06:56]

On 2012/03/24 17:43:27, noelallen1 wrote:
> lgtm
> 
> I do not believe the version in native_client_sdk is being used.  It probably
> got pulled in when we moved into the Chrome repo.  However, I there IS version
> in the native client repo, so I you may need to change it there and roll deps.

I've reverted the change to the test under native_client_sdk/.  That bit of the
patch isn't critical.

[commit-bot: I haz the power, 2012-03-27 22:46:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wez@chromium.org/9716020/34001

[commit-bot: I haz the power, 2012-03-27 22:47:13]

Presubmit check for 9716020-34001 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Messages **
If this change requires manual test instructions to QA team, add
TEST=[instructions].

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    content/browser/renderer_host/p2p

Presubmit checks took 2.7s to calculate.

Was the presubmit check useful? Please send feedback & hate mail to
maruel@chromium.org!

[Wez, 2012-03-27 23:09:02]

On 2012/03/27 22:47:13, I haz the power (commit-bot) wrote:
> Presubmit check for 9716020-34001 failed and returned exit status 1.
> 
> Running presubmit commit checks ...
> 
> ** Presubmit Messages **
> If this change requires manual test instructions to QA team, add
> TEST=[instructions].
> 
> ** Presubmit ERRORS **
> Missing LGTM from an OWNER for files in these directories:
>     content/browser/renderer_host/p2p
> 
> Presubmit checks took 2.7s to calculate.
> 
> Was the presubmit check useful? Please send feedback & hate mail to
> maruel@chromium.org!

sergeyu: Ping!

[Sergey Ulanov, 2012-03-27 23:17:52]

lgtm

[commit-bot: I haz the power, 2012-03-28 00:46:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wez@chromium.org/9716020/34001

[commit-bot: I haz the power, 2012-03-28 01:08:49]

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

[commit-bot: I haz the power, 2012-03-28 16:26:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wez@chromium.org/9716020/42001

[commit-bot: I haz the power, 2012-03-28 20:19:35]

Change committed as 129476