Move plugins to ChromeBrowsingDataRemoverDelegate.

Move plugins to ChromeBrowsingDataRemoverDelegate.

Plugins are known to //content and can be deleted in bulk by
PluginDataRemover in //content/public. However, the filtered deletion of
plugin data is done by BrowsingDataFlashLSOHelper which has strong
//chrome dependencies (through PepperFlashSettingsManager, PluginPrefs
etc.). It is not clear yet whether these dependencies can be resolved,
and so it should be assumed that the filtered deletion of plugin data
must remain in //chrome.

As it makes no sense to split the PLUGINS datatype into bulk deletion
performed in //chrome and filtered deletion performed in //content, we
move the entire datatype into //chrome - at least for now. Note that
no functionality is lost, as //content can still use PluginDataRemover
for bulk removal of plugin data directly.

BUG=668114
Review-Url: https://codereview.chromium.org/2637043002
Cr-Commit-Position: refs/heads/master@{#444037}
Committed: https://chromium.googlesource.com/chromium/src/+/1e12faa9d69777beeccec850b17f79b98682d2a7

[msramek, 2017-01-17 10:45:30]

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

[commit-bot: I haz the power, 2017-01-17 10:45:46]

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-17 10:47:39]

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

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

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-17 10:47:49]

Patchset #1 (id:1) has been deleted

[msramek, 2017-01-17 10:52:05]

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

[msramek, 2017-01-17 10:52:05]

Hi Mike!

Please have a look!

I don't know yet if it is actually possible to move BrowsingDataFlashLSOHelper
to //content. But if it is, it's going to take some time again to remove all the
//chrome dependencies and I don't want to go down that rabbit hole right now.

I think it should be fine if we move BrowsingDataRemover to //content without
plugins at first, to unblock Clear-Site-Data, and we can investigate this later.
It's not a regression, since no datatypes are in //content as of now...

Thanks!
Martin

[Mike West, 2017-01-17 11:22:54]

On 2017/01/17 at 10:52:05, msramek wrote:
> Hi Mike!
> 
> Please have a look!
> 
> I don't know yet if it is actually possible to move BrowsingDataFlashLSOHelper
to //content. But if it is, it's going to take some time again to remove all the
//chrome dependencies and I don't want to go down that rabbit hole right now.
> 
> I think it should be fine if we move BrowsingDataRemover to //content without
plugins at first, to unblock Clear-Site-Data, and we can investigate this later.
It's not a regression, since no datatypes are in //content as of now...
> 
> Thanks!
> Martin

LGTM to move from one place in //chrome to a different place in //chrome, but
please add a TODO noting that this should probably move to //content. It would
be unfortunate if non-Chrome embedders missed the fact that they needed to
implement this bit themselves once they start migrating over to the new way of
doing things.

[commit-bot: I haz the power, 2017-01-17 11:30:49]

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

[commit-bot: I haz the power, 2017-01-17 11:30:50]

Dry run: This issue passed the CQ dry run.

[msramek, 2017-01-17 11:41:31]

Thanks!

There already is a comment and a TODO in the delegate, I now also added a
top-level comment with a TODO to browsing_data_remover.h.

[msramek, 2017-01-17 11:41:46]

The CQ bit was checked by msramek@chromium.org

[msramek, 2017-01-17 11:41:46]

The patchset sent to the CQ was uploaded after l-g-t-m from mkwst@chromium.org
Link to the patchset: https://codereview.chromium.org/2637043002/#ps40001
(title: "Added a TODO.")

[commit-bot: I haz the power, 2017-01-17 11:42:01]

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-17 12:22:18]

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

[commit-bot: I haz the power, 2017-01-17 12:22:20]

Failed to apply patch for
chrome/browser/browsing_data/browsing_data_remover_impl.cc:
While running git apply --index -p1;
  error: patch failed:
chrome/browser/browsing_data/browsing_data_remover_impl.cc:29
  error: chrome/browser/browsing_data/browsing_data_remover_impl.cc: patch does
not apply

Patch:       chrome/browser/browsing_data/browsing_data_remover_impl.cc
Index: chrome/browser/browsing_data/browsing_data_remover_impl.cc
diff --git a/chrome/browser/browsing_data/browsing_data_remover_impl.cc
b/chrome/browser/browsing_data/browsing_data_remover_impl.cc
index
927a3e936418de990b8ac262b68e6cec3ae9d1cf..431c3f1e7e75c2c1c49b24f763ab0801b17b9107
100644
--- a/chrome/browser/browsing_data/browsing_data_remover_impl.cc
+++ b/chrome/browser/browsing_data/browsing_data_remover_impl.cc
@@ -29,7 +29,6 @@
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/download_manager.h"
 #include "content/public/browser/notification_service.h"
-#include "content/public/browser/plugin_data_remover.h"
 #include "content/public/browser/storage_partition.h"
 #include "content/public/browser/user_metrics.h"
 #include "extensions/features/features.h"
@@ -47,10 +46,6 @@
 #include "storage/browser/quota/special_storage_policy.h"
 #include "url/origin.h"
 
-#if BUILDFLAG(ENABLE_PLUGINS)
-#include "chrome/browser/browsing_data/browsing_data_flash_lso_helper.h"
-#endif
-
 using base::UserMetricsAction;
 using content::BrowserContext;
 using content::BrowserThread;
@@ -162,9 +157,6 @@ BrowsingDataRemoverImpl::BrowsingDataRemoverImpl(
       remove_mask_(-1),
       origin_type_mask_(-1),
       is_removing_(false),
-#if BUILDFLAG(ENABLE_PLUGINS)
-      flash_lso_helper_(BrowsingDataFlashLSOHelper::Create(browser_context_)),
-#endif
       sub_task_forward_callback_(
           base::Bind(&BrowsingDataRemoverImpl::NotifyIfDone,
                      base::Unretained(this))),
@@ -503,38 +495,6 @@ void BrowsingDataRemoverImpl::RemoveImpl(
   }
 
  
//////////////////////////////////////////////////////////////////////////////
-  // REMOVE_PLUGINS
-#if BUILDFLAG(ENABLE_PLUGINS)
-  // Plugin is data not separated for protected and unprotected web origins. We
-  // check the origin_type_mask_ to prevent unintended deletion.
-  if (remove_mask & REMOVE_PLUGIN_DATA &&
-      origin_type_mask_ & BrowsingDataHelper::UNPROTECTED_WEB) {
-    content::RecordAction(UserMetricsAction("ClearBrowsingData_LSOData"));
-    clear_plugin_data_count_ = 1;
-
-    if (filter_builder.IsEmptyBlacklist()) {
-      DCHECK(!plugin_data_remover_);
-      plugin_data_remover_.reset(
-          content::PluginDataRemover::Create(browser_context_));
-      base::WaitableEvent* event =
-          plugin_data_remover_->StartRemoving(delete_begin_);
-
-      base::WaitableEventWatcher::EventCallback watcher_callback =
-          base::Bind(&BrowsingDataRemoverImpl::OnWaitableEventSignaled,
-                     weak_ptr_factory_.GetWeakPtr());
-      watcher_.StartWatching(event, watcher_callback);
-    } else {
-      // TODO(msramek): Store filters from the currently executed task on the
-      // object to avoid having to copy them to callback methods.
-      flash_lso_helper_->StartFetching(base::Bind(
-          &BrowsingDataRemoverImpl::OnSitesWithFlashDataFetched,
-          weak_ptr_factory_.GetWeakPtr(),
-          filter_builder.BuildPluginFilter()));
-    }
-  }
-#endif
-
- 
//////////////////////////////////////////////////////////////////////////////
   // CACHE
   if (remove_mask & REMOVE_CACHE) {
     // Tell the renderers to clear their cache.
@@ -607,13 +567,6 @@ void
BrowsingDataRemoverImpl::OverrideStoragePartitionForTesting(
   storage_partition_for_testing_ = storage_partition;
 }
 
-#if BUILDFLAG(ENABLE_PLUGINS)
-void BrowsingDataRemoverImpl::OverrideFlashLSOHelperForTesting(
-    scoped_refptr<BrowsingDataFlashLSOHelper> flash_lso_helper) {
-  flash_lso_helper_ = flash_lso_helper;
-}
-#endif
-
 const base::Time& BrowsingDataRemoverImpl::GetLastUsedBeginTime() {
   return delete_begin_;
 }
@@ -652,8 +605,7 @@ bool BrowsingDataRemoverImpl::AllDone() {
          !clear_cache_.is_pending() &&
          !clear_channel_ids_.is_pending() &&
          !clear_http_auth_cache_.is_pending() &&
-         !clear_storage_partition_data_.is_pending() &&
-         !clear_plugin_data_count_;
+         !clear_storage_partition_data_.is_pending();
 }
 
 void BrowsingDataRemoverImpl::Notify() {
@@ -710,46 +662,3 @@ void BrowsingDataRemoverImpl::NotifyIfDone() {
 
   Notify();
 }
-
-#if BUILDFLAG(ENABLE_PLUGINS)
-void BrowsingDataRemoverImpl::OnWaitableEventSignaled(
-    base::WaitableEvent* waitable_event) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-
-  DCHECK_EQ(1, clear_plugin_data_count_);
-  clear_plugin_data_count_ = 0;
-
-  plugin_data_remover_.reset();
-  watcher_.StopWatching();
-  NotifyIfDone();
-}
-
-void BrowsingDataRemoverImpl::OnSitesWithFlashDataFetched(
-    base::Callback<bool(const std::string&)> plugin_filter,
-    const std::vector<std::string>& sites) {
-  DCHECK_EQ(1, clear_plugin_data_count_);
-  clear_plugin_data_count_ = 0;
-
-  std::vector<std::string> sites_to_delete;
-  for (const std::string& site : sites) {
-    if (plugin_filter.Run(site))
-      sites_to_delete.push_back(site);
-  }
-
-  clear_plugin_data_count_ = sites_to_delete.size();
-
-  for (const std::string& site : sites_to_delete) {
-    flash_lso_helper_->DeleteFlashLSOsForSite(
-        site,
-        base::Bind(&BrowsingDataRemoverImpl::OnFlashDataDeleted,
-                   weak_ptr_factory_.GetWeakPtr()));
-  }
-
-  NotifyIfDone();
-}
-
-void BrowsingDataRemoverImpl::OnFlashDataDeleted() {
-  clear_plugin_data_count_--;
-  NotifyIfDone();
-}
-#endif

[msramek, 2017-01-17 12:32:10]

The CQ bit was checked by msramek@chromium.org

[msramek, 2017-01-17 12:32:11]

The patchset sent to the CQ was uploaded after l-g-t-m from mkwst@chromium.org
Link to the patchset: https://codereview.chromium.org/2637043002/#ps60001
(title: "Rebase.")

[commit-bot: I haz the power, 2017-01-17 12:32:25]

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-17 13:12:29]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1484656330752580,
"parent_rev": "b850623ef5632ca96ffcc65c13d036ca0ab3271d", "commit_rev":
"1e12faa9d69777beeccec850b17f79b98682d2a7"}

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

Description was changed from

==========
Move plugins to ChromeBrowsingDataRemoverDelegate.

Plugins are known to //content and can be deleted in bulk by
PluginDataRemover in //content/public. However, the filtered deletion of
plugin data is done by BrowsingDataFlashLSOHelper which has strong
//chrome dependencies (through PepperFlashSettingsManager, PluginPrefs
etc.). It is not clear yet whether these dependencies can be resolved,
and so it should be assumed that the filtered deletion of plugin data
must remain in //chrome.

As it makes no sense to split the PLUGINS datatype into bulk deletion
performed in //chrome and filtered deletion performed in //content, we
move the entire datatype into //chrome - at least for now. Note that
no functionality is lost, as //content can still use PluginDataRemover
for bulk removal of plugin data directly.

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

to

==========
Move plugins to ChromeBrowsingDataRemoverDelegate.

Plugins are known to //content and can be deleted in bulk by
PluginDataRemover in //content/public. However, the filtered deletion of
plugin data is done by BrowsingDataFlashLSOHelper which has strong
//chrome dependencies (through PepperFlashSettingsManager, PluginPrefs
etc.). It is not clear yet whether these dependencies can be resolved,
and so it should be assumed that the filtered deletion of plugin data
must remain in //chrome.

As it makes no sense to split the PLUGINS datatype into bulk deletion
performed in //chrome and filtered deletion performed in //content, we
move the entire datatype into //chrome - at least for now. Note that
no functionality is lost, as //content can still use PluginDataRemover
for bulk removal of plugin data directly.

BUG=668114

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

[commit-bot: I haz the power, 2017-01-17 13:12:59]

Committed patchset #3 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/1e12faa9d69777beeccec850b17f...