[Backport] Dependency for security bug 925035

Manual backport of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/1712949:

Move CodeCacheHost and GeneratedCodeCache to UI thread

See the issue for more details. It doesn't look this needs to live on
IO thread, this patch moves all the code cache code onto UI thread
so that security check can become easier.

(Alternatively we can only move CodeCacheHost to UI thread
but leave everything else on IO thread or on a sequenced task runner.
e.g. https://chromium-review.googlesource.com/c/chromium/src/+/1705540)

Bug: 985681
Change-Id: I3a780902135b19d2c55a5b844c230aa694856667
Reviewed-by: Jüri Valdmann <juri.valdmann@qt.io>

12 files changed, 59 insertions, 60 deletions

diff --git a/chromium/content/browser/browsing_data/conditional_cache_deletion_helper.cc b/chromium/content/browser/browsing_data/conditional_cache_deletion_helper.cc
index c74b8e14907..57763205a09 100644
--- a/chromium/content/browser/browsing_data/conditional_cache_deletion_helper.cc
+++ b/chromium/content/browser/browsing_data/conditional_cache_deletion_helper.cc
@@ -7,6 +7,7 @@
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/location.h"
+#include "base/sequence_checker.h"
 #include "base/single_thread_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "content/public/browser/browser_thread.h"
@@ -38,7 +39,6 @@ ConditionalCacheDeletionHelper::ConditionalCacheDeletionHelper(
       condition_(std::move(condition)),
       current_entry_(nullptr),
       previous_entry_(nullptr) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
 }
 
 // static
@@ -70,7 +70,7 @@ ConditionalCacheDeletionHelper::CreateCustomKeyURLAndTimeCondition(
 
 int ConditionalCacheDeletionHelper::DeleteAndDestroySelfWhenFinished(
     net::CompletionOnceCallback completion_callback) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   completion_callback_ = std::move(completion_callback);
   iterator_ = cache_->CreateIterator();
@@ -82,6 +82,7 @@ int ConditionalCacheDeletionHelper::DeleteAndDestroySelfWhenFinished(
 ConditionalCacheDeletionHelper::~ConditionalCacheDeletionHelper() {}
 
 void ConditionalCacheDeletionHelper::IterateOverEntries(int error) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   while (error != net::ERR_IO_PENDING) {
     // If the entry obtained in the previous iteration matches the condition,
     // mark it for deletion. The iterator is already one step forward, so it
diff --git a/chromium/content/browser/browsing_data/conditional_cache_deletion_helper.h b/chromium/content/browser/browsing_data/conditional_cache_deletion_helper.h
index 64fe71514f7..b98c3c2bc5e 100644
--- a/chromium/content/browser/browsing_data/conditional_cache_deletion_helper.h
+++ b/chromium/content/browser/browsing_data/conditional_cache_deletion_helper.h
@@ -25,7 +25,6 @@ namespace content {
 class CONTENT_EXPORT ConditionalCacheDeletionHelper {
  public:
   // Creates a helper to delete |cache| entries that match the |condition|.
-  // Must be created on the IO thread!
   ConditionalCacheDeletionHelper(
       disk_cache::Backend* cache,
       base::RepeatingCallback<bool(const disk_cache::Entry*)> condition);
@@ -56,7 +55,6 @@ class CONTENT_EXPORT ConditionalCacheDeletionHelper {
 
   // Deletes the cache entries according to the specified condition. Destroys
   // this instance of ConditionalCacheDeletionHelper when finished.
-  // Must be called on the IO thread!
   //
   // The return value is a net error code. If this method returns
   // ERR_IO_PENDING, the |completion_callback| will be invoked when the
@@ -75,6 +73,8 @@ class CONTENT_EXPORT ConditionalCacheDeletionHelper {
 
   net::CompletionOnceCallback completion_callback_;
 
+  SEQUENCE_CHECKER(sequence_checker_);
+
   std::unique_ptr<disk_cache::Backend::Iterator> iterator_;
   disk_cache::Entry* current_entry_;
   disk_cache::Entry* previous_entry_;
diff --git a/chromium/content/browser/browsing_data/storage_partition_code_cache_data_remover.cc b/chromium/content/browser/browsing_data/storage_partition_code_cache_data_remover.cc
index 973f8c1d7ab..df92e53f08f 100644
--- a/chromium/content/browser/browsing_data/storage_partition_code_cache_data_remover.cc
+++ b/chromium/content/browser/browsing_data/storage_partition_code_cache_data_remover.cc
@@ -55,16 +55,7 @@ void StoragePartitionCodeCacheDataRemover::Remove(
       << __func__ << " called with a null callback";
   done_callback_ = std::move(done_callback);
 
-  base::PostTaskWithTraits(
-      FROM_HERE, {BrowserThread::IO},
-      base::BindOnce(&StoragePartitionCodeCacheDataRemover::ClearJSCodeCache,
-                     base::Unretained(this)));
-}
-
-void StoragePartitionCodeCacheDataRemover::ClearedCodeCache() {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  std::move(done_callback_).Run();
-  base::ThreadTaskRunnerHandle::Get()->DeleteSoon(FROM_HERE, this);
+  ClearJSCodeCache();
 }
 
 void StoragePartitionCodeCacheDataRemover::ClearCache(
@@ -140,11 +131,9 @@ void StoragePartitionCodeCacheDataRemover::ClearWASMCodeCache(int rv) {
 // |rv| is the returned when clearing the code cache. We don't handle
 // any errors here, so the result value is ignored.
 void StoragePartitionCodeCacheDataRemover::DoneClearCodeCache(int rv) {
-  // Notify the UI thread that we are done.
-  base::PostTaskWithTraits(
-      FROM_HERE, {BrowserThread::UI},
-      base::BindOnce(&StoragePartitionCodeCacheDataRemover::ClearedCodeCache,
-                     base::Unretained(this)));
+  // Notify that we are done.
+  std::move(done_callback_).Run();
+  base::ThreadTaskRunnerHandle::Get()->DeleteSoon(FROM_HERE, this);
 }
 
 }  // namespace content
diff --git a/chromium/content/browser/browsing_data/storage_partition_code_cache_data_remover.h b/chromium/content/browser/browsing_data/storage_partition_code_cache_data_remover.h
index 989b596631f..113d31d5e5a 100644
--- a/chromium/content/browser/browsing_data/storage_partition_code_cache_data_remover.h
+++ b/chromium/content/browser/browsing_data/storage_partition_code_cache_data_remover.h
@@ -23,14 +23,7 @@ class StoragePartition;
 class GeneratedCodeCacheContext;
 
 // Helper to remove code cache data from a StoragePartition. This class is
-// created on the UI thread and calls the provided callback and destroys itself
-// on the UI thread after the code caches are cleared. This class also takes a
-// reference to the generated_code_cache_context and is used in read-only mode
-// on both the UI / IO thread. Since this isn't modified, it is OK to access it
-// on both threads. The caches are actually cleared on the IO threads. When the
-// Remove function is called, it posts tasks on the IO thread to clear the code
-// caches. Once the the caches are cleared, the callback is called on the UI
-// thread.
+// created on and acts on the UI thread.
 class StoragePartitionCodeCacheDataRemover {
  public:
   // Creates a StoragePartitionCodeCacheDataRemover that deletes all cache
@@ -64,10 +57,6 @@ class StoragePartitionCodeCacheDataRemover {
 
   ~StoragePartitionCodeCacheDataRemover();
 
-  // Executed on UI thread.
-  void ClearedCodeCache();
-
-  // Executed on IO thread.
   void ClearJSCodeCache();
   void ClearWASMCodeCache(int rv);
   void ClearCache(net::CompletionOnceCallback callback,
diff --git a/chromium/content/browser/code_cache/generated_code_cache_context.cc b/chromium/content/browser/code_cache/generated_code_cache_context.cc
index beb40b80224..738e8b95b59 100644
--- a/chromium/content/browser/code_cache/generated_code_cache_context.cc
+++ b/chromium/content/browser/code_cache/generated_code_cache_context.cc
@@ -20,14 +20,6 @@ void GeneratedCodeCacheContext::Initialize(const base::FilePath& path,
                                            int max_bytes) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  base::PostTaskWithTraits(
-      FROM_HERE, {BrowserThread::IO},
-      base::BindOnce(&GeneratedCodeCacheContext::InitializeOnIO, this, path,
-                     max_bytes));
-}
-
-void GeneratedCodeCacheContext::InitializeOnIO(const base::FilePath& path,
-                                               int max_bytes) {
   generated_js_code_cache_.reset(
       new GeneratedCodeCache(path.AppendASCII("js"), max_bytes,
                              GeneratedCodeCache::CodeCacheType::kJavaScript));
@@ -40,14 +32,20 @@ void GeneratedCodeCacheContext::InitializeOnIO(const base::FilePath& path,
   }
 }
 
+void GeneratedCodeCacheContext::Shutdown() {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  generated_js_code_cache_.reset();
+  generated_wasm_code_cache_.reset();
+}
+
 GeneratedCodeCache* GeneratedCodeCacheContext::generated_js_code_cache() const {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
   return generated_js_code_cache_.get();
 }
 
 GeneratedCodeCache* GeneratedCodeCacheContext::generated_wasm_code_cache()
     const {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
   return generated_wasm_code_cache_.get();
 }
 
diff --git a/chromium/content/browser/code_cache/generated_code_cache_context.h b/chromium/content/browser/code_cache/generated_code_cache_context.h
index 7e26e3269ee..b28d34b22f6 100644
--- a/chromium/content/browser/code_cache/generated_code_cache_context.h
+++ b/chromium/content/browser/code_cache/generated_code_cache_context.h
@@ -16,9 +16,9 @@ class GeneratedCodeCache;
 
 // One instance exists per disk-backed (non in-memory) storage contexts. This
 // owns the instance of GeneratedCodeCache that is used to store the data
-// generated by the renderer (for ex: code caches for script resources). This
-// initializes and closes the code cache on the I/O thread. The instance of
-// this class (|this|) itself is constructed on the UI thread.
+// generated by the renderer (for ex: code caches for script resources).
+// The instance of this class (|this|) is constructed and lives on the
+// UI thread. All methods must be called on UI thread.
 class CONTENT_EXPORT GeneratedCodeCacheContext
     : public base::RefCountedThreadSafe<GeneratedCodeCacheContext> {
  public:
@@ -30,7 +30,8 @@ class CONTENT_EXPORT GeneratedCodeCacheContext
   // being setup.
   void Initialize(const base::FilePath& path, int max_bytes);
 
-  // Call on the IO thread to get the code cache instances.
+  void Shutdown();
+
   GeneratedCodeCache* generated_js_code_cache() const;
   GeneratedCodeCache* generated_wasm_code_cache() const;
 
@@ -38,13 +39,9 @@ class CONTENT_EXPORT GeneratedCodeCacheContext
   friend class base::RefCountedThreadSafe<GeneratedCodeCacheContext>;
   ~GeneratedCodeCacheContext();
 
-  void InitializeOnIO(const base::FilePath& path, int max_bytes);
-
-  // Created, used and deleted on the IO thread.
-  std::unique_ptr<GeneratedCodeCache, BrowserThread::DeleteOnIOThread>
-      generated_js_code_cache_;
-  std::unique_ptr<GeneratedCodeCache, BrowserThread::DeleteOnIOThread>
-      generated_wasm_code_cache_;
+  // Created, used and deleted on the UI thread.
+  std::unique_ptr<GeneratedCodeCache> generated_js_code_cache_;
+  std::unique_ptr<GeneratedCodeCache> generated_wasm_code_cache_;
 
   DISALLOW_COPY_AND_ASSIGN(GeneratedCodeCacheContext);
 };
diff --git a/chromium/content/browser/renderer_host/code_cache_host_impl.cc b/chromium/content/browser/renderer_host/code_cache_host_impl.cc
index dac52c089cf..22d4b361eef 100644
--- a/chromium/content/browser/renderer_host/code_cache_host_impl.cc
+++ b/chromium/content/browser/renderer_host/code_cache_host_impl.cc
@@ -104,7 +104,7 @@ CodeCacheHostImpl::CodeCacheHostImpl(
       generated_code_cache_context_(std::move(generated_code_cache_context)) {}
 
 CodeCacheHostImpl::~CodeCacheHostImpl() {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
 }
 
 // static
@@ -131,7 +131,7 @@ void CodeCacheHostImpl::DidGenerateCacheableMetadata(
     return;
   }
 
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   GeneratedCodeCache* code_cache = GetCodeCache(cache_type);
   if (!code_cache)
diff --git a/chromium/content/browser/renderer_host/code_cache_host_impl.h b/chromium/content/browser/renderer_host/code_cache_host_impl.h
index bae531f274a..1f4a7f5401e 100644
--- a/chromium/content/browser/renderer_host/code_cache_host_impl.h
+++ b/chromium/content/browser/renderer_host/code_cache_host_impl.h
@@ -38,7 +38,7 @@ class GeneratedCodeCacheContext;
 // metadata, either bytecode or native code, generated by a renderer process.
 // Instances of this class are owned by by the Mojo pipe that's passed to the
 // renderer process via StrongBinding.
-// Instances of this class must be created and used on the IO thread.
+// Instances of this class must be created and used on the UI thread.
 class CONTENT_EXPORT CodeCacheHostImpl : public blink::mojom::CodeCacheHost {
  public:
   CodeCacheHostImpl(
diff --git a/chromium/content/browser/renderer_host/render_process_host_impl.cc b/chromium/content/browser/renderer_host/render_process_host_impl.cc
index c0b7e50a9a8..8dc869f6971 100644
--- a/chromium/content/browser/renderer_host/render_process_host_impl.cc
+++ b/chromium/content/browser/renderer_host/render_process_host_impl.cc
@@ -2159,11 +2159,13 @@ void RenderProcessHostImpl::RegisterMojoInterfaces() {
   registry->AddInterface(
       base::BindRepeating(&metrics::CreateSingleSampleMetricsProvider));
 
-  registry->AddInterface(base::BindRepeating(
-      &CodeCacheHostImpl::Create, GetID(),
-      base::RetainedRef(storage_partition_impl_->GetCacheStorageContext()),
-      base::RetainedRef(
-          storage_partition_impl_->GetGeneratedCodeCacheContext())));
+  AddUIThreadInterface(
+      registry.get(),
+      base::BindRepeating(
+          &CodeCacheHostImpl::Create, GetID(),
+          base::RetainedRef(storage_partition_impl_->GetCacheStorageContext()),
+          base::RetainedRef(
+              storage_partition_impl_->GetGeneratedCodeCacheContext())));
 
 #if BUILDFLAG(ENABLE_REPORTING)
   AddUIThreadInterface(
diff --git a/chromium/content/browser/storage_partition_impl.cc b/chromium/content/browser/storage_partition_impl.cc
index e218f7fec0d..bf5eb0bfc4e 100644
--- a/chromium/content/browser/storage_partition_impl.cc
+++ b/chromium/content/browser/storage_partition_impl.cc
@@ -33,6 +33,7 @@
 #include "content/browser/browsing_data/clear_site_data_handler.h"
 #include "content/browser/browsing_data/storage_partition_code_cache_data_remover.h"
 #include "content/browser/browsing_data/storage_partition_http_cache_data_remover.h"
+#include "content/browser/code_cache/generated_code_cache.h"
 #include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/code_cache/generated_code_cache_context.h"
 #include "content/browser/cookie_store/cookie_store_context.h"
@@ -833,6 +834,9 @@ StoragePartitionImpl::~StoragePartitionImpl() {
     }
   }
 
+  if (GetGeneratedCodeCacheContext())
+    GetGeneratedCodeCacheContext()->Shutdown();
+
   BrowserThread::DeleteSoon(BrowserThread::IO, FROM_HERE,
                             std::move(network_context_owner_));
 }
@@ -1712,6 +1716,21 @@ void StoragePartitionImpl::WaitForDeletionTasksForTesting() {
   }
 }
 
+void StoragePartitionImpl::WaitForCodeCacheShutdownForTesting() {
+  if (generated_code_cache_context_) {
+    // If this is still running its initialization task it may check
+    // enabled features on a sequenced worker pool which could race
+    // between ScopedFeatureList destruction.
+    base::RunLoop loop;
+    generated_code_cache_context_->generated_js_code_cache()->GetBackend(
+        base::BindOnce([](base::OnceClosure quit,
+                          disk_cache::Backend*) { std::move(quit).Run(); },
+                       loop.QuitClosure()));
+    loop.Run();
+    generated_code_cache_context_->Shutdown();
+  }
+}
+
 BrowserContext* StoragePartitionImpl::browser_context() const {
   return browser_context_;
 }
diff --git a/chromium/content/browser/storage_partition_impl.h b/chromium/content/browser/storage_partition_impl.h
index a633bfe2281..44a0d2d680e 100644
--- a/chromium/content/browser/storage_partition_impl.h
+++ b/chromium/content/browser/storage_partition_impl.h
@@ -163,6 +163,7 @@ class CONTENT_EXPORT StoragePartitionImpl
   void ClearBluetoothAllowedDevicesMapForTesting() override;
   void FlushNetworkInterfaceForTesting() override;
   void WaitForDeletionTasksForTesting() override;
+  void WaitForCodeCacheShutdownForTesting() override;
   BackgroundFetchContext* GetBackgroundFetchContext();
   PaymentAppContextImpl* GetPaymentAppContext();
   BroadcastChannelProvider* GetBroadcastChannelProvider();
diff --git a/chromium/content/public/browser/storage_partition.h b/chromium/content/public/browser/storage_partition.h
index fbbf13c77b4..f072135d05a 100644
--- a/chromium/content/public/browser/storage_partition.h
+++ b/chromium/content/public/browser/storage_partition.h
@@ -264,6 +264,9 @@ class CONTENT_EXPORT StoragePartition {
   // Wait until all deletions tasks are finished. For test use only.
   virtual void WaitForDeletionTasksForTesting() = 0;
 
+  // Wait until code cache's shutdown is complete. For test use only.
+  virtual void WaitForCodeCacheShutdownForTesting() = 0;
+
  protected:
   virtual ~StoragePartition() {}
 };