Remove unused HostResolver::Observer.

Remove unused HostResolver::Observer.

BUG=
TEST=compiles, ./net_unittests


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

[eroman, 2011-11-14 20:01:39]

- If the Observer is gone, I wander if the "referrer" field can be removed (as I
recall that was introduced by the DNS prefetcher observer).

- I was sure the DNS predictor was using the observer mechanism, I wander when
that changed.

- Glad to see the observer interface go!

http://codereview.chromium.org/8533011/diff/1/net/base/host_resolver_impl.h
File net/base/host_resolver_impl.h (right):

http://codereview.chromium.org/8533011/diff/1/net/base/host_resolver_impl.h#n...
net/base/host_resolver_impl.h:330: int next_request_id_;
Can you delete this too? Or maybe it is still used by logging. In that case,
please update the comment to remove mention of "Observers".

[szym, 2011-11-14 23:50:16]

Thanks for the review!

DNS predictor uses ConnectInterceptor in place of HostResolver::Observer since:
http://src.chromium.org/viewvc/chrome?view=rev&revision=56163

http://codereview.chromium.org/8533011/diff/1/net/base/host_resolver_impl.h
File net/base/host_resolver_impl.h (right):

http://codereview.chromium.org/8533011/diff/1/net/base/host_resolver_impl.h#n...
net/base/host_resolver_impl.h:330: int next_request_id_;
On 2011/11/14 20:01:39, eroman wrote:
> Can you delete this too? Or maybe it is still used by logging. In that case,
> please update the comment to remove mention of "Observers".

Currently HostResolverImpl does not use it, so I removed it.

However, I'm inclined to use an id as the RequestHandle (see
MockHostResolverBase). I'm not a fan of using opaque Request* (although it's
fast), because calling CancelRequest(req) after |req| has already been deleted
is scary. Perhaps it would help to document for the consumers that the
RequestHandle must not be used after the CompletionCallback is run.

[szym, 2011-11-14 23:52:50]

http://codereview.chromium.org/8533011/diff/39/net/net.gyp
File net/net.gyp (left):

http://codereview.chromium.org/8533011/diff/39/net/net.gyp#oldcode1315
net/net.gyp:1315: '../build/temp_gyp/googleurl.gyp:googleurl',
This was added to satisfy GURL dependency in compiler-generated
RequestInfo(const RequestInfo&), but since GURL has been removed with referrer,
this is no longer needed. I'll confirm by trying _shared builds.

[eroman, 2011-11-15 03:54:01]

lgtm

http://codereview.chromium.org/8533011/diff/39/net/socket/socks_client_socket...
File net/socket/socks_client_socket_pool.cc (right):

http://codereview.chromium.org/8533011/diff/39/net/socket/socks_client_socket...
net/socket/socks_client_socket_pool.cc:25: const GURL& referrer)
please add a TODO to remove the referrer.

http://codereview.chromium.org/8533011/diff/39/net/socket/transport_client_so...
File net/socket/transport_client_socket_pool.cc (right):

http://codereview.chromium.org/8533011/diff/39/net/socket/transport_client_so...
net/socket/transport_client_socket_pool.cc:74: const GURL& referrer,
Please add a todo to remove |referrer| as it is no longer used.

[cbentzel, 2011-11-15 10:49:01]

Nice cleanup!

http://codereview.chromium.org/8533011/diff/1/net/base/host_resolver_impl.h
File net/base/host_resolver_impl.h (right):

http://codereview.chromium.org/8533011/diff/1/net/base/host_resolver_impl.h#n...
net/base/host_resolver_impl.h:330: int next_request_id_;
On 2011/11/14 23:50:17, szym wrote:
> On 2011/11/14 20:01:39, eroman wrote:
> > Can you delete this too? Or maybe it is still used by logging. In that case,
> > please update the comment to remove mention of "Observers".
> 
> Currently HostResolverImpl does not use it, so I removed it.
> 
> However, I'm inclined to use an id as the RequestHandle (see
> MockHostResolverBase). I'm not a fan of using opaque Request* (although it's
> fast), because calling CancelRequest(req) after |req| has already been deleted
> is scary. Perhaps it would help to document for the consumers that the
> RequestHandle must not be used after the CompletionCallback is run.

You could always maintain a std::set<Request*> in the Host Resolver
implementations, and only cancel if it's present there.

http://codereview.chromium.org/8533011/diff/39/net/base/mock_host_resolver.cc
File net/base/mock_host_resolver.cc (right):

http://codereview.chromium.org/8533011/diff/39/net/base/mock_host_resolver.cc...
net/base/mock_host_resolver.cc:92: *handle =
reinterpret_cast<RequestHandle>(id);
Can you remove next_request_id_ from the mock by having handle be a
reinterpret_cast of |req| (like HostResolverImpl) and changing requests_ to be a
std::set<Request*>?

http://codereview.chromium.org/8533011/diff/39/net/dns/async_host_resolver.cc
File net/dns/async_host_resolver.cc (right):

http://codereview.chromium.org/8533011/diff/39/net/dns/async_host_resolver.cc...
net/dns/async_host_resolver.cc:395: int id = next_request_id_++;
Could you remove id here, and from Request?

[commit-bot: I haz the power, 2011-11-15 16:38:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/szym@chromium.org/8533011/11001

[commit-bot: I haz the power, 2011-11-15 17:34:27]

Change committed as 110109