net: Add NET_API to a few more files.

net: Add NET_API to a few more files.

BUG=76997
TEST=NONE
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=86181

[wtc, 2011-05-20 22:21:18]

LGTM.  I have some questions.

http://codereview.chromium.org/7056019/diff/4003/net/http/http_proxy_client_s...
File net/http/http_proxy_client_socket_pool.h (right):

http://codereview.chromium.org/7056019/diff/4003/net/http/http_proxy_client_s...
net/http/http_proxy_client_socket_pool.h:15: #include
"net/base/host_port_pair.h"
Should this file include net_api.h?

http://codereview.chromium.org/7056019/diff/4003/net/net.gyp
File net/net.gyp (right):

http://codereview.chromium.org/7056019/diff/4003/net/net.gyp#newcode825
net/net.gyp:825:
'../base/third_party/dynamic_annotations/dynamic_annotations.gyp:dynamic_annotations',
I remember you added some ynamic_annotations.gyp:dynamic _annotations
dependencies to net.gyp before.

Why do we need to add more?

Why do we need to add the googleurl.gyp:googleurl dependencies
to some executable targets?

http://codereview.chromium.org/7056019/diff/4003/net/proxy/dhcp_proxy_script_...
File net/proxy/dhcp_proxy_script_fetcher_factory.h (right):

http://codereview.chromium.org/7056019/diff/4003/net/proxy/dhcp_proxy_script_...
net/proxy/dhcp_proxy_script_fetcher_factory.h:33: class NET_API
DhcpProxyScriptFetcherFactory {
I am surprised that DhcpProxyScriptFetcherFactory needs to
be exposed to the user of our network stack.  This is a sign
that we leak too many implementation details.

Is this used by the IOThread to set up the various
URLRequestContext objects?

http://codereview.chromium.org/7056019/diff/4003/net/tools/dump_cache/dump_fi...
File net/tools/dump_cache/dump_files.cc (right):

http://codereview.chromium.org/7056019/diff/4003/net/tools/dump_cache/dump_fi...
net/tools/dump_cache/dump_files.cc:20: #include
"net/disk_cache/storage_block-inl.h"
How is this change related to building net as a DLL?

[rvargas (doing something else), 2011-05-20 22:54:55]

Thanks.

http://codereview.chromium.org/7056019/diff/4003/net/http/http_proxy_client_s...
File net/http/http_proxy_client_socket_pool.h (right):

http://codereview.chromium.org/7056019/diff/4003/net/http/http_proxy_client_s...
net/http/http_proxy_client_socket_pool.h:15: #include
"net/base/host_port_pair.h"
On 2011/05/20 22:21:19, wtc wrote:
> Should this file include net_api.h?

yes, fixed.

http://codereview.chromium.org/7056019/diff/4003/net/net.gyp
File net/net.gyp (right):

http://codereview.chromium.org/7056019/diff/4003/net/net.gyp#newcode825
net/net.gyp:825:
'../base/third_party/dynamic_annotations/dynamic_annotations.gyp:dynamic_annotations',
On 2011/05/20 22:21:19, wtc wrote:
> I remember you added some ynamic_annotations.gyp:dynamic _annotations
> dependencies to net.gyp before.
> 
> Why do we need to add more?
> 
> Why do we need to add the googleurl.gyp:googleurl dependencies
> to some executable targets?

Yes, but that was to net(.dll). For a static_library that would be enough for
the linker to find the right code for anybody linking against net, but now that
net_unittest only sees the exported interface of net (and not its
implementation) the linker doesn't see dyn_annotations. And of course, files
inside net_unittests are using dyn_annotations.

The answer is basically the same for gurl... now it will not come through net so
if an exe uses gurl, it has to depend on it.

http://codereview.chromium.org/7056019/diff/4003/net/proxy/dhcp_proxy_script_...
File net/proxy/dhcp_proxy_script_fetcher_factory.h (right):

http://codereview.chromium.org/7056019/diff/4003/net/proxy/dhcp_proxy_script_...
net/proxy/dhcp_proxy_script_fetcher_factory.h:33: class NET_API
DhcpProxyScriptFetcherFactory {
chrome/browser/net/connection_tester.cc and
chrome/browser/net/proxy_service_factory.cc use it to setup the proxy service.

On 2011/05/20 22:21:19, wtc wrote:
> I am surprised that DhcpProxyScriptFetcherFactory needs to
> be exposed to the user of our network stack.  This is a sign
> that we leak too many implementation details.
> 
> Is this used by the IOThread to set up the various
> URLRequestContext objects?

http://codereview.chromium.org/7056019/diff/4003/net/tools/dump_cache/dump_fi...
File net/tools/dump_cache/dump_files.cc (right):

http://codereview.chromium.org/7056019/diff/4003/net/tools/dump_cache/dump_fi...
net/tools/dump_cache/dump_files.cc:20: #include
"net/disk_cache/storage_block-inl.h"
On 2011/05/20 22:21:19, wtc wrote:
> How is this change related to building net as a DLL?

I really don't understand how it works for the static case: the template is
supposed to be fully visible by the compiler at the time that a given cc file is
being compiled, but apparently it is happy to rely on the linker to find the
implementation of the few missing methods (they are part of net.lib, and this
exe uses net.lib). I would have expected an error in any configuration.

With net.dll there is no way for the linker to see the missing methods because
the template is not exported.