Stop FetchDataLoaderTest from using [Web|Fetch]DataConsumerHandle.

Stop FetchDataLoaderTest from using [Web|Fetch]DataConsumerHandle.

With this CL, FetchDataLoaderTest uses BytesConsumer interface instead
of [Web|Fetch]DataConsumerHandle.

BUG=610195
Committed: https://crrev.com/10c6fdcbc485d6ba37eb8ad21830dde144b99920
Cr-Commit-Position: refs/heads/master@{#422114}

[yhirano, 2016-09-30 06:46:42]

The CQ bit was checked by yhirano@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-30 06:46:53]

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

[yhirano, 2016-09-30 06:48:42]

The CQ bit was checked by yhirano@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-30 06:48:53]

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

[yhirano, 2016-09-30 06:50:25]

Description was changed from

==========
Stop FetchDataLoaderTest from using [Web|Fetch]DataConsumerHandle.

BUG=610195
==========

to

==========
Stop FetchDataLoaderTest from using [Web|Fetch]DataConsumerHandle.

With this CL, FetchDataLoaderTest uses BytesConsumer interface instead
of [Web|Fetch]DataConsumerHandle.

BUG=610195
==========

[yhirano, 2016-09-30 06:50:25]

yhirano@chromium.org changed reviewers:
+ hiroshige@chromium.org

[commit-bot: I haz the power, 2016-09-30 08:01:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-30 08:01:37]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[hiroshige, 2016-09-30 08:16:48]

Basically looks good.

https://codereview.chromium.org/2376193004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/fetch/FetchDataLoaderTest.cpp (right):

https://codereview.chromium.org/2376193004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/fetch/FetchDataLoaderTest.cpp:55:
EXPECT_CALL(*consumer, cancel());
Memo: In this and the following tests, there are two patterns:

1. Previously there's EXPECT_CALL(destruct()), and this CL replaces it with
EXPECT_CALL(cancel()).
This is/was testing that BytesConsumer/FetchDataConsumerHandle's loading is
cancelled.

2. Previously there's not EXPECT_CALL(), and this CL adds EXPECT_CALL(cancel()).
This was testing that FetchDataConsumerHandle's loading was already
cancelled/completed and thus no further action was taken when FetchDataLoader is
cancelled.
However, BytesConsumer::cancel() is always called, and whether some actions are
taken for actually cancelling the loading depends on BytesConsumer (and
MockBytesConsumer lacks such logic).

I think this CL is OK, because from the sequence of EXPECT_CALL()s before
cancel() and EXPECT_CALL(cancel()), we can infer that getPublicState() would be
ReadableOrWaiting for Cases 1 and Closed/Errored for Cases 2.

https://codereview.chromium.org/2376193004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/fetch/FetchDataLoaderTest.cpp:64:
consumer->cancel();
I think |fetchDataLoader->cancel();| should be called here instead.

[hiroshige, 2016-09-30 08:17:45]

On 2016/09/30 08:16:48, hiroshige wrote:
> Basically looks good.
> 
>
https://codereview.chromium.org/2376193004/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/modules/fetch/FetchDataLoaderTest.cpp (right):
> 
>
https://codereview.chromium.org/2376193004/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/modules/fetch/FetchDataLoaderTest.cpp:55:
> EXPECT_CALL(*consumer, cancel());
> Memo: In this and the following tests, there are two patterns:
> 
> 1. Previously there's EXPECT_CALL(destruct()), and this CL replaces it with
> EXPECT_CALL(cancel()).
> This is/was testing that BytesConsumer/FetchDataConsumerHandle's loading is
> cancelled.
> 
> 2. Previously there's not EXPECT_CALL(), and this CL adds
EXPECT_CALL(cancel()).
> This was testing that FetchDataConsumerHandle's loading was already
> cancelled/completed and thus no further action was taken when FetchDataLoader
is
> cancelled.
> However, BytesConsumer::cancel() is always called, and whether some actions
are
> taken for actually cancelling the loading depends on BytesConsumer (and
> MockBytesConsumer lacks such logic).
> 
> I think this CL is OK, because from the sequence of EXPECT_CALL()s before
> cancel() and EXPECT_CALL(cancel()), we can infer that getPublicState() would
be
> ReadableOrWaiting for Cases 1 and Closed/Errored for Cases 2.
> 
>
https://codereview.chromium.org/2376193004/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/modules/fetch/FetchDataLoaderTest.cpp:64:
> consumer->cancel();
> I think |fetchDataLoader->cancel();| should be called here instead.

Er, FormDataBytesConsumerTest.BeginReadAffectsDrainingWithComplexFormData is
failing.

[yhirano, 2016-09-30 09:10:51]

The CQ bit was checked by yhirano@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-30 09:11:06]

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

[yhirano, 2016-09-30 09:15:06]

The CQ bit was checked by yhirano@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-30 09:15:14]

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

[yhirano, 2016-09-30 09:18:09]

https://codereview.chromium.org/2376193004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/fetch/FetchDataLoaderTest.cpp (right):

https://codereview.chromium.org/2376193004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/fetch/FetchDataLoaderTest.cpp:64:
consumer->cancel();
On 2016/09/30 08:16:48, hiroshige wrote:
> I think |fetchDataLoader->cancel();| should be called here instead.

Done.

https://codereview.chromium.org/2376193004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/fetch/FormDataBytesConsumerTest.cpp
(right):

https://codereview.chromium.org/2376193004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/fetch/FormDataBytesConsumerTest.cpp:310: //
drainAsFormData / drainAsBlobDataHandle should not be called here.
I think this expectation was wrong. Calling drainAsBLobDataHandle is not a
problem because it will return null as beginRead or cancel has been requested.

MockBytesConsumer defined this class didn't have drainAsFormData and
drainAsBlobDataHandle implementations, so this test had been broken anyway.

[hiroshige, 2016-09-30 09:57:59]

lgtm.

[commit-bot: I haz the power, 2016-09-30 10:29:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-30 10:29:57]

Dry run: This issue passed the CQ dry run.

[yhirano, 2016-09-30 11:41:52]

The CQ bit was checked by yhirano@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-30 11:42:12]

Dry run: 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-09-30 12:33:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-30 12:33:48]

Dry run: This issue passed the CQ dry run.

[yhirano, 2016-09-30 14:59:22]

The CQ bit was checked by yhirano@chromium.org

[yhirano, 2016-09-30 14:59:22]

The patchset sent to the CQ was uploaded after l-g-t-m from
hiroshige@chromium.org
Link to the patchset: https://codereview.chromium.org/2376193004/#ps80001
(title: "fix")

[commit-bot: I haz the power, 2016-09-30 14:59:49]

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-09-30 15:05:04]

Description was changed from

==========
Stop FetchDataLoaderTest from using [Web|Fetch]DataConsumerHandle.

With this CL, FetchDataLoaderTest uses BytesConsumer interface instead
of [Web|Fetch]DataConsumerHandle.

BUG=610195
==========

to

==========
Stop FetchDataLoaderTest from using [Web|Fetch]DataConsumerHandle.

With this CL, FetchDataLoaderTest uses BytesConsumer interface instead
of [Web|Fetch]DataConsumerHandle.

BUG=610195
==========

[commit-bot: I haz the power, 2016-09-30 15:05:05]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-09-30 15:07:22]

Description was changed from

==========
Stop FetchDataLoaderTest from using [Web|Fetch]DataConsumerHandle.

With this CL, FetchDataLoaderTest uses BytesConsumer interface instead
of [Web|Fetch]DataConsumerHandle.

BUG=610195
==========

to

==========
Stop FetchDataLoaderTest from using [Web|Fetch]DataConsumerHandle.

With this CL, FetchDataLoaderTest uses BytesConsumer interface instead
of [Web|Fetch]DataConsumerHandle.

BUG=610195

Committed: https://crrev.com/10c6fdcbc485d6ba37eb8ad21830dde144b99920
Cr-Commit-Position: refs/heads/master@{#422114}
==========

[commit-bot: I haz the power, 2016-09-30 15:07:23]

Patchset 5 (id:??) landed as
https://crrev.com/10c6fdcbc485d6ba37eb8ad21830dde144b99920
Cr-Commit-Position: refs/heads/master@{#422114}