Server push cancellation: Use TestPushDelegate in QuicChromiumClientSessionTest to cancel push.

Server push cancellation: Use TestPushDelegate in QuicChromiumClientSessionTest to cancel push.

BUG=232040
Committed: https://crrev.com/0009f3e541663d5b28251983df3f7ebc1506fbdd
Cr-Commit-Position: refs/heads/master@{#431621}

[Zhongyi Shi, 2016-11-11 01:13:08]

zhongyi@chromium.org changed reviewers:
+ ckrasic@chromium.org, rch@chromium.org

[Ryan Hamilton, 2016-11-11 01:22:57]

Looks good. One small question.

https://codereview.chromium.org/2492993002/diff/1/net/spdy/spdy_test_utils.cc
File net/spdy/spdy_test_utils.cc (right):

https://codereview.chromium.org/2492993002/diff/1/net/spdy/spdy_test_utils.cc...
net/spdy/spdy_test_utils.cc:204: return false;
Are there any cases where this should be called with a url that is not in the
map? I'd be inclined to error out in this case.

[Zhongyi Shi, 2016-11-11 01:38:46]

Update: error out TestServerPushDelegate::CancelPush if the given url is never
pushed before.

https://codereview.chromium.org/2492993002/diff/1/net/spdy/spdy_test_utils.cc
File net/spdy/spdy_test_utils.cc (right):

https://codereview.chromium.org/2492993002/diff/1/net/spdy/spdy_test_utils.cc...
net/spdy/spdy_test_utils.cc:204: return false;
On 2016/11/11 01:22:57, Ryan Hamilton wrote:
> Are there any cases where this should be called with a url that is not in the
> map? I'd be inclined to error out in this case.

It shouldn't be, unless someone is writing wrong test case. That's a good
suggestion to help write correct testcases.

[Ryan Hamilton, 2016-11-11 01:42:28]

lgtm

[Buck, 2016-11-11 18:44:51]

lgtm

[Zhongyi Shi, 2016-11-11 18:57:09]

The CQ bit was checked by zhongyi@chromium.org

[Zhongyi Shi, 2016-11-11 18:57:10]

The patchset sent to the CQ was uploaded after l-g-t-m from rch@chromium.org
Link to the patchset: https://codereview.chromium.org/2492993002/#ps40001
(title: "rebase")

[commit-bot: I haz the power, 2016-11-11 18:57:38]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-11 19:48:23]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-11-11 20:00:12]

Description was changed from

==========
Server push cancellation: Use TestPushDelegate in QuicChromiumClientSessionTest
to cancel push.

BUG=232040
==========

to

==========
Server push cancellation: Use TestPushDelegate in QuicChromiumClientSessionTest
to cancel push.

BUG=232040

Committed: https://crrev.com/0009f3e541663d5b28251983df3f7ebc1506fbdd
Cr-Commit-Position: refs/heads/master@{#431621}
==========

[commit-bot: I haz the power, 2016-11-11 20:00:13]

Patchset 3 (id:??) landed as
https://crrev.com/0009f3e541663d5b28251983df3f7ebc1506fbdd
Cr-Commit-Position: refs/heads/master@{#431621}