Change MockBrowsingDataRemover to MockBrowsingDataRemoverDelegate

Change MockBrowsingDataRemover to MockBrowsingDataRemoverDelegate

BrowsingDataRemover is about to move to content/, and does not expect
to be subclassed from the embedder. The responsibility of embedder
specialization is now on BrowsingDataRemoverDelegate.

Therefore, tests in the embedder should now mock the delegate instead
of the remover itself. This CL changes MockBrowsingDataRemover to
MockBrowsingDataRemoverDelegate and updates its (currently only one)
callsite.

Related changes:
- Added missing forward declarations to BrowsingDataRemoverDelegate
  (the code compiled, as its only implementation did have the correct
  includes)
- BrowsingDataRemoverDelegate takes BrowsingDataFilterBuilder through
  a const ref (not a unique_ptr), therefore we need to make a copy
  in MockBrowsingDataRemoverDelegate
- For consistency, the MockBrowsingDataRemoverDelegate's ExpectCall
  methods now also take BrowsingDataFilterBuilder as a const ref;
  this creates an unnecessary copy in the current test, but is
  a cleaner interface if the class is reused
- StoragePartitionHttpCacheDataRemover has been changed to tolerate
  non-exist URLRequestContextGetter-s (since BrowsingDataRemover
  now calls it to delete cache even in this test)

BUG=668114
Review-Url: https://codereview.chromium.org/2768883002
Cr-Commit-Position: refs/heads/master@{#459121}
Committed: https://chromium.googlesource.com/chromium/src/+/2232811f6fb779616100f1c3fed25ebe8f545703

[msramek, 2017-03-22 18:27:41]

Description was changed from

==========
Change MockBrowsingDataRemover to MockBrowsingDataRemoverDelegate

BrowsingDataRemover is about to move to content/, and does not expect
to be subclassed from the embedder. The responsibility of embedder
specialization is now on BrowsingDataRemoverDelegate.

Therefore, tests in the embedder should now mock the delegate instead
of the remover itself. This CL changes MockBrowsingDataRemover to
MockBrowsingDataRemoverDelegate and updates its (currently only one)
callsite.

Related changes:
- BrowsingDataRemoverDelegate takes BrowsingDataFilterBuilder through
  a const ref (not a unique_ptr), therefore we need to make a copy.
- StoragePartitionHttpCacheDataRemover has been changed to tolerate
  non-exist URLRequestContextGetter-s (since BrowsingDataRemover
  now calls it to delete cache even in this test)

BUG=668114
==========

to

==========
Change MockBrowsingDataRemover to MockBrowsingDataRemoverDelegate

BrowsingDataRemover is about to move to content/, and does not expect
to be subclassed from the embedder. The responsibility of embedder
specialization is now on BrowsingDataRemoverDelegate.

Therefore, tests in the embedder should now mock the delegate instead
of the remover itself. This CL changes MockBrowsingDataRemover to
MockBrowsingDataRemoverDelegate and updates its (currently only one)
callsite.

Related changes:
- Added missing forward declarations to BrowsingDataRemoverDelegate
  (the code compiled, as its only implementation did have the correct
  includes)
- BrowsingDataRemoverDelegate takes BrowsingDataFilterBuilder through
  a const ref (not a unique_ptr), therefore we need to make a copy
  in MockBrowsingDataRemoverDelegate
- StoragePartitionHttpCacheDataRemover has been changed to tolerate
  non-exist URLRequestContextGetter-s (since BrowsingDataRemover
  now calls it to delete cache even in this test)

BUG=668114
==========

[msramek, 2017-03-22 18:31:59]

Description was changed from

==========
Change MockBrowsingDataRemover to MockBrowsingDataRemoverDelegate

BrowsingDataRemover is about to move to content/, and does not expect
to be subclassed from the embedder. The responsibility of embedder
specialization is now on BrowsingDataRemoverDelegate.

Therefore, tests in the embedder should now mock the delegate instead
of the remover itself. This CL changes MockBrowsingDataRemover to
MockBrowsingDataRemoverDelegate and updates its (currently only one)
callsite.

Related changes:
- Added missing forward declarations to BrowsingDataRemoverDelegate
  (the code compiled, as its only implementation did have the correct
  includes)
- BrowsingDataRemoverDelegate takes BrowsingDataFilterBuilder through
  a const ref (not a unique_ptr), therefore we need to make a copy
  in MockBrowsingDataRemoverDelegate
- StoragePartitionHttpCacheDataRemover has been changed to tolerate
  non-exist URLRequestContextGetter-s (since BrowsingDataRemover
  now calls it to delete cache even in this test)

BUG=668114
==========

to

==========
Change MockBrowsingDataRemover to MockBrowsingDataRemoverDelegate

BrowsingDataRemover is about to move to content/, and does not expect
to be subclassed from the embedder. The responsibility of embedder
specialization is now on BrowsingDataRemoverDelegate.

Therefore, tests in the embedder should now mock the delegate instead
of the remover itself. This CL changes MockBrowsingDataRemover to
MockBrowsingDataRemoverDelegate and updates its (currently only one)
callsite.

Related changes:
- Added missing forward declarations to BrowsingDataRemoverDelegate
  (the code compiled, as its only implementation did have the correct
  includes)
- BrowsingDataRemoverDelegate takes BrowsingDataFilterBuilder through
  a const ref (not a unique_ptr), therefore we need to make a copy
  in MockBrowsingDataRemoverDelegate
- For consistency, the MockBrowsingDataRemoverDelegate's ExpectCall
  methods now also take BrowsingDataFilterBuilder as a const ref;
  this creates an unnecessary copy in the current test, but is
  a cleaner interface if the class is reused
- StoragePartitionHttpCacheDataRemover has been changed to tolerate
  non-exist URLRequestContextGetter-s (since BrowsingDataRemover
  now calls it to delete cache even in this test)

BUG=668114
==========

[msramek, 2017-03-22 18:33:06]

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

[msramek, 2017-03-22 18:33:14]

Patchset #1 (id:1) has been deleted

[msramek, 2017-03-22 18:34:18]

msramek@chromium.org changed reviewers:
+ dullweber@chromium.org

[msramek, 2017-03-22 18:34:18]

Hi Christian,

Please have a look! Does this setup work for your future use of
MockBrowsingDataRemover(Delegate)?

Thanks,
Martin

[commit-bot: I haz the power, 2017-03-22 18:34:36]

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, 2017-03-22 20:49:55]

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

[commit-bot: I haz the power, 2017-03-22 20:49:56]

Dry run: This issue passed the CQ dry run.

[dullweber, 2017-03-23 10:09:14]

On 2017/03/22 18:34:18, msramek wrote:
> Hi Christian,
> 
> Please have a look! Does this setup work for your future use of
> MockBrowsingDataRemover(Delegate)?
> 
> Thanks,
> Martin

lgtm, I will try to write a test with it soon

[msramek, 2017-03-23 10:29:22]

msramek@chromium.org changed reviewers:
+ engedy@chromium.org

[msramek, 2017-03-23 10:29:23]

Thanks!

+Balázs, could you please have a look? Christian already reviewed this
feature-wise, but he's not a committer yet.

[engedy, 2017-03-23 15:23:07]

https://codereview.chromium.org/2768883002/diff/20001/components/browsing_dat...
File
components/browsing_data/content/storage_partition_http_cache_data_remover.cc
(right):

https://codereview.chromium.org/2768883002/diff/20001/components/browsing_dat...
components/browsing_data/content/storage_partition_http_cache_data_remover.cc:98:
// --> CacheState::PROCESS_MAIN --> CacheState::CREATE_MEDIA -->
nit: Could you please propagate new names of states to this comment?

[engedy, 2017-03-23 15:23:19]

LGTM % above nit.

[msramek, 2017-03-23 15:33:02]

Thanks!

https://codereview.chromium.org/2768883002/diff/20001/components/browsing_dat...
File
components/browsing_data/content/storage_partition_http_cache_data_remover.cc
(right):

https://codereview.chromium.org/2768883002/diff/20001/components/browsing_dat...
components/browsing_data/content/storage_partition_http_cache_data_remover.cc:98:
// --> CacheState::PROCESS_MAIN --> CacheState::CREATE_MEDIA -->
On 2017/03/23 15:23:07, engedy wrote:
> nit: Could you please propagate new names of states to this comment?

Done.

[msramek, 2017-03-23 15:33:09]

The CQ bit was checked by msramek@chromium.org

[msramek, 2017-03-23 15:33:09]

The patchset sent to the CQ was uploaded after l-g-t-m from
dullweber@chromium.org, engedy@chromium.org
Link to the patchset: https://codereview.chromium.org/2768883002/#ps40001
(title: "Fixed the comment.")

[commit-bot: I haz the power, 2017-03-23 15:33:34]

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, 2017-03-23 16:50:34]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1490283189233090,
"parent_rev": "712a32b9cdf3b50e63eb25f62e901eff8e84ccf9", "commit_rev":
"2232811f6fb779616100f1c3fed25ebe8f545703"}

[commit-bot: I haz the power, 2017-03-23 16:51:19]

Description was changed from

==========
Change MockBrowsingDataRemover to MockBrowsingDataRemoverDelegate

BrowsingDataRemover is about to move to content/, and does not expect
to be subclassed from the embedder. The responsibility of embedder
specialization is now on BrowsingDataRemoverDelegate.

Therefore, tests in the embedder should now mock the delegate instead
of the remover itself. This CL changes MockBrowsingDataRemover to
MockBrowsingDataRemoverDelegate and updates its (currently only one)
callsite.

Related changes:
- Added missing forward declarations to BrowsingDataRemoverDelegate
  (the code compiled, as its only implementation did have the correct
  includes)
- BrowsingDataRemoverDelegate takes BrowsingDataFilterBuilder through
  a const ref (not a unique_ptr), therefore we need to make a copy
  in MockBrowsingDataRemoverDelegate
- For consistency, the MockBrowsingDataRemoverDelegate's ExpectCall
  methods now also take BrowsingDataFilterBuilder as a const ref;
  this creates an unnecessary copy in the current test, but is
  a cleaner interface if the class is reused
- StoragePartitionHttpCacheDataRemover has been changed to tolerate
  non-exist URLRequestContextGetter-s (since BrowsingDataRemover
  now calls it to delete cache even in this test)

BUG=668114
==========

to

==========
Change MockBrowsingDataRemover to MockBrowsingDataRemoverDelegate

BrowsingDataRemover is about to move to content/, and does not expect
to be subclassed from the embedder. The responsibility of embedder
specialization is now on BrowsingDataRemoverDelegate.

Therefore, tests in the embedder should now mock the delegate instead
of the remover itself. This CL changes MockBrowsingDataRemover to
MockBrowsingDataRemoverDelegate and updates its (currently only one)
callsite.

Related changes:
- Added missing forward declarations to BrowsingDataRemoverDelegate
  (the code compiled, as its only implementation did have the correct
  includes)
- BrowsingDataRemoverDelegate takes BrowsingDataFilterBuilder through
  a const ref (not a unique_ptr), therefore we need to make a copy
  in MockBrowsingDataRemoverDelegate
- For consistency, the MockBrowsingDataRemoverDelegate's ExpectCall
  methods now also take BrowsingDataFilterBuilder as a const ref;
  this creates an unnecessary copy in the current test, but is
  a cleaner interface if the class is reused
- StoragePartitionHttpCacheDataRemover has been changed to tolerate
  non-exist URLRequestContextGetter-s (since BrowsingDataRemover
  now calls it to delete cache even in this test)

BUG=668114

Review-Url: https://codereview.chromium.org/2768883002
Cr-Commit-Position: refs/heads/master@{#459121}
Committed:
https://chromium.googlesource.com/chromium/src/+/2232811f6fb779616100f1c3fed2...
==========

[commit-bot: I haz the power, 2017-03-23 16:51:20]

Committed patchset #2 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/2232811f6fb779616100f1c3fed2...