Remove dependency on SingleThreadedProxyResolver from resolve_proxy_msg_helper_unittest

Remove dependency on SingleThreadedProxyResolver from resolve_proxy_msg_helper_unittest.cc.

Extracts MockAsyncProxyResolver to "mock_proxy_resolver.h".

This should be the last unittest that needs cleanup post r21631.

BUG=http://crbug.com/11079


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

[eroman, 2009-08-05 01:14:14]

previous publish had wrong title -- resent.

[willchan no longer on Chromium, 2009-08-05 02:28:06]

LGTM

http://codereview.chromium.org/160619/diff/1/5
File chrome/browser/net/resolve_proxy_msg_helper_unittest.cc (right):

http://codereview.chromium.org/160619/diff/1/5#newcode10
Line 10: #include "net/proxy/mock_proxy_resolver.h"
sort alphabetically

http://codereview.chromium.org/160619/diff/1/5#newcode80
Line 80: EXPECT_EQ(1u, resolver->pending_requests().size());
ASSERT_EQ

http://codereview.chromium.org/160619/diff/1/5#newcode93
Line 93: EXPECT_EQ(1u, resolver->pending_requests().size());
ASSERT_EQ

http://codereview.chromium.org/160619/diff/1/5#newcode106
Line 106: EXPECT_EQ(1u, resolver->pending_requests().size());
ASSERT_EQ

http://codereview.chromium.org/160619/diff/1/5#newcode139
Line 139: // Finish ProxyService's initialization.
Is it critical for any reason to have ProxyService initialize after the first
request has been started?  You seem to do it for all the tests, so I'm just
wondering if there's a special reason it needs to be done that way.

http://codereview.chromium.org/160619/diff/1/5#newcode147
Line 147: EXPECT_EQ(1u, resolver->pending_requests().size());
ASSERT_EQ

http://codereview.chromium.org/160619/diff/1/5#newcode159
Line 159: EXPECT_EQ(1u, resolver->pending_requests().size());
ASSERT_EQ

http://codereview.chromium.org/160619/diff/1/5#newcode171
Line 171: EXPECT_EQ(1u, resolver->pending_requests().size());
ASSERT_EQ

http://codereview.chromium.org/160619/diff/1/5#newcode203
Line 203: // Start three requests. Since the proxy resolver is async, all the
indentation is off here

[eroman, 2009-08-06 03:05:23]

thx for the review.

http://codereview.chromium.org/160619/diff/1/5
File chrome/browser/net/resolve_proxy_msg_helper_unittest.cc (right):

http://codereview.chromium.org/160619/diff/1/5#newcode10
Line 10: #include "net/proxy/mock_proxy_resolver.h"
On 2009/08/05 02:28:06, willchan wrote:
> sort alphabetically

Done.

http://codereview.chromium.org/160619/diff/1/5#newcode80
Line 80: EXPECT_EQ(1u, resolver->pending_requests().size());
On 2009/08/05 02:28:06, willchan wrote:
> ASSERT_EQ

Done.

(All such instances).

http://codereview.chromium.org/160619/diff/1/5#newcode139
Line 139: // Finish ProxyService's initialization.
On 2009/08/05 02:28:06, willchan wrote:
> Is it critical for any reason to have ProxyService initialize after the first
> request has been started?  You seem to do it for all the tests, so I'm just
> wondering if there's a special reason it needs to be done that way.

It is required, because with the MockAsyncProxyResolver dependency,
SetPacScriptByUrl() always completes asynchronously (and requires the user to
complete it).

SetPacScriptByUrl is always going to be called by ProxyService before it ships
requests into the ProxyResolver (which is where we want them to reach).

I could have avoided the need to call
resolver->pending_set_pac_script_request()->CompleteNow(net::OK) in those 2-3
places by doing something like:

class XXX : public net::MockAsyncProxyResolver {
public:
  virtual int SetPacScript(...) {
    // Always complete synchronously with success.
    return net::OK;
  }
};

But since I couldn't come up with a good name for such class I didn't bother.
If you think this style is better I can change it, but given the few occurrences
neither choice probably matters too much.

http://codereview.chromium.org/160619/diff/1/5#newcode203
Line 203: // Start three requests. Since the proxy resolver is async, all the
On 2009/08/05 02:28:06, willchan wrote:
> indentation is off here

Done.