Split browsing data masks between content and embedder

Split browsing data masks between content and embedder

As a part of the refactoring to move content-related browsing data
infrastructure to content, we split BrowsingDataRemover::RemoveDataMask
and BrowsingDataHelper::OriginTypeMask between content and embedder.

The CL consists of the following steps:

1. BrowsingDataRemover::RemoveDataMask now only contains content datatypes.
Chrome-specific datatypes are moved to ChromeBrowsingDataRemoverDelegate.
The enum in ChromeBrowsingDataRemoverDelegate also copies all values
from BrowsingDataRemover so that this enum inheritance better mimics
class inheritance. Note that this is a second attempt at this change, following
the discussion in https://codereview.chromium.org/2697123004/ where we tried
converting enums into pointers first.

The concept of FILTERABLE_DATATYPES has been moved to
ChromeBrowsingDataRemoverDelegate, since all content data types are filterable.

2. The same exercise is done for BrowsingDataHelper::OriginTypeMask. Its
first two items, UNPROTECTED_WEB and PROTECTED_WEB correspond to the boolean
returned by content::SpecialStoragePolicy::IsProtected(). Therefore, they
are moved to content as a new enum in BrowsingDataRemover. The third item,
EXTENSION, is a Chrome-specific concept and stays in chrome/ code, as a new
enum in ChromeBrowsingDataRemoverDelegate.

BrowsingDataRemoverImpl is now able to tell about each origin whether it is
UNPROTECTED_WEB or PROTECTED_WEB. It delegates the decision what is an
EXTENSION to ChromeBrowsingDataRemoverDelegate. A new method has been added
to the BrowsingDataRemoverDelegate for this purpose.

BrowsingDataHelper now serves no other purpose than recognizing webby and
extension URLs for various classes in browsing_data/ and can be eventually
deprecated (though doing so in this CL would significantly blow it in size).

The behavior of UNPROTECTED_WEB and PROTECTED_WEB is already sufficiently
tested in StoragePartition-related tests of BrowsingDataRemoverTest. We just
had to replace MockExtensionSpecialStoragePolicy with the regular
MockSpecialStoragePolicy. Testcases from BrowsingDataHelperTest have been moved
to ChromeBrowsingDataRemoverDelegateTest to cover the remaining EXTENSION type.

3. Since BrowsingDataRemover and ChromeBrowsingDataRemoverDelegate now host
both the data type and origin type enums, we changed the naming to call out
the difference; all data types are now prefixed with DATA_TYPE, all origin
types with ORIGIN_TYPE.

4. All other files contain equivalent replacements.

TBR=dbeam@chromium.org,jochen@chromium.org
BUG=668114
Review-Url: https://codereview.chromium.org/2733393003
Cr-Commit-Position: refs/heads/master@{#456757}
Committed: https://chromium.googlesource.com/chromium/src/+/1c2b3ca05716cc40a8a89f51527b0eef52dddde6

[msramek, 2017-03-08 11:59:39]

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

[commit-bot: I haz the power, 2017-03-08 11:59:52]

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-08 12:02:19]

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

[commit-bot: I haz the power, 2017-03-08 12:02:20]

Dry run: Try jobs failed on following builders:
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[msramek, 2017-03-08 12:51:10]

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

[commit-bot: I haz the power, 2017-03-08 12:51:24]

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-08 12:58:16]

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

[commit-bot: I haz the power, 2017-03-08 12:58:17]

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

[msramek, 2017-03-08 13:51:26]

Description was changed from

==========
Split browsing data masks between content and embedder

As a part of the refactoring to move content-related browsing data
infrastructure to content, we split BrowsingDataRemover::RemoveDataMask
and BrowsingDataHelper::OriginTypeMask between content and embedder.

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

to

==========
Split browsing data masks between content and embedder

As a part of the refactoring to move content-related browsing data
infrastructure to content, we split BrowsingDataRemover::RemoveDataMask
and BrowsingDataHelper::OriginTypeMask between content and embedder.

The CL consists of the following steps:

1. BrowsingDataRemover::RemoveDataMask now only contains content datatypes.
Chrome-specific datatypes are moved to ChromeBrowsingDataRemoverDelegate.
The enum in ChromeBrowsingDataRemoverDelegate also copies all values
from BrowsingDataRemover so that this enum inheritance better mimics
class inheritance. Note that this is a second attempt at this change, following
the discussion in https://codereview.chromium.org/2697123004/ where we tried
converting enums into pointers first.

The concept of FILTERABLE_DATATYPES has been moved to
ChromeBrowsingDataRemoverDelegate, since all content data types are filterable.

2. The same exercise is done for BrowsingDataHelper::OriginTypeMask. Its
first two items, UNPROTECTED_WEB and PROTECTED_WEB correspond to the boolean
returned by content::SpecialStoragePolicy::IsProtected(). Therefore, they
are moved to content as a new enum in BrowsingDataRemover. The third item,
EXTENSION, is a Chrome-specific concept and stays in chrome/ code, as a new
enum in ChromeBrowsingDataRemoverDelegate.

BrowsingDataRemoverImpl is now able to tell about each origin whether it is
UNPROTECTED_WEB or PROTECTED_WEB. It delegates the decision what is an
EXTENSION to ChromeBrowsingDataRemoverDelegate. A new method has been added
to the BrowsingDataRemoverDelegate for this purpose.

BrowsingDataHelper now serves no other purpose than recognizing webby and
extension URLs for various classes in browsing_data/ and can be eventually
deprecated (though doing so in this CL would significantly blow it in size).

The behavior of UNPROTECTED_WEB and PROTECTED_WEB is already sufficiently
tested in StoragePartition-related tests of BrowsingDataRemoverTest. We just
had to replace MockExtensionSpecialStoragePolicy with the regular
MockSpecialStoragePolicy. Testcases from BrowsingDataHelperTest have been moved
to ChromeBrowsingDataRemoverDelegateTest to cover the remaining EXTENSION type.

3. Since BrowsingDataRemover and ChromeBrowsingDataRemoverDelegate now host
both the data type and origin type enums, we changed the naming to call out
the difference; all data types are now prefixed with DATA_TYPE, all origin
types with ORIGIN_TYPE.

4. All other files contain equivalent replacements.

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

[msramek, 2017-03-08 13:51:37]

Patchset #2 (id:20001) has been deleted

[msramek, 2017-03-08 13:51:55]

Patchset #1 (id:1) has been deleted

[msramek, 2017-03-08 13:52:00]

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

[commit-bot: I haz the power, 2017-03-08 13:52:06]

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

[msramek, 2017-03-08 14:02:22]

msramek@chromium.org changed reviewers:
+ bauerb@chromium.org, jam@chromium.org, mkwst@chromium.org

[msramek, 2017-03-08 14:02:22]

Hi Bernhard, Mike, and John!

This CL is another attempt at splitting RemoveDataMask between content and
embedder, following the discussions in
https://codereview.chromium.org/2697123004/ which I now closed. It's also a bit
larger, as it does the same change for OriginTypeMask.

This time I'm looping in John right from the beginning to be sure :) Please have
a look! (...when you're done perfing :) ).

Thanks!
Martin

[commit-bot: I haz the power, 2017-03-08 14:04:23]

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

[commit-bot: I haz the power, 2017-03-08 14:04:24]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[msramek, 2017-03-08 14:55:15]

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

[commit-bot: I haz the power, 2017-03-08 14:55:25]

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-08 15:08:26]

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

[commit-bot: I haz the power, 2017-03-08 15:08:28]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[msramek, 2017-03-08 15:13:51]

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

[commit-bot: I haz the power, 2017-03-08 15:14:02]

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-08 15:25:55]

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

[commit-bot: I haz the power, 2017-03-08 15:25:56]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[msramek, 2017-03-08 15:56:21]

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

[commit-bot: I haz the power, 2017-03-08 15:56:49]

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-08 16:39:27]

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

[commit-bot: I haz the power, 2017-03-08 16:39:28]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Mike West, 2017-03-09 08:14:03]

I skimmed this quickly, and it looks reasonable. That said, I apologize that I'm
not going to be able to give you more concrete feedback until tomorrowish. I'm
buried. :/

[msramek, 2017-03-09 09:52:59]

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

[commit-bot: I haz the power, 2017-03-09 09:53: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, 2017-03-09 10:45:45]

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

[commit-bot: I haz the power, 2017-03-09 10:45:46]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[msramek, 2017-03-09 14:53:30]

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

[commit-bot: I haz the power, 2017-03-09 14:53:44]

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-09 15:34:48]

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

[commit-bot: I haz the power, 2017-03-09 15:34:49]

Dry run: This issue passed the CQ dry run.

[Bernhard Bauer, 2017-03-09 17:35:49]

LGTM!

https://codereview.chromium.org/2733393003/diff/140001/chrome/browser/browsin...
File chrome/browser/browsing_data/browsing_data_remover_impl.cc (right):

https://codereview.chromium.org/2733393003/diff/140001/chrome/browser/browsin...
chrome/browser/browsing_data/browsing_data_remover_impl.cc:80: // If
BrowsingDataRemoverImpl is null, it is not possible to determine which
Nit: I would maybe move this first and return in that case.

https://codereview.chromium.org/2733393003/diff/140001/chrome/browser/browsin...
File chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h
(right):

https://codereview.chromium.org/2733393003/diff/140001/chrome/browser/browsin...
chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h:169:
static_assert(0 == (IMPORTANT_SITES_DATA_TYPES & ~FILTERABLE_DATA_TYPES),
Nit: Can you flip the condition around so it reads more straightforward? It's
not like we have to avoid an accidental assignment here anyway.

[Mike West, 2017-03-10 14:06:48]

Thanks for the detailed CL description. This LGTM too.

[jam, 2017-03-10 16:13:43]

https://codereview.chromium.org/2733393003/diff/140001/chrome/browser/browsin...
File chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h
(right):

https://codereview.chromium.org/2733393003/diff/140001/chrome/browser/browsin...
chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h:56:
DATA_TYPE_APP_CACHE = BrowsingDataRemover::DATA_TYPE_APP_CACHE,
why do you duplicate the list in content here? Now every time content changes,
chrome has to as well.

Is this just to avoid having different namespaces? If so I don't think this is
something that matters; this is what we do today for all the other enums that
chrome/ extends.

[msramek, 2017-03-13 17:43:35]

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

[commit-bot: I haz the power, 2017-03-13 17:44:09]

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

[msramek, 2017-03-13 17:57:20]

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

[msramek, 2017-03-13 17:57:33]

Thanks, Bernhard and Mike!

https://codereview.chromium.org/2733393003/diff/140001/chrome/browser/browsin...
File chrome/browser/browsing_data/browsing_data_remover_impl.cc (right):

https://codereview.chromium.org/2733393003/diff/140001/chrome/browser/browsin...
chrome/browser/browsing_data/browsing_data_remover_impl.cc:80: // If
BrowsingDataRemoverImpl is null, it is not possible to determine which
On 2017/03/09 17:35:48, Bernhard Bauer wrote:
> Nit: I would maybe move this first and return in that case.

Done.

https://codereview.chromium.org/2733393003/diff/140001/chrome/browser/browsin...
File chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h
(right):

https://codereview.chromium.org/2733393003/diff/140001/chrome/browser/browsin...
chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h:169:
static_assert(0 == (IMPORTANT_SITES_DATA_TYPES & ~FILTERABLE_DATA_TYPES),
On 2017/03/09 17:35:48, Bernhard Bauer wrote:
> Nit: Can you flip the condition around so it reads more straightforward? It's
> not like we have to avoid an accidental assignment here anyway.

Done.

[commit-bot: I haz the power, 2017-03-13 17:58:01]

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

[msramek, 2017-03-13 17:58:57]

John - as explained in the CL description, my intention was to mimic public
class inheritance, where all public members of the base class are also moved to
the subclass namespace; and the reason was mostly convenience (to unburden
callers from having to know which datatypes belong to content and which ones to
chrome) and code cleanliness (bitmasks etc. read easier if I'm not mixing two
enums).

You're right that this duplication opens a possibility of extending the content/
enum without doing the same in chrome/, but this would not actually break
anything (although admittedly it would look very inconsistent).

Nevertheless... I don't have such a strong opinion and you do seem to have one,
so I removed the inheritance in patchset #8. Does this look better to you?

[commit-bot: I haz the power, 2017-03-13 19:12:20]

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

[commit-bot: I haz the power, 2017-03-13 19:12:22]

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

[msramek, 2017-03-14 09:16:18]

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

[commit-bot: I haz the power, 2017-03-14 09:16:33]

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-14 09:44:31]

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

[commit-bot: I haz the power, 2017-03-14 09:44:32]

Dry run: This issue passed the CQ dry run.

[jam, 2017-03-14 16:18:00]

lgtm

[msramek, 2017-03-14 16:33:16]

Description was changed from

==========
Split browsing data masks between content and embedder

As a part of the refactoring to move content-related browsing data
infrastructure to content, we split BrowsingDataRemover::RemoveDataMask
and BrowsingDataHelper::OriginTypeMask between content and embedder.

The CL consists of the following steps:

1. BrowsingDataRemover::RemoveDataMask now only contains content datatypes.
Chrome-specific datatypes are moved to ChromeBrowsingDataRemoverDelegate.
The enum in ChromeBrowsingDataRemoverDelegate also copies all values
from BrowsingDataRemover so that this enum inheritance better mimics
class inheritance. Note that this is a second attempt at this change, following
the discussion in https://codereview.chromium.org/2697123004/ where we tried
converting enums into pointers first.

The concept of FILTERABLE_DATATYPES has been moved to
ChromeBrowsingDataRemoverDelegate, since all content data types are filterable.

2. The same exercise is done for BrowsingDataHelper::OriginTypeMask. Its
first two items, UNPROTECTED_WEB and PROTECTED_WEB correspond to the boolean
returned by content::SpecialStoragePolicy::IsProtected(). Therefore, they
are moved to content as a new enum in BrowsingDataRemover. The third item,
EXTENSION, is a Chrome-specific concept and stays in chrome/ code, as a new
enum in ChromeBrowsingDataRemoverDelegate.

BrowsingDataRemoverImpl is now able to tell about each origin whether it is
UNPROTECTED_WEB or PROTECTED_WEB. It delegates the decision what is an
EXTENSION to ChromeBrowsingDataRemoverDelegate. A new method has been added
to the BrowsingDataRemoverDelegate for this purpose.

BrowsingDataHelper now serves no other purpose than recognizing webby and
extension URLs for various classes in browsing_data/ and can be eventually
deprecated (though doing so in this CL would significantly blow it in size).

The behavior of UNPROTECTED_WEB and PROTECTED_WEB is already sufficiently
tested in StoragePartition-related tests of BrowsingDataRemoverTest. We just
had to replace MockExtensionSpecialStoragePolicy with the regular
MockSpecialStoragePolicy. Testcases from BrowsingDataHelperTest have been moved
to ChromeBrowsingDataRemoverDelegateTest to cover the remaining EXTENSION type.

3. Since BrowsingDataRemover and ChromeBrowsingDataRemoverDelegate now host
both the data type and origin type enums, we changed the naming to call out
the difference; all data types are now prefixed with DATA_TYPE, all origin
types with ORIGIN_TYPE.

4. All other files contain equivalent replacements.

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

to

==========
Split browsing data masks between content and embedder

As a part of the refactoring to move content-related browsing data
infrastructure to content, we split BrowsingDataRemover::RemoveDataMask
and BrowsingDataHelper::OriginTypeMask between content and embedder.

The CL consists of the following steps:

1. BrowsingDataRemover::RemoveDataMask now only contains content datatypes.
Chrome-specific datatypes are moved to ChromeBrowsingDataRemoverDelegate.
The enum in ChromeBrowsingDataRemoverDelegate also copies all values
from BrowsingDataRemover so that this enum inheritance better mimics
class inheritance. Note that this is a second attempt at this change, following
the discussion in https://codereview.chromium.org/2697123004/ where we tried
converting enums into pointers first.

The concept of FILTERABLE_DATATYPES has been moved to
ChromeBrowsingDataRemoverDelegate, since all content data types are filterable.

2. The same exercise is done for BrowsingDataHelper::OriginTypeMask. Its
first two items, UNPROTECTED_WEB and PROTECTED_WEB correspond to the boolean
returned by content::SpecialStoragePolicy::IsProtected(). Therefore, they
are moved to content as a new enum in BrowsingDataRemover. The third item,
EXTENSION, is a Chrome-specific concept and stays in chrome/ code, as a new
enum in ChromeBrowsingDataRemoverDelegate.

BrowsingDataRemoverImpl is now able to tell about each origin whether it is
UNPROTECTED_WEB or PROTECTED_WEB. It delegates the decision what is an
EXTENSION to ChromeBrowsingDataRemoverDelegate. A new method has been added
to the BrowsingDataRemoverDelegate for this purpose.

BrowsingDataHelper now serves no other purpose than recognizing webby and
extension URLs for various classes in browsing_data/ and can be eventually
deprecated (though doing so in this CL would significantly blow it in size).

The behavior of UNPROTECTED_WEB and PROTECTED_WEB is already sufficiently
tested in StoragePartition-related tests of BrowsingDataRemoverTest. We just
had to replace MockExtensionSpecialStoragePolicy with the regular
MockSpecialStoragePolicy. Testcases from BrowsingDataHelperTest have been moved
to ChromeBrowsingDataRemoverDelegateTest to cover the remaining EXTENSION type.

3. Since BrowsingDataRemover and ChromeBrowsingDataRemoverDelegate now host
both the data type and origin type enums, we changed the naming to call out
the difference; all data types are now prefixed with DATA_TYPE, all origin
types with ORIGIN_TYPE.

4. All other files contain equivalent replacements.

TBR=dbeam@chromium.org,jochen@chromium.org
BUG=668114
==========

[msramek, 2017-03-14 16:34:23]

msramek@chromium.org changed reviewers:
+ dbeam@chromium.org, jochen@chromium.org

[msramek, 2017-03-14 16:34:24]

Thanks, John! \o/

Unlike the previous attempt in https://codereview.chromium.org/2697123004/, this
time all the remaining files only contain equivalent replacements, i.e.:
- Updated the enum values that were renamed
- Updated BrowsingDataRemover:: -> ChromeBrowsingDataRemoverDelegate:: for
values that moved namespace
- Added missing includes for chrome_browsing_data_remover_delegate.h
- Ran git cl format

I'm therefore leaving them to TBR. Adding:
+Dan to TBR for webui/options/
+Jochen to TBR for everything else

...and proceeding to land this.

[msramek, 2017-03-14 16:34:41]

The CQ bit was checked by msramek@chromium.org

[msramek, 2017-03-14 16:34:42]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org,
mkwst@chromium.org
Link to the patchset: https://codereview.chromium.org/2733393003/#ps200001
(title: "git cl format")

[commit-bot: I haz the power, 2017-03-14 16:35:05]

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-14 16:40:56]

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

[commit-bot: I haz the power, 2017-03-14 16:40:59]

Failed to apply patch for
chrome/browser/push_messaging/push_messaging_browsertest.cc:
While running git apply --index -p1;
  error: patch failed:
chrome/browser/push_messaging/push_messaging_browsertest.cc:22
  error: chrome/browser/push_messaging/push_messaging_browsertest.cc: patch does
not apply

Patch:       chrome/browser/push_messaging/push_messaging_browsertest.cc
Index: chrome/browser/push_messaging/push_messaging_browsertest.cc
diff --git a/chrome/browser/push_messaging/push_messaging_browsertest.cc
b/chrome/browser/push_messaging/push_messaging_browsertest.cc
index
024accdb7bb1a361dfd5708353fbac0ca04492bf..0e676870615b27b4fad7e3f788efc430c2a1c105
100644
--- a/chrome/browser/push_messaging/push_messaging_browsertest.cc
+++ b/chrome/browser/push_messaging/push_messaging_browsertest.cc
@@ -22,6 +22,7 @@
 #include "chrome/browser/browsing_data/browsing_data_remover.h"
 #include "chrome/browser/browsing_data/browsing_data_remover_factory.h"
 #include "chrome/browser/browsing_data/browsing_data_remover_test_util.h"
+#include "chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/engagement/site_engagement_service.h"
 #include "chrome/browser/gcm/fake_gcm_profile_service.h"
@@ -2054,9 +2055,10 @@ IN_PROC_BROWSER_TEST_F(
   BrowsingDataRemover* remover =
      
BrowsingDataRemoverFactory::GetForBrowserContext(GetBrowser()->profile());
   BrowsingDataRemoverCompletionObserver observer(remover);
-  remover->RemoveAndReply(base::Time(), base::Time::Max(),
-                          BrowsingDataRemover::REMOVE_SITE_DATA,
-                          BrowsingDataHelper::UNPROTECTED_WEB, &observer);
+  remover->RemoveAndReply(
+      base::Time(), base::Time::Max(),
+      ChromeBrowsingDataRemoverDelegate::DATA_TYPE_SITE_DATA,
+      BrowsingDataRemover::ORIGIN_TYPE_UNPROTECTED_WEB, &observer);
   observer.BlockUntilCompletion();
 
   base::RunLoop run_loop;

[msramek, 2017-03-14 17:02:15]

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

[commit-bot: I haz the power, 2017-03-14 17:03: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-03-14 17:03:27]

The CQ bit was unchecked by msramek@chromium.org

[msramek, 2017-03-14 17:04:30]

The CQ bit was checked by msramek@chromium.org

[msramek, 2017-03-14 17:04:31]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org,
jam@chromium.org, mkwst@chromium.org
Link to the patchset: https://codereview.chromium.org/2733393003/#ps220001
(title: "Rebase (merged automatically)")

[commit-bot: I haz the power, 2017-03-14 17:04:53]

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-14 17:58:00]

CQ is committing da patch.

Bot data: {"patchset_id": 220001, "attempt_start_ts": 1489511070700130,
"parent_rev": "87c322b0dc2f4a7595fd288ab0c529d4aa1aafd9", "commit_rev":
"1c2b3ca05716cc40a8a89f51527b0eef52dddde6"}

[commit-bot: I haz the power, 2017-03-14 17:58:42]

Description was changed from

==========
Split browsing data masks between content and embedder

As a part of the refactoring to move content-related browsing data
infrastructure to content, we split BrowsingDataRemover::RemoveDataMask
and BrowsingDataHelper::OriginTypeMask between content and embedder.

The CL consists of the following steps:

1. BrowsingDataRemover::RemoveDataMask now only contains content datatypes.
Chrome-specific datatypes are moved to ChromeBrowsingDataRemoverDelegate.
The enum in ChromeBrowsingDataRemoverDelegate also copies all values
from BrowsingDataRemover so that this enum inheritance better mimics
class inheritance. Note that this is a second attempt at this change, following
the discussion in https://codereview.chromium.org/2697123004/ where we tried
converting enums into pointers first.

The concept of FILTERABLE_DATATYPES has been moved to
ChromeBrowsingDataRemoverDelegate, since all content data types are filterable.

2. The same exercise is done for BrowsingDataHelper::OriginTypeMask. Its
first two items, UNPROTECTED_WEB and PROTECTED_WEB correspond to the boolean
returned by content::SpecialStoragePolicy::IsProtected(). Therefore, they
are moved to content as a new enum in BrowsingDataRemover. The third item,
EXTENSION, is a Chrome-specific concept and stays in chrome/ code, as a new
enum in ChromeBrowsingDataRemoverDelegate.

BrowsingDataRemoverImpl is now able to tell about each origin whether it is
UNPROTECTED_WEB or PROTECTED_WEB. It delegates the decision what is an
EXTENSION to ChromeBrowsingDataRemoverDelegate. A new method has been added
to the BrowsingDataRemoverDelegate for this purpose.

BrowsingDataHelper now serves no other purpose than recognizing webby and
extension URLs for various classes in browsing_data/ and can be eventually
deprecated (though doing so in this CL would significantly blow it in size).

The behavior of UNPROTECTED_WEB and PROTECTED_WEB is already sufficiently
tested in StoragePartition-related tests of BrowsingDataRemoverTest. We just
had to replace MockExtensionSpecialStoragePolicy with the regular
MockSpecialStoragePolicy. Testcases from BrowsingDataHelperTest have been moved
to ChromeBrowsingDataRemoverDelegateTest to cover the remaining EXTENSION type.

3. Since BrowsingDataRemover and ChromeBrowsingDataRemoverDelegate now host
both the data type and origin type enums, we changed the naming to call out
the difference; all data types are now prefixed with DATA_TYPE, all origin
types with ORIGIN_TYPE.

4. All other files contain equivalent replacements.

TBR=dbeam@chromium.org,jochen@chromium.org
BUG=668114
==========

to

==========
Split browsing data masks between content and embedder

As a part of the refactoring to move content-related browsing data
infrastructure to content, we split BrowsingDataRemover::RemoveDataMask
and BrowsingDataHelper::OriginTypeMask between content and embedder.

The CL consists of the following steps:

1. BrowsingDataRemover::RemoveDataMask now only contains content datatypes.
Chrome-specific datatypes are moved to ChromeBrowsingDataRemoverDelegate.
The enum in ChromeBrowsingDataRemoverDelegate also copies all values
from BrowsingDataRemover so that this enum inheritance better mimics
class inheritance. Note that this is a second attempt at this change, following
the discussion in https://codereview.chromium.org/2697123004/ where we tried
converting enums into pointers first.

The concept of FILTERABLE_DATATYPES has been moved to
ChromeBrowsingDataRemoverDelegate, since all content data types are filterable.

2. The same exercise is done for BrowsingDataHelper::OriginTypeMask. Its
first two items, UNPROTECTED_WEB and PROTECTED_WEB correspond to the boolean
returned by content::SpecialStoragePolicy::IsProtected(). Therefore, they
are moved to content as a new enum in BrowsingDataRemover. The third item,
EXTENSION, is a Chrome-specific concept and stays in chrome/ code, as a new
enum in ChromeBrowsingDataRemoverDelegate.

BrowsingDataRemoverImpl is now able to tell about each origin whether it is
UNPROTECTED_WEB or PROTECTED_WEB. It delegates the decision what is an
EXTENSION to ChromeBrowsingDataRemoverDelegate. A new method has been added
to the BrowsingDataRemoverDelegate for this purpose.

BrowsingDataHelper now serves no other purpose than recognizing webby and
extension URLs for various classes in browsing_data/ and can be eventually
deprecated (though doing so in this CL would significantly blow it in size).

The behavior of UNPROTECTED_WEB and PROTECTED_WEB is already sufficiently
tested in StoragePartition-related tests of BrowsingDataRemoverTest. We just
had to replace MockExtensionSpecialStoragePolicy with the regular
MockSpecialStoragePolicy. Testcases from BrowsingDataHelperTest have been moved
to ChromeBrowsingDataRemoverDelegateTest to cover the remaining EXTENSION type.

3. Since BrowsingDataRemover and ChromeBrowsingDataRemoverDelegate now host
both the data type and origin type enums, we changed the naming to call out
the difference; all data types are now prefixed with DATA_TYPE, all origin
types with ORIGIN_TYPE.

4. All other files contain equivalent replacements.

TBR=dbeam@chromium.org,jochen@chromium.org
BUG=668114

Review-Url: https://codereview.chromium.org/2733393003
Cr-Commit-Position: refs/heads/master@{#456757}
Committed:
https://chromium.googlesource.com/chromium/src/+/1c2b3ca05716cc40a8a89f51527b...
==========

[commit-bot: I haz the power, 2017-03-14 17:58:44]

Committed patchset #10 (id:220001) as
https://chromium.googlesource.com/chromium/src/+/1c2b3ca05716cc40a8a89f51527b...