Add tests for the cache deletion in BrowsingDataRemover

Add tests for the cache deletion in BrowsingDataRemover

The cache deletion was previously not tested in BrowsingDataRemover,
presumably because the helper class StoragePartitionHttpCacheDataRemover
instantiates a number of services that are difficult / not possible to
mock in the unittest.

Therefore, we will test this in the browsertest. The test added in this
CL only verifies two things:
1. That the cache deletion completes successfuly without crashing.
2. That the size of the cache decreases after deletion.

Whether the correct cache entries would be deleted is already tested
in the browsertest of ConditionalCacheDeletionHelper, which is the helper
class used by BrowsingDataRemover for partial cache deletion.

The added test uncovers two bugs in the conditional cache deletion (which
is currently not live) and fixes them.
1. Missing return value assignment in StoragePartitionHttpCacheDataRemover.
2. It is not possible to partially clear cookies for origin, and therefore
   the creation of CookieMatcherFunction from an OriginFilterBuilder DCHECKs.
   However, it should be possible to delete cache for an origin. When doing
   so, we unnecessarily try to also instantiate CookieMatcherFunction, which
   hits the mentioned DCHECK.

Finally, we replace URLRequestMockHTTPJob with an EmbeddedTestServer in the
browsertest. This is because to populate the cache, we need to make an actual
resource fetch.

BUG=113967
Committed: https://crrev.com/eeabde240e3f049dc8b0a2b353cf74e6f528d131
Cr-Commit-Position: refs/heads/master@{#396929}

[msramek, 2016-05-25 14:26:13]

Patchset #1 (id:1) has been deleted

[msramek, 2016-05-25 14:29:50]

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

[msramek, 2016-05-25 14:29:50]

Hi Dan,

please have a look!

Thanks,
Martin

[dmurph, 2016-05-25 19:04:59]

Awesome, lgtm!

[dmurph, 2016-05-27 02:08:38]

On 2016/05/25 at 19:04:59, dmurph wrote:
> Awesome, lgtm!

Looks like you got some leaks from the test.

[msramek, 2016-05-31 18:57:38]

Patchset #2 (id:40001) has been deleted

[msramek, 2016-05-31 19:51:14]

Sorry for the delay with landing this. I didn't have time to investigate the
memory leak, until it dawned on me today that if the leak is in OpenEntry(),
then maybe I should close then entry? :)

PTAL!

[dmurph, 2016-05-31 20:11:51]

nice catch, lgtm

[msramek, 2016-05-31 21:02:32]

The CQ bit was checked by msramek@chromium.org

[commit-bot: I haz the power, 2016-05-31 21:03:45]

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

[commit-bot: I haz the power, 2016-05-31 21:08:46]

Committed patchset #2 (id:60001)

[commit-bot: I haz the power, 2016-05-31 21:09:53]

Description was changed from

==========
Add tests for the cache deletion in BrowsingDataRemover

The cache deletion was previously not tested in BrowsingDataRemover,
presumably because the helper class StoragePartitionHttpCacheDataRemover
instantiates a number of services that are difficult / not possible to
mock in the unittest.

Therefore, we will test this in the browsertest. The test added in this
CL only verifies two things:
1. That the cache deletion completes successfuly without crashing.
2. That the size of the cache decreases after deletion.

Whether the correct cache entries would be deleted is already tested
in the browsertest of ConditionalCacheDeletionHelper, which is the helper
class used by BrowsingDataRemover for partial cache deletion.

The added test uncovers two bugs in the conditional cache deletion (which
is currently not live) and fixes them.
1. Missing return value assignment in StoragePartitionHttpCacheDataRemover.
2. It is not possible to partially clear cookies for origin, and therefore
   the creation of CookieMatcherFunction from an OriginFilterBuilder DCHECKs.
   However, it should be possible to delete cache for an origin. When doing
   so, we unnecessarily try to also instantiate CookieMatcherFunction, which
   hits the mentioned DCHECK.

Finally, we replace URLRequestMockHTTPJob with an EmbeddedTestServer in the
browsertest. This is because to populate the cache, we need to make an actual
resource fetch.

BUG=113967
==========

to

==========
Add tests for the cache deletion in BrowsingDataRemover

The cache deletion was previously not tested in BrowsingDataRemover,
presumably because the helper class StoragePartitionHttpCacheDataRemover
instantiates a number of services that are difficult / not possible to
mock in the unittest.

Therefore, we will test this in the browsertest. The test added in this
CL only verifies two things:
1. That the cache deletion completes successfuly without crashing.
2. That the size of the cache decreases after deletion.

Whether the correct cache entries would be deleted is already tested
in the browsertest of ConditionalCacheDeletionHelper, which is the helper
class used by BrowsingDataRemover for partial cache deletion.

The added test uncovers two bugs in the conditional cache deletion (which
is currently not live) and fixes them.
1. Missing return value assignment in StoragePartitionHttpCacheDataRemover.
2. It is not possible to partially clear cookies for origin, and therefore
   the creation of CookieMatcherFunction from an OriginFilterBuilder DCHECKs.
   However, it should be possible to delete cache for an origin. When doing
   so, we unnecessarily try to also instantiate CookieMatcherFunction, which
   hits the mentioned DCHECK.

Finally, we replace URLRequestMockHTTPJob with an EmbeddedTestServer in the
browsertest. This is because to populate the cache, we need to make an actual
resource fetch.

BUG=113967

Committed: https://crrev.com/eeabde240e3f049dc8b0a2b353cf74e6f528d131
Cr-Commit-Position: refs/heads/master@{#396929}
==========

[commit-bot: I haz the power, 2016-05-31 21:09:54]

Patchset 2 (id:??) landed as
https://crrev.com/eeabde240e3f049dc8b0a2b353cf74e6f528d131
Cr-Commit-Position: refs/heads/master@{#396929}