diff options
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() {} }; |