Add crypto/wincrypt_shim.h wrapper header to resolve BoringSSL conflicts.

Add crypto/wincrypt_shim.h wrapper header to resolve BoringSSL conflicts.

wincrypt.h defines macros that conflict with BoringSSL. Introduce a
crypto/wincrypt_shim.h wrapper header which #undefs those macros and
instead #defines replacement ones. All Chromium headers should
use this wrapper header.

BUG=338884
Committed: https://crrev.com/c6ac2609dd032e468440cd4f7f86770b4cb9774d
Cr-Commit-Position: refs/heads/master@{#301994}

[davidben, 2014-10-28 20:22:05]

davidben@chromium.org changed reviewers:
+ rsleevi@chromium.org

[davidben, 2014-10-28 20:22:06]

Splitting this out from the rest of the net_unittests fix since there's a lot of
boilerplate in this one.

[Ryan Sleevi, 2014-10-29 20:40:24]

I'm nominally nervous because not including windows.h first can cause other
weird issues (notably its notorious min/max issues)

For naming, please call this wincrypt_shim or wincrypt_wrapper so that it's
clear it's a shim header.

Otherwise, LGTM, with a new nits below.

https://codereview.chromium.org/686883002/diff/20001/chrome/browser/password_...
File chrome/browser/password_manager/password_store_win_unittest.cc (right):

https://codereview.chromium.org/686883002/diff/20001/chrome/browser/password_...
chrome/browser/password_manager/password_store_win_unittest.cc:6: #include
<string>
should be a newline between 5 and 6 here

https://codereview.chromium.org/686883002/diff/20001/net/cert/x509_certificate.h
File net/cert/x509_certificate.h (right):

https://codereview.chromium.org/686883002/diff/20001/net/cert/x509_certificat...
net/cert/x509_certificate.h:24: 
no newline (here or line 28)

https://codereview.chromium.org/686883002/diff/20001/net/test/spawned_test_se...
File net/test/spawned_test_server/local_test_server_win.cc (left):

https://codereview.chromium.org/686883002/diff/20001/net/test/spawned_test_se...
net/test/spawned_test_server/local_test_server_win.cc:26: #pragma comment(lib,
"crypt32.lib")
que?

[davidben, 2014-10-29 22:49:58]

davidben@chromium.org changed reviewers:
+ thestig@chromium.org

[davidben, 2014-10-29 22:49:59]

+thestig for chrome/ and components/ OWNERS.

https://codereview.chromium.org/686883002/diff/20001/chrome/browser/password_...
File chrome/browser/password_manager/password_store_win_unittest.cc (right):

https://codereview.chromium.org/686883002/diff/20001/chrome/browser/password_...
chrome/browser/password_manager/password_store_win_unittest.cc:6: #include
<string>
On 2014/10/29 20:40:24, Ryan Sleevi wrote:
> should be a newline between 5 and 6 here

Done.

https://codereview.chromium.org/686883002/diff/20001/net/cert/x509_certificate.h
File net/cert/x509_certificate.h (right):

https://codereview.chromium.org/686883002/diff/20001/net/cert/x509_certificat...
net/cert/x509_certificate.h:24: 
On 2014/10/29 20:40:24, Ryan Sleevi wrote:
> no newline (here or line 28)

Done.

https://codereview.chromium.org/686883002/diff/20001/net/test/spawned_test_se...
File net/test/spawned_test_server/local_test_server_win.cc (left):

https://codereview.chromium.org/686883002/diff/20001/net/test/spawned_test_se...
net/test/spawned_test_server/local_test_server_win.cc:26: #pragma comment(lib,
"crypt32.lib")
On 2014/10/29 20:40:24, Ryan Sleevi wrote:
> que?

wincrypt does not appear to be used in this file. I suspect it was at one point,
but isn't anymore?

[davidben, 2014-10-29 22:52:46]

On 2014/10/29 20:40:24, Ryan Sleevi wrote:
> I'm nominally nervous because not including windows.h first can cause other
> weird issues (notably its notorious min/max issues)

Hrm. This wouldn't make that worse, would it? windows.h is still at the top.
It's true that, if you include wincrypt.h directly, it's more obvious that you
need to include windows.h, but I think you still have that problem with any
other header that includes windows.h for you. (And it doesn't look like
wincrypt.h itself pulled in windows.h. At least, if it does, it does so via
another header.)

> For naming, please call this wincrypt_shim or wincrypt_wrapper so that it's
> clear it's a shim header.

Done.

[Lei Zhang, 2014-10-29 22:55:15]

On 2014/10/29 22:49:59, David Benjamin wrote:
> +thestig for chrome/ and components/ OWNERS.

lgtm

[Ryan Sleevi, 2014-10-29 22:55:37]

On 2014/10/29 22:52:46, David Benjamin wrote:
> On 2014/10/29 20:40:24, Ryan Sleevi wrote:
> > I'm nominally nervous because not including windows.h first can cause other
> > weird issues (notably its notorious min/max issues)
> 
> Hrm. This wouldn't make that worse, would it? windows.h is still at the top.
> It's true that, if you include wincrypt.h directly, it's more obvious that you
> need to include windows.h, but I think you still have that problem with any
> other header that includes windows.h for you. (And it doesn't look like
> wincrypt.h itself pulled in windows.h. At least, if it does, it does so via
> another header.)

Correct. But your shim includes windows.h, which means people including
crypt/wincrypt_shim may fail to include windows.h and not notice things, and
then have them break at some later point. It was just a minor unease, which is
why I LGd.

[davidben, 2014-10-29 23:51:10]

The CQ bit was checked by davidben@chromium.org

[commit-bot: I haz the power, 2014-10-29 23:53:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/686883002/40001

[commit-bot: I haz the power, 2014-10-30 00:39:50]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2014-10-30 00:40:42]

Patchset 3 (id:??) landed as
https://crrev.com/c6ac2609dd032e468440cd4f7f86770b4cb9774d
Cr-Commit-Position: refs/heads/master@{#301994}