Fix the multi-threaded access to WeakPtr<BrowsingDataRemoverImpl>

chrome/browser/browsing_data/browsing_data_remover_impl.h

 // 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.
 
 #ifndef CHROME_BROWSER_BROWSING_DATA_BROWSING_DATA_REMOVER_IMPL_H_
 #define CHROME_BROWSER_BROWSING_DATA_BROWSING_DATA_REMOVER_IMPL_H_
 
 #include <stdint.h>
 
 #include <queue>
 #include <set>
 
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
+#include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
 #include "base/synchronization/waitable_event_watcher.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
 #include "chrome/browser/browsing_data/browsing_data_remover.h"
 #include "chrome/common/features.h"
 #include "ppapi/features/features.h"
 #include "storage/common/quota/quota_types.h"
 #include "url/gurl.h"
@@ skipping 140 matching lines @@
     ~RemovalTask();
 
     base::Time delete_begin;
     base::Time delete_end;
     int remove_mask;
     int origin_type_mask;
     std::unique_ptr<content::BrowsingDataFilterBuilder> filter_builder;
     Observer* observer;
   };
 
+  // OriginMaskAndURLMatcher handles the implementation of DoesOriginMatchMask.
+  // As an intrinsic part of BrowsingDataRemoverImpl, it should be destroyed
+  // together with it on the UI thread. However, it is possible that at this
+  // time a StoragePartition is still using it on the IO thread. Therefore,
+  // we hold it ref-counted.
+  class OriginMaskAndURLMatcher
+      : public base::RefCountedThreadSafe<OriginMaskAndURLMatcher> {
+   public:
+    using MaskMatcherFunction =
+        BrowsingDataRemoverDelegate::MaskMatcherFunction;
+
+    OriginMaskAndURLMatcher();
+
+    // Returns whether |origin| matches |origin_type_mask| given the special
+    // storage |policy|; and if |predicate| is not null, then also whether
+    // it matches |predicate|.
+    bool DoesOriginMatchMaskAndURLs(
+        int origin_type_mask,
+        const base::Callback<bool(const GURL&)>& predicate,
+        const GURL& origin,
+        storage::SpecialStoragePolicy* policy) const;
+
+    // Called from BrowsingDataRemoverImpl::SetEmbedderDelegate to inject
+    // the matching function for embedder datatypes.
+    void set_embedder_matcher(const MaskMatcherFunction& embedder_matcher) {
+      embedder_matcher_ = embedder_matcher;
+    }
+
+   private:
+    friend class base::RefCountedThreadSafe<OriginMaskAndURLMatcher>;
+    ~OriginMaskAndURLMatcher();
+
+    MaskMatcherFunction embedder_matcher_;

[Bernhard Bauer, 2017/03/30 10:49:26]

If this is the only state in the class, and it's a callback, so the object
itself is thread-safe, why do we need to wrap it in a class? Couldn't we just
make DoesOriginMatchMaskAndURLs() static and bind this as an additional
parameter?

[msramek, 2017/04/06 12:06:38]

Because I panicked and overengineered it. Done.

+  };
+
   void Shutdown() override;
 
   // Setter for |is_removing_|; DCHECKs that we can only start removing if we're
   // not already removing, and vice-versa.
   void SetRemoving(bool is_removing);
 
   // Executes the next removal task. Called after the previous task was finished
   // or directly from Remove() if the task queue was empty.
   void RunNextTask();
 
@@ skipping 60 matching lines @@
   SubTask clear_channel_ids_;
   SubTask clear_http_auth_cache_;
   SubTask clear_storage_partition_data_;
 
   // Observers of the global state and individual tasks.
   base::ObserverList<Observer, true> observer_list_;
 
   // We do not own this.
   content::StoragePartition* storage_partition_for_testing_ = nullptr;
 
+  // Handles the implementation of DoesOriginMatchMask(). Used on the UI and IO
+  // threads.
+  scoped_refptr<OriginMaskAndURLMatcher> origin_mask_and_url_matcher_;
+
   base::WeakPtrFactory<BrowsingDataRemoverImpl> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(BrowsingDataRemoverImpl);
 };
 
 #endif  // CHROME_BROWSER_BROWSING_DATA_BROWSING_DATA_REMOVER_IMPL_H_