[Downloads] Stop keying TestFileErrorInjector off of URLs.

[Downloads] Stop keying TestFileErrorInjector off of URLs.

DownloadFiles don't typically care about URLs. Hence the
TestFileErrorInjector shouldn't be using URLs as a key for looking up
injected errors.

Incidentally, none of the tests using TestFileErrorInjector depend on
there being a 1-1 mapping between download URLs and injected errors.
Hence, this CL removes the role that URLs were playing in
TestFileErrorInjector.

R=svaldez@chromium.org, davidben@chromium.org
BUG=7648
Committed: https://crrev.com/43b66935f03e974fe5a052fd8382e7611b60ece6
Cr-Commit-Position: refs/heads/master@{#378944}

[asanka, 2016-03-01 16:11:38]

svaldez: downloads stuffz
davidben: content/public/test stuffz

The URL is being passed into the DownloadFile solely for the final file source
annotation. It was being used here as a key for the TestFileErrorInjector.

I'm reworking the hash generation logic so that we don't need to persist hash
state in history for resumption. As a part of that work I need to clean up the
DownloadFile interface so that we aren't stuffing it with more state than
necessary. I can't remove the URL dependency without fixing this test fixture to
not use it.

https://codereview.chromium.org/1750943002/diff/60001/content/public/test/tes...
File content/public/test/test_file_error_injector.cc (right):

https://codereview.chromium.org/1750943002/diff/60001/content/public/test/tes...
content/public/test/test_file_error_injector.cc:317: // DownloadManager is
always a DownloadManagerImpl.
This is .. special. I'm going to move ownership of download files to download
item, then we'd need to rip out most of this logic anyway. So not going to
address this now.

[svaldez, 2016-03-01 18:01:49]

https://codereview.chromium.org/1750943002/diff/60001/content/public/test/tes...
File content/public/test/test_file_error_injector.cc (right):

https://codereview.chromium.org/1750943002/diff/60001/content/public/test/tes...
content/public/test/test_file_error_injector.cc:301: injected_errors_.empty() ?
kNoOpErrorInfo : injected_errors_.front();
I might be misunderstanding the code, but don't you want pop_front here to cycle
the deque?

https://codereview.chromium.org/1750943002/diff/60001/content/public/test/tes...
File content/public/test/test_file_error_injector.h (right):

https://codereview.chromium.org/1750943002/diff/60001/content/public/test/tes...
content/public/test/test_file_error_injector.h:120: ErrorQueue injected_errors_;
Possibly assert on empty upon destruction?

[asanka, 2016-03-01 18:34:16]

Patchset #5 (id:80001) has been deleted

[asanka, 2016-03-01 18:58:51]

https://codereview.chromium.org/1750943002/diff/60001/content/public/test/tes...
File content/public/test/test_file_error_injector.cc (right):

https://codereview.chromium.org/1750943002/diff/60001/content/public/test/tes...
content/public/test/test_file_error_injector.cc:301: injected_errors_.empty() ?
kNoOpErrorInfo : injected_errors_.front();
On 2016/03/01 at 18:01:49, svaldez wrote:
> I might be misunderstanding the code, but don't you want pop_front here to
cycle the deque?

Indeed. None of the tests depend on that, which makes me want to just change the
interface to SetError() instead.

Y'know what, I'll do that. This seems like unnecessary complexity.

https://codereview.chromium.org/1750943002/diff/60001/content/public/test/tes...
File content/public/test/test_file_error_injector.h (right):

https://codereview.chromium.org/1750943002/diff/60001/content/public/test/tes...
content/public/test/test_file_error_injector.h:120: ErrorQueue injected_errors_;
On 2016/03/01 at 18:01:49, svaldez wrote:
> Possibly assert on empty upon destruction?

Now a no-op.

[svaldez, 2016-03-01 19:13:07]

lgtm

[davidben, 2016-03-03 00:39:03]

lgtm

[asanka, 2016-03-03 03:53:06]

The CQ bit was checked by asanka@chromium.org

[commit-bot: I haz the power, 2016-03-03 03:53:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1750943002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1750943002/120001

[commit-bot: I haz the power, 2016-03-03 03:58:29]

Committed patchset #6 (id:120001)

[commit-bot: I haz the power, 2016-03-03 03:59:39]

Description was changed from

==========
[Downloads] Stop keying TestFileErrorInjector off of URLs.

DownloadFiles don't typically care about URLs. Hence the
TestFileErrorInjector shouldn't be using URLs as a key for looking up
injected errors.

Incidentally, none of the tests using TestFileErrorInjector depend on
there being a 1-1 mapping between download URLs and injected errors.
Hence, this CL removes the role that URLs were playing in
TestFileErrorInjector.

R=svaldez@chromium.org, davidben@chromium.org
BUG=7648
==========

to

==========
[Downloads] Stop keying TestFileErrorInjector off of URLs.

DownloadFiles don't typically care about URLs. Hence the
TestFileErrorInjector shouldn't be using URLs as a key for looking up
injected errors.

Incidentally, none of the tests using TestFileErrorInjector depend on
there being a 1-1 mapping between download URLs and injected errors.
Hence, this CL removes the role that URLs were playing in
TestFileErrorInjector.

R=svaldez@chromium.org, davidben@chromium.org
BUG=7648

Committed: https://crrev.com/43b66935f03e974fe5a052fd8382e7611b60ece6
Cr-Commit-Position: refs/heads/master@{#378944}
==========

[commit-bot: I haz the power, 2016-03-03 03:59:40]

Patchset 6 (id:??) landed as
https://crrev.com/43b66935f03e974fe5a052fd8382e7611b60ece6
Cr-Commit-Position: refs/heads/master@{#378944}