Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(10)

Unified Diff: chrome/browser/browsing_data/browsing_data_remover_impl.cc

Issue 2781083002: Fix the multi-threaded access to WeakPtr<BrowsingDataRemoverImpl> (Closed)
Patch Set: Created 3 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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 b157897e73a9e8c3a7e57764097d673c1f18a5c0..28bbd4af54fac29bb5d67306484e32fab2871dc0 100644
--- a/chrome/browser/browsing_data/browsing_data_remover_impl.cc
+++ b/chrome/browser/browsing_data/browsing_data_remover_impl.cc
@@ -60,25 +60,6 @@ base::Callback<void(T)> IgnoreArgument(const base::Closure& callback) {
return base::Bind(&IgnoreArgumentHelper<T>, callback);
}
-bool DoesOriginMatchMaskAndUrls(
- const base::WeakPtr<BrowsingDataRemoverImpl>& remover_weak_ptr,
- int origin_type_mask,
- const base::Callback<bool(const GURL&)>& predicate,
- const GURL& origin,
- storage::SpecialStoragePolicy* special_storage_policy) {
- // If BrowsingDataRemoverImpl is null, it is not possible to determine which
- // origins should have their data deleted, and so we do not delete
- // anything. This is not a problem, because this can only happen shortly
- // before shutdown and thus the deletion would likely not be able to
- // finish anyway.
- if (!remover_weak_ptr)
- return false;
-
- return predicate.Run(origin) &&
- remover_weak_ptr->DoesOriginMatchMask(origin_type_mask, origin,
- special_storage_policy);
-}
-
void ClearHttpAuthCacheOnIOThread(
scoped_refptr<net::URLRequestContextGetter> context_getter,
base::Time delete_begin) {
@@ -154,6 +135,47 @@ void BrowsingDataRemoverImpl::SubTask::CompletionCallback() {
forward_callback_.Run();
}
+BrowsingDataRemoverImpl::OriginMaskAndURLMatcher::OriginMaskAndURLMatcher() {}
+BrowsingDataRemoverImpl::OriginMaskAndURLMatcher::~OriginMaskAndURLMatcher() {}
+
+bool BrowsingDataRemoverImpl::OriginMaskAndURLMatcher::
+ DoesOriginMatchMaskAndURLs(
+ int origin_type_mask,
+ const base::Callback<bool(const GURL&)>& predicate,
+ const GURL& origin,
+ storage::SpecialStoragePolicy* policy) const {
+ if (!predicate.is_null() && !predicate.Run(origin))
+ return false;
+
+ const std::vector<std::string>& schemes = url::GetWebStorageSchemes();
+ bool is_web_scheme =
+ (std::find(schemes.begin(), schemes.end(), origin.GetOrigin().scheme()) !=
+ schemes.end());
+
+ // If a websafe origin is unprotected, it matches iff UNPROTECTED_WEB.
+ if ((!policy || !policy->IsStorageProtected(origin.GetOrigin())) &&
+ is_web_scheme && (origin_type_mask & ORIGIN_TYPE_UNPROTECTED_WEB)) {
+ return true;
+ }
+ origin_type_mask &= ~ORIGIN_TYPE_UNPROTECTED_WEB;
+
+ // Hosted applications (protected and websafe origins) iff PROTECTED_WEB.
+ if (policy && policy->IsStorageProtected(origin.GetOrigin()) &&
+ is_web_scheme && (origin_type_mask & ORIGIN_TYPE_PROTECTED_WEB)) {
+ return true;
+ }
+ origin_type_mask &= ~ORIGIN_TYPE_PROTECTED_WEB;
+
+ DCHECK(embedder_matcher_ || !origin_type_mask)
+ << "The mask contains embedder-defined origin types, but there is no "
+ << "embedder delegate matcher to process them.";
+
+ if (embedder_matcher_)
+ return embedder_matcher_.Run(origin_type_mask, origin, policy);
+
+ return false;
+}
+
BrowsingDataRemoverImpl::BrowsingDataRemoverImpl(
content::BrowserContext* browser_context)
: browser_context_(browser_context),
@@ -169,6 +191,7 @@ BrowsingDataRemoverImpl::BrowsingDataRemoverImpl(
clear_channel_ids_(sub_task_forward_callback_),
clear_http_auth_cache_(sub_task_forward_callback_),
clear_storage_partition_data_(sub_task_forward_callback_),
+ origin_mask_and_url_matcher_(new OriginMaskAndURLMatcher()),
weak_ptr_factory_(this) {
DCHECK(browser_context_);
}
@@ -202,6 +225,11 @@ void BrowsingDataRemoverImpl::SetRemoving(bool is_removing) {
void BrowsingDataRemoverImpl::SetEmbedderDelegate(
std::unique_ptr<BrowsingDataRemoverDelegate> embedder_delegate) {
embedder_delegate_ = std::move(embedder_delegate);
+
+ if (embedder_delegate_) {
+ origin_mask_and_url_matcher_->set_embedder_matcher(
+ embedder_delegate_->GetMaskMatcherFunction());
+ }
}
BrowsingDataRemoverDelegate*
@@ -213,35 +241,8 @@ bool BrowsingDataRemoverImpl::DoesOriginMatchMask(
int origin_type_mask,
const GURL& origin,
storage::SpecialStoragePolicy* policy) const {
- const std::vector<std::string>& schemes = url::GetWebStorageSchemes();
- bool is_web_scheme =
- (std::find(schemes.begin(), schemes.end(), origin.GetOrigin().scheme()) !=
- schemes.end());
-
- // If a websafe origin is unprotected, it matches iff UNPROTECTED_WEB.
- if ((!policy || !policy->IsStorageProtected(origin.GetOrigin())) &&
- is_web_scheme && (origin_type_mask & ORIGIN_TYPE_UNPROTECTED_WEB)) {
- return true;
- }
- origin_type_mask &= ~ORIGIN_TYPE_UNPROTECTED_WEB;
-
- // Hosted applications (protected and websafe origins) iff PROTECTED_WEB.
- if (policy && policy->IsStorageProtected(origin.GetOrigin()) &&
- is_web_scheme && (origin_type_mask & ORIGIN_TYPE_PROTECTED_WEB)) {
- return true;
- }
- origin_type_mask &= ~ORIGIN_TYPE_PROTECTED_WEB;
-
- DCHECK(embedder_delegate_ || !origin_type_mask)
- << "The mask contains embedder-defined origin types, but there is no "
- << "embedder delegate to process them.";
-
- if (embedder_delegate_) {
- return embedder_delegate_->DoesOriginMatchEmbedderMask(origin_type_mask,
- origin, policy);
- }
-
- return false;
+ return origin_mask_and_url_matcher_->DoesOriginMatchMaskAndURLs(
+ origin_type_mask, base::Callback<bool(const GURL&)>(), origin, policy);
}
void BrowsingDataRemoverImpl::Remove(const base::Time& delete_begin,
@@ -503,8 +504,8 @@ void BrowsingDataRemoverImpl::RemoveImpl(
storage_partition->ClearData(
storage_partition_remove_mask, quota_storage_remove_mask,
- base::Bind(&DoesOriginMatchMaskAndUrls, weak_ptr_factory_.GetWeakPtr(),
- origin_type_mask_, filter),
+ base::Bind(&OriginMaskAndURLMatcher::DoesOriginMatchMaskAndURLs,
+ origin_mask_and_url_matcher_, origin_type_mask_, filter),
cookie_matcher, delete_begin_, delete_end_,
clear_storage_partition_data_.GetCompletionCallback());
}

Powered by Google App Engine
This is Rietveld 408576698