Fix the multi-threaded access to WeakPtr<BrowsingDataRemoverImpl>

chrome/browser/browsing_data/browsing_data_remover_impl.cc

 // Copyright (c) 2012 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.
 
 #include "chrome/browser/browsing_data/browsing_data_remover_impl.h"
 
 #include <map>
 #include <set>
 #include <string>
 #include <utility>
@@ skipping 42 matching lines @@
   callback.Run();
 }
 
 // Another convenience method to turn a callback without arguments into one that
 // accepts (and ignores) a single argument.
 template <typename T>
 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) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   net::HttpNetworkSession* http_session = context_getter->GetURLRequestContext()
                                               ->http_transaction_factory()
                                               ->GetSession();
   DCHECK(http_session);
   http_session->http_auth_cache()->ClearEntriesAddedWithin(base::Time::Now() -
@@ skipping 55 matching lines @@
                     weak_ptr_factory_.GetWeakPtr());
 }
 
 void BrowsingDataRemoverImpl::SubTask::CompletionCallback() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(is_pending_);
   is_pending_ = false;
   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),
       remove_mask_(-1),
       origin_type_mask_(-1),
       is_removing_(false),
       sub_task_forward_callback_(
           base::Bind(&BrowsingDataRemoverImpl::NotifyIfDone,
                      base::Unretained(this))),
       synchronous_clear_operations_(sub_task_forward_callback_),
       clear_embedder_data_(sub_task_forward_callback_),
       clear_cache_(sub_task_forward_callback_),
       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_);
 }
 
 BrowsingDataRemoverImpl::~BrowsingDataRemoverImpl() {
   if (!task_queue_.empty()) {
     VLOG(1) << "BrowsingDataRemoverImpl shuts down with " << task_queue_.size()
             << " pending tasks";
   }
 
@@ skipping 13 matching lines @@
 }
 
 void BrowsingDataRemoverImpl::SetRemoving(bool is_removing) {
   DCHECK_NE(is_removing_, is_removing);
   is_removing_ = 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*
 BrowsingDataRemoverImpl::GetEmbedderDelegate() const {
   return embedder_delegate_.get();
 }
 
 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,
                                  const base::Time& delete_end,
                                  int remove_mask,
                                  int origin_type_mask) {
   RemoveInternal(delete_begin, delete_end, remove_mask, origin_type_mask,
                  std::unique_ptr<BrowsingDataFilterBuilder>(), nullptr);
 }
 
@@ skipping 241 matching lines @@
     // partition, create a cookie matcher function.
     content::StoragePartition::CookieMatcherFunction cookie_matcher;
     if (!filter_builder.IsEmptyBlacklist() &&
         (storage_partition_remove_mask &
             content::StoragePartition::REMOVE_DATA_MASK_COOKIES)) {
       cookie_matcher = filter_builder.BuildCookieFilter();
     }
 
     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());
   }
 
   //////////////////////////////////////////////////////////////////////////////
   // CACHE
   if (remove_mask & DATA_TYPE_CACHE) {
     base::RecordAction(UserMetricsAction("ClearBrowsingData_Cache"));
 
     // TODO(msramek): Clear the cache of all renderers.
@@ skipping 143 matching lines @@
 
   if (completion_inhibitor_) {
     completion_inhibitor_->OnBrowsingDataRemoverWouldComplete(
         this, base::Bind(&BrowsingDataRemoverImpl::Notify,
                          weak_ptr_factory_.GetWeakPtr()));
     return;
   }
 
   Notify();
 }