Extract embedder-specific data types from BrowsingDataRemover

chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h

Index: chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h
diff --git a/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h b/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h
new file mode 100644
index 0000000000000000000000000000000000000000..cb8ffbda894298b2857382dfd7cf2b03173262c9
--- /dev/null
+++ b/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h
@@ -0,0 +1,202 @@
+// 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_CHROME_BROWSING_DATA_REMOVER_DELEGATE_H_
+#define CHROME_BROWSER_BROWSING_DATA_CHROME_BROWSING_DATA_REMOVER_DELEGATE_H_
+
+#include "base/memory/ref_counted.h"
+#include "base/memory/weak_ptr.h"
+#include "base/synchronization/waitable_event_watcher.h"
+#include "base/task/cancelable_task_tracker.h"
+#include "chrome/browser/browsing_data/browsing_data_remover.h"
+#include "chrome/browser/browsing_data/browsing_data_remover_delegate.h"
+#include "chrome/common/features.h"
+#include "components/browsing_data/core/browsing_data_utils.h"
+#include "components/offline_pages/core/offline_page_model.h"
+#include "components/search_engines/template_url_service.h"
+#include "media/media_features.h"
+#include "ppapi/features/features.h"
+
+#if BUILDFLAG(ENABLE_PLUGINS)
+#include "chrome/browser/pepper_flash_settings_manager.h"
+#endif
+
+#if defined(OS_CHROMEOS)
+#include "chromeos/dbus/dbus_method_call_status.h"
+#endif
+
+class WebappRegistry;
+
+// A delegate used by BrowsingDataRemover to delete data specific to Chrome
+// as the embedder.
+class ChromeBrowsingDataRemoverDelegate : public BrowsingDataRemoverDelegate
+#if BUILDFLAG(ENABLE_PLUGINS)
+    , public PepperFlashSettingsManager::Client
+#endif
+{
+ public:
+  ChromeBrowsingDataRemoverDelegate(content::BrowserContext* browser_context);
+  ~ChromeBrowsingDataRemoverDelegate() override;
+
+  // Removes Chrome-specific data.
+  void RemoveEmbedderData(
+      const base::Time& delete_begin,
+      const base::Time& delete_end,
+      int remove_mask,
+      const BrowsingDataFilterBuilder& filter_builder,
+      int origin_type_mask,
+      const base::Closure& callback) override;
+
+#if BUILDFLAG(ANDROID_JAVA_UI)
+  void OverrideWebappRegistryForTesting(
+      std::unique_ptr<WebappRegistry> webapp_registry);
+#endif
+
+ private:
+  // If AllDone(), calls the callback provided in RemoveEmbedderData().
+  void NotifyIfDone();
+
+  // Whether there are no running deletion tasks.
+  bool AllDone();
+
+  // Callback for when TemplateURLService has finished loading. Clears the data,
+  // clears the respective waiting flag, and invokes NotifyIfDone.
+  void OnKeywordsLoaded(base::Callback<bool(const GURL&)> url_filter);
+
+#if defined (OS_CHROMEOS)
+  void OnClearPlatformKeys(chromeos::DBusMethodCallStatus call_status,
+                           bool result);
+#endif
+
+  // Called when history deletion is done.
+  void OnHistoryDeletionDone();
+
+  // Callback for when the hostname resolution cache has been cleared.
+  // Clears the respective waiting flag and invokes NotifyIfDone.
+  void OnClearedHostnameResolutionCache();
+
+  // Callback for when speculative data in the network Predictor has been
+  // cleared. Clears the respective waiting flag and invokes
+  // NotifyIfDone.
+  void OnClearedNetworkPredictor();
+
+  // Callback for when network related data in ProfileIOData has been cleared.
+  // Clears the respective waiting flag and invokes NotifyIfDone.
+  void OnClearedNetworkingHistory();
+
+#if !defined(DISABLE_NACL)
+  // Callback for when the NaCl cache has been deleted. Invokes
+  // NotifyIfDone.
+  void ClearedNaClCache();
+
+  // Callback for when the PNaCl translation cache has been deleted. Invokes
+  // NotifyIfDone.
+  void ClearedPnaclCache();
+#endif
+
+  // Callback for when cookies have been deleted. Invokes NotifyIfDone.
+  void OnClearedCookies();
+
+  // Callback for when passwords for the requested time range have been cleared.
+  void OnClearedPasswords();
+
+  // Callback for when passwords stats for the requested time range have been
+  // cleared.
+  void OnClearedPasswordsStats();
+
+  // Callback for when the autosignin state of passwords has been revoked.
+  void OnClearedAutoSignIn();
+
+  // Callback from the above method.
+  void OnClearedFormData();
+
+  // Callback for when the Autofill profile and credit card origin URLs have
+  // been deleted.
+  void OnClearedAutofillOriginURLs();
+
+#if BUILDFLAG(ENABLE_WEBRTC)
+  // Callback on UI thread when the WebRTC logs have been deleted.
+  void OnClearedWebRtcLogs();
+#endif
+
+#if BUILDFLAG(ANDROID_JAVA_UI)
+  // Callback on UI thread when the precache history has been cleared.
+  void OnClearedPrecacheHistory();
+
+  // Callback on UI thread when the offline page data has been cleared.
+  void OnClearedOfflinePageData(
+      offline_pages::OfflinePageModel::DeletePageResult result);
+#endif
+
+#if BUILDFLAG(ENABLE_PLUGINS)
+  // PepperFlashSettingsManager::Client implementation.
+  void OnDeauthorizeFlashContentLicensesCompleted(uint32_t request_id,
+                                                  bool success) override;
+#endif
+
+  void OnClearedDomainReliabilityMonitor();
+
+  // The profile for which the data will be deleted.
+  Profile* profile_;
+
+  // Start time to delete from.
+  base::Time delete_begin_;
+
+  // End time to delete to.
+  base::Time delete_end_;
+
+  // Completion callback to call when all data are deleted.
+  base::Closure callback_;
+
+  bool waiting_for_synchronous_clear_operations_ = false;

[Bernhard Bauer, 2016/12/12 14:52:06]

If we're rewriting this class anyway...

I feel a bit like this list and the list of callbacks have grown out of hand.
Would it make sense to replace these with a list of tasks, which essentially
have a promise-style interface, so each of the callbacks above can be replaced
with a callback with the fulfillment method of a task? We could use those tasks
in the content/ BrowsingDataRemover as well.

[msramek, 2016/12/12 20:48:56]

Done.

I created a "SubTask" class, the "sub" part is supposed to emphasize that this
is something smaller than "RemovalTask".

I replaced as much as possible in BDR and CBDRD, just left some custom callback
methods as they were.

This cut a lot of lines of code, but blew up the constructor (unfortunately, a
SubTask needs to have some kind of link to its owner), and also the memory
footprint (because of weak ptr factories; this is something that might be worth
optimizing).

I also tried optimizing the callback to NotifyIfDone() that needs to be shared
by all instances of SubTask by instantiating it once and passing a reference, as
opposed to copying it to each instance.

[Bernhard Bauer, 2016/12/13 13:26:22]

OK. We could cut down the constructor (and NotifyIfDone() as well!) by putting
all the subtasks into a vector (after adding the required move constructors),
and only pushing the subtasks to it that are actually needed.

For reducing the memory footprint, we could plumb all the callbacks through the
existing weak_ptr_factory_ in the outer class instead, which will cancel all
callbacks on destruction.
Eh, callback copies are actually pretty cheap (they have internal refcounted
storage).

+  bool waiting_for_clear_autofill_origin_urls_ = false;
+  bool waiting_for_clear_flash_content_licenses_ = false;
+  // Non-zero if waiting for cookies to be cleared.
+  int waiting_for_clear_cookies_count_ = 0;
+  bool waiting_for_clear_domain_reliability_monitor_ = false;
+  bool waiting_for_clear_form_ = false;
+  bool waiting_for_clear_history_ = false;
+  bool waiting_for_clear_keyword_data_ = false;
+  bool waiting_for_clear_nacl_cache_ = false;
+  bool waiting_for_clear_hostname_resolution_cache_ = false;
+  bool waiting_for_clear_network_predictor_ = false;
+  bool waiting_for_clear_networking_history_ = false;
+  bool waiting_for_clear_passwords_ = false;
+  bool waiting_for_clear_passwords_stats_ = false;
+  bool waiting_for_clear_platform_keys_ = false;
+  bool waiting_for_clear_pnacl_cache_ = false;
+#if BUILDFLAG(ANDROID_JAVA_UI)
+  bool waiting_for_clear_precache_history_ = false;
+  bool waiting_for_clear_offline_page_data_ = false;
+#endif
+
+#if BUILDFLAG(ENABLE_WEBRTC)
+  bool waiting_for_clear_webrtc_logs_ = false;
+#endif
+  bool waiting_for_clear_auto_sign_in_ = false;
+
+#if BUILDFLAG(ENABLE_PLUGINS)
+  uint32_t deauthorize_flash_content_licenses_request_id_ = 0;
+
+  // Used to deauthorize content licenses for Pepper Flash.
+  std::unique_ptr<PepperFlashSettingsManager> pepper_flash_settings_manager_;
+#endif
+
+  // Used if we need to clear history.
+  base::CancelableTaskTracker history_task_tracker_;
+
+  std::unique_ptr<TemplateURLService::Subscription> template_url_sub_;
+
+#if BUILDFLAG(ANDROID_JAVA_UI)
+  // WebappRegistry makes calls across the JNI. In unit tests, the Java side is
+  // not initialised, so the registry must be mocked out.
+  std::unique_ptr<WebappRegistry> webapp_registry_;
+#endif
+
+  base::WeakPtrFactory<ChromeBrowsingDataRemoverDelegate> weak_ptr_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(ChromeBrowsingDataRemoverDelegate);
+};
+
+#endif  // CHROME_BROWSER_BROWSING_DATA_CHROME_BROWSING_DATA_REMOVER_DELEGATE_H_