Commit 3248025c authored by msramek's avatar msramek Committed by Commit bot

Extract embedder-specific data types from BrowsingDataRemover

Define BrowsingDataRemoverDelegate, a class to which BrowsingDataRemover
will delegate all embedder-specific deletions. This class is in 1:1
relationship to BrowsingDataRemover and is owned by it.

Create a Chrome-specific subclass ChromeBrowsingDataRemoverDelegate by
extracting all Chrome datatypes from BrowsingDataRemover::RemoveImpl().

This is the first step of the implementation plan. See the design doc for more
background and explanation of the next steps:
https://docs.google.com/document/d/1I6m4QwbTNhG6wdtazamhTnArJN-UMUGqpvwH6InBEaM/edit#heading=h.m5pzu7ah79n9

This change is covered by BrowsingDataRemoverTest and
BrowsingDataRemoverBrowserTest.

NOTES FOR REVIEWERS:
1. This CL tries to keep the diffs at minimum for easier review, even if the
method ordering could be improved. That will be improved in a follow-up CL.
2. However, the diffs widely differ in BrowsingDataRemover::RemoveImpl and
ChromeBrowsingDataRemoverDelegate::RemoveEmbedderData. Fortunately, one can
convince themself that they are still equivalent by trusting the test coverage
(58 unittests and 6 browsertests).
3. One can convince themself that no datatype was left out by checking the
list of asynchronous tasks, such as "waiting_for_clear_cache_",
"waiting_for_clear_cookies_" etc. present in both .h files and in the AllDone()
method of both classes.
4. The distribution of datatypes might not be final. For example, downloads are
known to content but have a chrome/ dependency. Similarly, the DNS cache lives
in net/ but is retrieved via a class in chrome/. Future CLs might correct their
placement based on how strong the chrome/ dependencies are.

Patchset 3: Simplify the "bool waiting_for_clear_<type>" paradigm by encapsulating it to a SubTask class.

BUG=668114

Review-Url: https://codereview.chromium.org/2554413002
Cr-Commit-Position: refs/heads/master@{#438175}
parent c8dcfe32
...@@ -168,6 +168,7 @@ split_static_library("browser") { ...@@ -168,6 +168,7 @@ split_static_library("browser") {
"browsing_data/browsing_data_quota_helper_impl.h", "browsing_data/browsing_data_quota_helper_impl.h",
"browsing_data/browsing_data_remover.cc", "browsing_data/browsing_data_remover.cc",
"browsing_data/browsing_data_remover.h", "browsing_data/browsing_data_remover.h",
"browsing_data/browsing_data_remover_delegate.h",
"browsing_data/browsing_data_remover_factory.cc", "browsing_data/browsing_data_remover_factory.cc",
"browsing_data/browsing_data_remover_factory.h", "browsing_data/browsing_data_remover_factory.h",
"browsing_data/browsing_data_service_worker_helper.cc", "browsing_data/browsing_data_service_worker_helper.cc",
...@@ -176,6 +177,8 @@ split_static_library("browser") { ...@@ -176,6 +177,8 @@ split_static_library("browser") {
"browsing_data/cache_counter.h", "browsing_data/cache_counter.h",
"browsing_data/canonical_cookie_hash.cc", "browsing_data/canonical_cookie_hash.cc",
"browsing_data/canonical_cookie_hash.h", "browsing_data/canonical_cookie_hash.h",
"browsing_data/chrome_browsing_data_remover_delegate.cc",
"browsing_data/chrome_browsing_data_remover_delegate.h",
"browsing_data/cookies_tree_model.cc", "browsing_data/cookies_tree_model.cc",
"browsing_data/cookies_tree_model.h", "browsing_data/cookies_tree_model.h",
"browsing_data/downloads_counter.cc", "browsing_data/downloads_counter.cc",
......
// 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_
class BrowsingDataFilterBuilder;
class BrowsingDataRemoverDelegate {
public:
virtual ~BrowsingDataRemoverDelegate() {}
virtual 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) = 0;
};
#endif // CHROME_BROWSER_BROWSING_DATA_BROWSING_DATA_REMOVER_DELEGATE_H_
...@@ -4,9 +4,11 @@ ...@@ -4,9 +4,11 @@
#include "chrome/browser/browsing_data/browsing_data_remover_factory.h" #include "chrome/browser/browsing_data/browsing_data_remover_factory.h"
#include "base/memory/ptr_util.h"
#include "base/memory/singleton.h" #include "base/memory/singleton.h"
#include "chrome/browser/autofill/personal_data_manager_factory.h" #include "chrome/browser/autofill/personal_data_manager_factory.h"
#include "chrome/browser/browsing_data/browsing_data_remover.h" #include "chrome/browser/browsing_data/browsing_data_remover.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/content_settings/host_content_settings_map_factory.h"
#include "chrome/browser/domain_reliability/service_factory.h" #include "chrome/browser/domain_reliability/service_factory.h"
#include "chrome/browser/history/history_service_factory.h" #include "chrome/browser/history/history_service_factory.h"
...@@ -92,5 +94,8 @@ content::BrowserContext* BrowsingDataRemoverFactory::GetBrowserContextToUse( ...@@ -92,5 +94,8 @@ content::BrowserContext* BrowsingDataRemoverFactory::GetBrowserContextToUse(
KeyedService* BrowsingDataRemoverFactory::BuildServiceInstanceFor( KeyedService* BrowsingDataRemoverFactory::BuildServiceInstanceFor(
content::BrowserContext* context) const { content::BrowserContext* context) const {
return new BrowsingDataRemover(context); BrowsingDataRemover* remover = new BrowsingDataRemover(context);
remover->set_embedder_delegate(
base::MakeUnique<ChromeBrowsingDataRemoverDelegate>(context));
return remover;
} }
...@@ -34,6 +34,7 @@ ...@@ -34,6 +34,7 @@
#include "chrome/browser/browsing_data/browsing_data_helper.h" #include "chrome/browser/browsing_data/browsing_data_helper.h"
#include "chrome/browser/browsing_data/browsing_data_remover_factory.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/browsing_data_remover_test_util.h"
#include "chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h"
#include "chrome/browser/browsing_data/registrable_domain_filter_builder.h" #include "chrome/browser/browsing_data/registrable_domain_filter_builder.h"
#include "chrome/browser/content_settings/host_content_settings_map_factory.h" #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
#include "chrome/browser/domain_reliability/service_factory.h" #include "chrome/browser/domain_reliability/service_factory.h"
...@@ -1129,8 +1130,9 @@ class BrowsingDataRemoverTest : public testing::Test { ...@@ -1129,8 +1130,9 @@ class BrowsingDataRemoverTest : public testing::Test {
BrowsingDataRemoverFactory::GetForBrowserContext(profile_.get()); BrowsingDataRemoverFactory::GetForBrowserContext(profile_.get());
#if BUILDFLAG(ANDROID_JAVA_UI) #if BUILDFLAG(ANDROID_JAVA_UI)
remover_->OverrideWebappRegistryForTesting( static_cast<ChromeBrowsingDataRemoverDelegate*>(
std::unique_ptr<WebappRegistry>(new TestWebappRegistry())); remover_->get_embedder_delegate())->OverrideWebappRegistryForTesting(
base::WrapUnique<WebappRegistry>(new TestWebappRegistry()));
#endif #endif
} }
......
// 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
// Callback for when cookies have been deleted. Invokes NotifyIfDone.
void OnClearedCookies();
#if BUILDFLAG(ENABLE_PLUGINS)
// PepperFlashSettingsManager::Client implementation.
void OnDeauthorizeFlashContentLicensesCompleted(uint32_t request_id,
bool success) override;
#endif
// 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_;
// A callback to NotifyIfDone() used by SubTasks instances.
const base::Closure sub_task_forward_callback_;
// Keeping track of various subtasks to be completed.
// Non-zero if waiting for SafeBrowsing cookies to be cleared.
int clear_cookies_count_ = 0;
BrowsingDataRemover::SubTask synchronous_clear_operations_;
BrowsingDataRemover::SubTask clear_autofill_origin_urls_;
BrowsingDataRemover::SubTask clear_flash_content_licenses_;
BrowsingDataRemover::SubTask clear_domain_reliability_monitor_;
BrowsingDataRemover::SubTask clear_form_;
BrowsingDataRemover::SubTask clear_history_;
BrowsingDataRemover::SubTask clear_keyword_data_;
#if !defined(DISABLE_NACL)
BrowsingDataRemover::SubTask clear_nacl_cache_;
BrowsingDataRemover::SubTask clear_pnacl_cache_;
#endif
BrowsingDataRemover::SubTask clear_hostname_resolution_cache_;
BrowsingDataRemover::SubTask clear_network_predictor_;
BrowsingDataRemover::SubTask clear_networking_history_;
BrowsingDataRemover::SubTask clear_passwords_;
BrowsingDataRemover::SubTask clear_passwords_stats_;
BrowsingDataRemover::SubTask clear_platform_keys_;
#if BUILDFLAG(ANDROID_JAVA_UI)
BrowsingDataRemover::SubTask clear_precache_history_;
BrowsingDataRemover::SubTask clear_offline_page_data_;
#endif
#if BUILDFLAG(ENABLE_WEBRTC)
BrowsingDataRemover::SubTask clear_webrtc_logs_;
#endif
BrowsingDataRemover::SubTask clear_auto_sign_in_;
#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_
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment