Move HISTORY and PASSWORDS from BrowsingDataRemover to the delegate

Move HISTORY and PASSWORDS from BrowsingDataRemover to the delegate

In https://codereview.chromium.org/2554413002/, BrowsingDataRemover was
split into content-related and embedder-specific datatypes
(BrowsingDataRemover[Impl] and [Chrome]BrowsingDataRemoverDelegate,
respectively).

The split was done according to where the relevant data storage backends
resided. This CL intends to make the split more precise by looking at it
more semantically.

1. While SSLHostStateDelegate lives in content/, it's state is deleted
   as a part of the HISTORY datatype, which is not a content/ concept.
   Therefore, this was moved to ChromeBrowsingDataRemoverDelegate.

2. The DCHECK verifying that all UI entrypoints to history deletion
   were disabled if !|may_delete_history| was also moved to
   ChromeBrowsingDataRemoverDelegate, as UI is also not a content/
   concept.

3. The deletion of auth cache is done in BrowsingDataRemoverDelegate
   for both the COOKIES and PASSWORDS datatypes. However, PASSWORDS
   are not a content/ concept. Therefore, BrowsingDataRemoverImpl now
   deletes the auth cache for COOKIES, and
   ChromeBrowsingDataRemoverDelegate for PASSWORDS. This is currently
   done by the duplication of the code, as some duplication is
   necessary anyway, and only about ~10 lines of code are duplicated
   unnecessarily; this seems to be too few to justify creating a new
   file in content/public to be shared between content/ and chrome/.

   TODO: This is the second duplication between the two classes. If
   this keeps happenning, consider putting the shared functionality
   to content/public anyway.

BUG=668114
Review-Url: https://codereview.chromium.org/2617403005
Cr-Commit-Position: refs/heads/master@{#443233}
Committed: https://chromium.googlesource.com/chromium/src/+/8733eb80974a809a19c16535bfd39c867845acab

[msramek, 2017-01-10 15:08:37]

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

[commit-bot: I haz the power, 2017-01-10 15:08:53]

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

[msramek, 2017-01-10 15:19:02]

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

[commit-bot: I haz the power, 2017-01-10 15:19:17]

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

[msramek, 2017-01-10 15:21:22]

Patchset #1 (id:1) has been deleted

[msramek, 2017-01-10 15:24:24]

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

[msramek, 2017-01-10 15:24:25]

Hi Mike!

Please have a look! This is a little cleanup needed to split the RemoveDataMask
enum between content/ and chrome/ datatypes.

Thanks,
Martin

[commit-bot: I haz the power, 2017-01-10 16:11:35]

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

[commit-bot: I haz the power, 2017-01-10 16:11:36]

Dry run: This issue passed the CQ dry run.

[msramek, 2017-01-10 16:26:44]

Hmm, regarding point #3 in the CL description, I now realized that there might
be even more duplication soon because of the filter builders.

I'll leave this CL as is for now, but I'll ask content/ owners for advice on how
to proceed.

[Mike West, 2017-01-12 12:56:18]

This CL LGTM, but reducing future duplication sounds like a reasonable goal. I
don't think it needs to block this CL, but having a cleaner mechanism for these
kinds of patches might be helpful if there are too many more coming.

[msramek, 2017-01-12 14:03:21]

On 2017/01/12 12:56:18, Mike West (sloooooow) wrote:
> This CL LGTM, but reducing future duplication sounds like a reasonable goal. I
> don't think it needs to block this CL, but having a cleaner mechanism for
these
> kinds of patches might be helpful if there are too many more coming.

Thanks Mike! I've been thinking about this a bit more and I think those filter
builders are the last remaining thing to be shared between chrome/ and content/,
and those have to be exposed in content/public in some way, as the delegate
class must know about them.

Let's chat about this again tomorrow! I will land this in the meantime to
unblock next steps of the migration.

[msramek, 2017-01-12 14:03:28]

The CQ bit was checked by msramek@chromium.org

[commit-bot: I haz the power, 2017-01-12 14:03:45]

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-01-12 15:15:06]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1484229808342090,
"parent_rev": "5cb986a6e5de91c01793068fdf8801900a0bd9bd", "commit_rev":
"8733eb80974a809a19c16535bfd39c867845acab"}

[commit-bot: I haz the power, 2017-01-12 15:15:43]

Description was changed from

==========
Move HISTORY and PASSWORDS from BrowsingDataRemover to the delegate

In https://codereview.chromium.org/2554413002/, BrowsingDataRemover was
split into content-related and embedder-specific datatypes
(BrowsingDataRemover[Impl] and [Chrome]BrowsingDataRemoverDelegate,
respectively).

The split was done according to where the relevant data storage backends
resided. This CL intends to make the split more precise by looking at it
more semantically.

1. While SSLHostStateDelegate lives in content/, it's state is deleted
   as a part of the HISTORY datatype, which is not a content/ concept.
   Therefore, this was moved to ChromeBrowsingDataRemoverDelegate.

2. The DCHECK verifying that all UI entrypoints to history deletion
   were disabled if !|may_delete_history| was also moved to
   ChromeBrowsingDataRemoverDelegate, as UI is also not a content/
   concept.

3. The deletion of auth cache is done in BrowsingDataRemoverDelegate
   for both the COOKIES and PASSWORDS datatypes. However, PASSWORDS
   are not a content/ concept. Therefore, BrowsingDataRemoverImpl now
   deletes the auth cache for COOKIES, and
   ChromeBrowsingDataRemoverDelegate for PASSWORDS. This is currently
   done by the duplication of the code, as some duplication is
   necessary anyway, and only about ~10 lines of code are duplicated
   unnecessarily; this seems to be too few to justify creating a new
   file in content/public to be shared between content/ and chrome/.

   TODO: This is the second duplication between the two classes. If
   this keeps happenning, consider putting the shared functionality
   to content/public anyway.

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

to

==========
Move HISTORY and PASSWORDS from BrowsingDataRemover to the delegate

In https://codereview.chromium.org/2554413002/, BrowsingDataRemover was
split into content-related and embedder-specific datatypes
(BrowsingDataRemover[Impl] and [Chrome]BrowsingDataRemoverDelegate,
respectively).

The split was done according to where the relevant data storage backends
resided. This CL intends to make the split more precise by looking at it
more semantically.

1. While SSLHostStateDelegate lives in content/, it's state is deleted
   as a part of the HISTORY datatype, which is not a content/ concept.
   Therefore, this was moved to ChromeBrowsingDataRemoverDelegate.

2. The DCHECK verifying that all UI entrypoints to history deletion
   were disabled if !|may_delete_history| was also moved to
   ChromeBrowsingDataRemoverDelegate, as UI is also not a content/
   concept.

3. The deletion of auth cache is done in BrowsingDataRemoverDelegate
   for both the COOKIES and PASSWORDS datatypes. However, PASSWORDS
   are not a content/ concept. Therefore, BrowsingDataRemoverImpl now
   deletes the auth cache for COOKIES, and
   ChromeBrowsingDataRemoverDelegate for PASSWORDS. This is currently
   done by the duplication of the code, as some duplication is
   necessary anyway, and only about ~10 lines of code are duplicated
   unnecessarily; this seems to be too few to justify creating a new
   file in content/public to be shared between content/ and chrome/.

   TODO: This is the second duplication between the two classes. If
   this keeps happenning, consider putting the shared functionality
   to content/public anyway.

BUG=668114

Review-Url: https://codereview.chromium.org/2617403005
Cr-Commit-Position: refs/heads/master@{#443233}
Committed:
https://chromium.googlesource.com/chromium/src/+/8733eb80974a809a19c16535bfd3...
==========

[commit-bot: I haz the power, 2017-01-12 15:15:44]

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