Introducing net::TestCompletionClosure.

Introducing net::TestCompletionClosure.

While deflaking some SimpleIndex tests, it became clear we needed a
variation net::TestCompletionCallback that worked work base::Closure
rather than a net::CompletionCallback.

R=jkarlin@chromium.org
BUG=255775
Committed: https://crrev.com/930649e51664c0bb51d04c5cf840b2e20e054656
Cr-Commit-Position: refs/heads/master@{#302564}

[gavinp, 2014-11-03 18:31:05]

jkarlin: This comes up in some of the deflaking we've been speaking about. WDYT?

[jkarlin, 2014-11-03 19:17:59]

https://codereview.chromium.org/699693002/diff/20001/net/base/test_completion...
File net/base/test_completion_callback.h (right):

https://codereview.chromium.org/699693002/diff/20001/net/base/test_completion...
net/base/test_completion_callback.h:85: ~TestClosureCallback();
virtual override

https://codereview.chromium.org/699693002/diff/20001/net/base/test_completion...
net/base/test_completion_callback.h:87: using
internal::TestCompletionCallbackBaseInternal::WaitForResult;
Why is this necessary?

[gavinp, 2014-11-03 22:54:49]

I found an alternative implementation of this in WebSockets, so I renamed it to
work like that. Now I'm refactoring the downstream on top of this.

QuitClosure() is deprecated, so it would be clever of us to switch those usages
to this at some point, but I don't know where to put all the TODOs.

wdyt?

https://codereview.chromium.org/699693002/diff/20001/net/base/test_completion...
File net/base/test_completion_callback.h (right):

https://codereview.chromium.org/699693002/diff/20001/net/base/test_completion...
net/base/test_completion_callback.h:85: ~TestClosureCallback();
On 2014/11/03 19:17:59, jkarlin wrote:
> virtual override

Sure. I have to update a bunch of base classes; but this is safer.

https://codereview.chromium.org/699693002/diff/20001/net/base/test_completion...
net/base/test_completion_callback.h:87: using
internal::TestCompletionCallbackBaseInternal::WaitForResult;
On 2014/11/03 19:17:59, jkarlin wrote:
> Why is this necessary?

Because TestCompletionCallbackBaseInternal::WaitForResult() is protected; this
is the syntax to readvertise it as public from this descendent class. I've moved
the "using" line up to right under the public:, as seems to be our habit.

[gavinp, 2014-11-03 23:06:38]

One question on my mind: perhaps we could just cut and paste the implementation
used in websockets (which was composed of a test completion callback) and add
test_closure.h to net/base?

There's a sense in which this is simpler though, and adding three files (.h,
.cc, _unittest.cc) seems... Overdoing it. But I'm open to suggestions.

[jkarlin, 2014-11-04 01:50:52]

lgtm

[jkarlin, 2014-11-04 01:51:54]

On 2014/11/03 23:06:38, gavinp wrote:
> One question on my mind: perhaps we could just cut and paste the
implementation
> used in websockets (which was composed of a test completion callback) and add
> test_closure.h to net/base?
> 
> There's a sense in which this is simpler though, and adding three files (.h,
> .cc, _unittest.cc) seems... Overdoing it. But I'm open to suggestions.

I prefer your way as it uses a Closure instead of a CompletionCallback.  I like
the rename though.  Let's stick with this CL.

[gavinp, 2014-11-04 02:15:45]

The CQ bit was checked by gavinp@chromium.org

[commit-bot: I haz the power, 2014-11-04 02:16:50]

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

[commit-bot: I haz the power, 2014-11-04 03:04:24]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2014-11-04 03:06:15]

Patchset 4 (id:??) landed as
https://crrev.com/930649e51664c0bb51d04c5cf840b2e20e054656
Cr-Commit-Position: refs/heads/master@{#302564}