Re: Enable and fix CRLSet and remoting tests on non-Android OpenSSL. (issue 418173004 by davidben@chromium.org)

[davi...@chromium.org]

On 2014/08/06 17:15:58, Sergey Ulanov wrote:
> On 2014/08/06 00:02:27, David Benjamin wrote:
> > +sergeyu: is building ssl_config_service.cc in net_nacl.gyp fine? It's
pulled
> in
> > to query the global CRLSet. I've filed a bug for routing it through the
> > SSLConfig since depending SSLClientSocket on SSLConfigService just for
> this
> > thing seems silly.

> I think you also need to add crl_set.cc for CRLSet destructor - that's
> why it
> fails on trybots now. In the past crl_set.cc was depending on zlib, but it
> doesn't anymore, so it should be possible to compile it for NaCl now. zlib
> dependency in crl_set.cc was the reason ssl_config_service.cc wasn't
> compiled
> for NaCl.

Done. Seems to be going through try bots this time. It's probably still
worth
removing the SSLClientSocket -> SSLConfigService dependency since that seems
unneeded, but I guess it wouldn't necessarily affect NaCl. It seems fragile
to
not build in crl_set.cc when files that are built for NaCl depend on the
type.
(Actually, it looks like cert_verifier.cc is built on NaCl but not
multi_threaded_cert_verifier.cc? How does that work? The former calls a
constructor in the latter...)

> >
> >

https://codereview.chromium.org/418173004/diff/60001/net/socket/ssl_client_socket_openssl.cc
> > File net/socket/ssl_client_socket_openssl.cc (right):
> >
> >

https://codereview.chromium.org/418173004/diff/60001/net/socket/ssl_client_socket_openssl.cc#newcode951
> > net/socket/ssl_client_socket_openssl.cc:951: // SSLClientSocket doesn't
depend
> > on SSLConfigService.
> > On 2014/08/04 23:48:46, Ryan Sleevi wrote:
> > > File a bug against a TODO like this?
> >
> > Filed https://code.google.com/p/chromium/issues/detail?id=400928
> >
> > > No clue why NaCl doesn't build the SSLConfigService. Can you fix that
> /
talk
> > > with gary||sergey?
> >
> > Added ssl_config_service.cc to net_nacl.gyp


https://codereview.chromium.org/418173004/

[davi...@chromium.org]

Friendly ping

https://codereview.chromium.org/418173004/

[ser...@chromium.org]

On 2014/08/14 22:31:51, David Benjamin wrote:
> Friendly ping

Not sure if you were pinging me or rsleevi. src/remoting LGTM.

https://codereview.chromium.org/418173004/

[rsl...@chromium.org]

Update BUILD.gn if necessary (don't believe SK), but otherwise LGTM.

Sorry, I thought I already did.

https://codereview.chromium.org/418173004/

[davi...@chromium.org]

On 2014/08/16 00:59:40, Ryan Sleevi wrote:
> Update BUILD.gn if necessary (don't believe SK), but otherwise LGTM.

> Sorry, I thought I already did.

I think BUILD.gn is fine; the file lists currently get pulled from gypi
files in
the gn build.

https://codereview.chromium.org/418173004/

[commi...@chromium.org]

CQ is trying da patch. Follow status at

https://chromium-status.appspot.com/cq/davi...@chromium.org/418173004/100001


https://codereview.chromium.org/418173004/

[commi...@chromium.org]

Committed patchset #6 (100001) as 290346

https://codereview.chromium.org/418173004/