Avoid loading ws2_32.dll just to get htons() and friends.

Avoid loading ws2_32.dll just to get htons() and friends.

Chrome does not load ws2_32.dll into the renderer sandbox, so code calling these functions would fail attempting to load the DLL.  This CL replaces the calls with direct use of the MSVC byte-swap intrinsics.

BUG=115477, 116591
TEST=Chromoting client does not crash on connect on Windows.


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

[Wez, 2012-03-06 00:52:39]

Quick-fix for the Chromoting crashers.

I'll follow up with a CL that instead defines base::htons() and pals, once I get
workaround the macro-hack that was added to libjingle...

[Wez, 2012-03-06 01:43:13]

PTAL

[willchan no longer on Chromium, 2012-03-06 01:44:59]

Do you need the base/ change (it seems to be whitespace only)? the net/ change
LGTM, although I think you should add TODOs to indicate they are supposed to be
removed. I can't approve the crypto/ change.

[Wez, 2012-03-06 01:52:09]

rsleevi: crypto/ approval, plz

[Wez, 2012-03-06 01:53:21]

On 2012/03/06 01:44:59, willchan wrote:
> Do you need the base/ change (it seems to be whitespace only)? the net/ change
> LGTM, although I think you should add TODOs to indicate they are supposed to
be
> removed. I can't approve the crypto/ change.

Done.

[Ryan Sleevi, 2012-03-06 01:56:47]

See the original discussion on http://codereview.chromium.org/8949026

I don't think there is opposition to adding the inline wrappers, especially as
we're trying to weed out dependencies on ws2_32.dll (eg: OTS removed one just
recently for the same reason, ntohs/ntohl)

http://codereview.chromium.org/9614004/diff/2002/crypto/p224.cc
File crypto/p224.cc (right):

http://codereview.chromium.org/9614004/diff/2002/crypto/p224.cc#newcode21
crypto/p224.cc:21: #endif // OS_WIN
This is the second or third time I've seen this pattern, which makes me think it
would be better served by updating base/sys_byteorder.h

After all, http://msdn.microsoft.com/en-us/library/a3140177(v=vs.80).aspx

[Ryan Sleevi, 2012-03-06 01:57:39]

Ah, just saw comment 1 under the fold.

LGTM as a quick fix to be followed by the real fix (although I'd vote for the
real fix as I expect you can get buy in in < 24 hours)

[Wez, 2012-03-06 02:00:15]

On 2012/03/06 01:57:39, Ryan Sleevi wrote:
> Ah, just saw comment 1 under the fold.
> 
> LGTM as a quick fix to be followed by the real fix (although I'd vote for the
> real fix as I expect you can get buy in in < 24 hours)

The real fix is non-trivial thanks to the macro hack approach being applied in a
libjingle header override - the macros prevent equivalent inlines in base/ from
compiling...

[willchan no longer on Chromium, 2012-03-06 02:00:19]

Sorry Ryan, wez@ and I chatted about this offline. I agreed with you, so I
asked wez@ to put the hacks in more targeted places, to avoid adding to
base/ headers. I trust wez@ to remove the hacks asap.

On Mon, Mar 5, 2012 at 5:57 PM, <rsleevi@chromium.org> wrote:

> Ah, just saw comment 1 under the fold.
>
> LGTM as a quick fix to be followed by the real fix (although I'd vote for
> the
> real fix as I expect you can get buy in in < 24 hours)
>
>
http://codereview.chromium.**org/9614004/<http://codereview.chromium.org/9614...
>

[commit-bot: I haz the power, 2012-03-06 02:26:08]

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

[commit-bot: I haz the power, 2012-03-06 04:34:25]

Change committed as 125110