Move BrowsingDataRemover to content/

content/public/browser/browsing_data_remover_delegate.h

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_BROWSER_BROWSING_DATA_BROWSING_DATA_REMOVER_DELEGATE_H_
-#define CHROME_BROWSER_BROWSING_DATA_BROWSING_DATA_REMOVER_DELEGATE_H_
+#ifndef CONTENT_PUBLIC_BROWSER_BROWSING_DATA_REMOVER_DELEGATE_H_
+#define CONTENT_PUBLIC_BROWSER_BROWSING_DATA_REMOVER_DELEGATE_H_
 
 #include "base/callback_forward.h"
+#include "content/common/content_export.h"
 
 class GURL;
 
 namespace base {
 class Time;
 }
 
-namespace content {
-class BrowsingDataFilterBuilder;
-}
-
 namespace storage {
 class SpecialStoragePolicy;
 }
 
-class BrowsingDataRemoverDelegate {
+namespace content {
+
+class BrowsingDataFilterBuilder;
+
+class CONTENT_EXPORT BrowsingDataRemoverDelegate {

[jam, 2017/04/20 00:39:48]

ditto re export

[msramek, 2017/04/21 15:22:12]

Done.

  public:
   // Determines whether |origin| matches |origin_type_mask| given
   // the |special_storage_policy|.
   typedef base::Callback<bool(int origin_type_mask,
                               const GURL& origin,
                               storage::SpecialStoragePolicy* policy)>
       EmbedderOriginTypeMatcher;
 
   virtual ~BrowsingDataRemoverDelegate() {}
 
   // Returns a MaskMatcherFunction to match embedder's origin types.
   // This MaskMatcherFunction will be called with an |origin_type_mask|
   // parameter containing ONLY embedder-defined origin types, and must be able
   // to handle ALL embedder-defined typed. It must be static and support
   // being called on the UI and IO thread.
   virtual EmbedderOriginTypeMatcher GetOriginTypeMatcher() const = 0;

[jam, 2017/04/20 00:39:48]

why is this passed through a delegate, can't it just be an argument for
constructing the BrowsingDataRemover object?

[msramek, 2017/04/21 15:22:12]

There should be one such method/callback per delegate (or rather per delegate
class, which could be achieved with templates, but rather not). When delegate
changes using SetEmbedderDelegate(), it needs to provide a new
EmbedderOriginTypeMatcher.

However, in practice delegates don't need to change during the lifetime of
BrowsingDataRemover, so we can simplify this. Remove SetEmbedderDelegate() and
pass both methods as callbacks in the constructor.

That means replacing

virtual BrowsingDataRemoverDelegate*
BrowserContext::GetBrowsingDataRemoverDelegate() = 0;

with

virtual EmbedderOriginTypeMatcher BrowserContext::GetOriginTypeMatcher() = 0;
virtual base::Callback<...> BrowserContext::GetRemoveEmbedderDataCallback() = 0;

?

Which seems to me that we're trading one less class in content/public with a
messier BrowserContext interface. I just want to confirm if you're sure about
that.

[jam, 2017/04/21 22:58:30]

ah, I was not thinking of the case when content creates the data remover itself.
i.e. if chrome is always creating it, it could have passed this in as an
argument. so nvm then.

[msramek, 2017/04/25 01:29:09]

Acknowledged. I'm keeping the current structure then.

 
   // Removes embedder-specific data.
   virtual void RemoveEmbedderData(
       const base::Time& delete_begin,
       const base::Time& delete_end,
       int remove_mask,
-      const content::BrowsingDataFilterBuilder& filter_builder,
+      const BrowsingDataFilterBuilder& filter_builder,
       int origin_type_mask,
       const base::Closure& callback) = 0;

[jam, 2017/04/20 00:39:48]

if the first method is removed, can this one also be passed as a callback to
BrowsingDataRemover and then we can get rid of this delegate interface
altogether?

[msramek, 2017/04/21 15:22:12]

Yes. Please see the above answer.

 };
 
-#endif  // CHROME_BROWSER_BROWSING_DATA_BROWSING_DATA_REMOVER_DELEGATE_H_
+}  // namespace content
+
+#endif  // CONTENT_PUBLIC_BROWSER_BROWSING_DATA_REMOVER_DELEGATE_H_