diff --git a/patches/chromium/.patches b/patches/chromium/.patches index 76a33962905d3..d16a6063d2181 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -100,4 +100,5 @@ move_readablestream_requests_onto_the_stack_before_iteration.patch streams_convert_state_dchecks_to_checks.patch audiocontext_haspendingactivity_unless_it_s_closed.patch protect_automatic_pull_handlers_with_mutex.patch +reland_sequentialise_access_to_callbacks_in.patch handle_err_cache_race_in_dodoneheadersaddtoentrycomplete.patch diff --git a/patches/chromium/reland_sequentialise_access_to_callbacks_in.patch b/patches/chromium/reland_sequentialise_access_to_callbacks_in.patch new file mode 100644 index 0000000000000..5f2da81e21621 --- /dev/null +++ b/patches/chromium/reland_sequentialise_access_to_callbacks_in.patch @@ -0,0 +1,216 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Dominik=20R=C3=B6ttsches?= +Date: Fri, 31 Jan 2020 21:40:11 +0000 +Subject: Reland: Sequentialise access to callbacks in + DWriteFontLookupTableBuilder +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +This reland fixes the sequenced task runner initialisation. Set the +task runner in the constructor and reset it for each unit test execution. + +Since there may be multiple instance of DWriteFontProxyImpl instantiated +for multiple RenderProcessHosts, and +DWriteFontProxyImpl::GetUniqueNameLookupTable may access +DWriteFontLookupTableBuilder::QueueShareMemoryRegionWhenReady from +separate threads, there may be race conditions around the +pending_callbacks_ member of DWriteFontLookupTableBuilder. + +Sequentialise and guard access to pending_callbacks_ with a separate +sequenced task runner. + +Remove explicit configuration of the FontUniqueNameBrowserTest cache +directory as [1] introduced a callback function in +ShellContentBrowserClient which sets this correctly. This avoids +instantiating DWriteFontLookupTableBuilder too early when the ThreadPool +is not yet available in a BrowserTest. + +[1] https://chromium-review.googlesource.com/c/chromium/src/+/1776358/9/content/shell/browser/shell_content_browser_client.cc#422 + +Fixed: 1047054 +Change-Id: I38cf8b84a48315980624b68bbf55d3727be457b8 +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2032119 +Reviewed-by: Matthew Denton +Commit-Queue: Dominik Röttsches +Cr-Commit-Position: refs/heads/master@{#737466} + +diff --git a/content/browser/font_unique_name_lookup/font_unique_name_browsertest.cc b/content/browser/font_unique_name_lookup/font_unique_name_browsertest.cc +index 18e63dfaec08f37e107215876f4a0511542148d8..8266e6fc1628310dafd497debbe1c6ba43aa02ca 100644 +--- a/content/browser/font_unique_name_lookup/font_unique_name_browsertest.cc ++++ b/content/browser/font_unique_name_lookup/font_unique_name_browsertest.cc +@@ -119,20 +119,10 @@ class FontUniqueNameBrowserTest : public DevToolsProtocolTest { + } + + #if defined(OS_WIN) +- // The Windows service for font unique name lookup needs a cache directory to +- // persist the cached information. Configure a temporary one before running +- // this test. +- void SetUpInProcessBrowserTestFixture() override { +- DevToolsProtocolTest::SetUpInProcessBrowserTestFixture(); +- DWriteFontLookupTableBuilder* table_builder = +- DWriteFontLookupTableBuilder::GetInstance(); +- ASSERT_TRUE(cache_directory_.CreateUniqueTempDir()); +- table_builder->SetCacheDirectoryForTesting(cache_directory_.GetPath()); +- } +- + void PreRunTestOnMainThread() override { + DWriteFontLookupTableBuilder* table_builder = + DWriteFontLookupTableBuilder::GetInstance(); ++ table_builder->ResetStateForTesting(); + table_builder->SchedulePrepareFontUniqueNameTableIfNeeded(); + DevToolsProtocolTest::PreRunTestOnMainThread(); + } +diff --git a/content/browser/renderer_host/dwrite_font_lookup_table_builder_win.cc b/content/browser/renderer_host/dwrite_font_lookup_table_builder_win.cc +index f8ba10769691864f59ea6f2ef55700d0cd9c4969..f151db4394e1b77f8ffbda6851af586943c8da3a 100644 +--- a/content/browser/renderer_host/dwrite_font_lookup_table_builder_win.cc ++++ b/content/browser/renderer_host/dwrite_font_lookup_table_builder_win.cc +@@ -116,16 +116,30 @@ DWriteFontLookupTableBuilder::FontFileWithUniqueNames::FontFileWithUniqueNames( + + DWriteFontLookupTableBuilder::DWriteFontLookupTableBuilder() + : font_indexing_timeout_(kFontIndexingTimeoutDefault) { ++ ResetCallbacksAccessTaskRunner(); + InitializeCacheDirectoryFromProfile(); + } + ++void DWriteFontLookupTableBuilder::ResetCallbacksAccessTaskRunner() { ++ callbacks_access_task_runner_ = base::CreateSequencedTaskRunner({ ++ base::ThreadPool(), ++ base::TaskPriority::USER_VISIBLE, ++#if DCHECK_IS_ON() ++ // Needed for DCHECK in DuplicateMemoryRegion() which performs file ++ // operations to detect cache directory. ++ base::MayBlock(), ++#endif ++ base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN ++ }); ++ DETACH_FROM_SEQUENCE(callbacks_access_sequence_checker_); ++} ++ + void DWriteFontLookupTableBuilder::InitializeCacheDirectoryFromProfile() { +- // In FontUniqueNameBrowserTest the DWriteFontLookupTableBuilder is +- // instantiated to configure the cache directory for testing explicitly before +- // GetContentClient() is available. Catch this case here. It is safe to not +- // set the cache directory here, as an invalid cache directory would be +- // detected by TableCacheFilePath and the LoadFromFile and PersistToFile +- // methods. ++ // Unit tests that do not launch a full browser environment usually don't need ++ // testing of src:local()-style font matching. Check that an environment is ++ // present here and configcure the cache directory based on that. If none is ++ // configured, catch this in DuplicateMemoryRegion(), i.e. when a client ++ // tries to use this API. + cache_directory_ = + GetContentClient() && GetContentClient()->browser() + ? GetContentClient()->browser()->GetFontLookupTableCacheDir() +@@ -237,7 +251,8 @@ base::TimeDelta DWriteFontLookupTableBuilder::IndexingTimeout() { + return font_indexing_timeout_; + } + +-void DWriteFontLookupTableBuilder::PostCallbacks() { ++void DWriteFontLookupTableBuilder::PostCallbacksImpl() { ++ DCHECK_CALLED_ON_VALID_SEQUENCE(callbacks_access_sequence_checker_); + for (auto& pending_callback : pending_callbacks_) { + pending_callback.task_runner->PostTask( + FROM_HERE, base::BindOnce(std::move(pending_callback.mojo_callback), +@@ -246,6 +261,13 @@ void DWriteFontLookupTableBuilder::PostCallbacks() { + pending_callbacks_.clear(); + } + ++void DWriteFontLookupTableBuilder::PostCallbacks() { ++ callbacks_access_task_runner_->PostTask( ++ FROM_HERE, ++ base::BindOnce(&DWriteFontLookupTableBuilder::PostCallbacksImpl, ++ base::Unretained(this))); ++} ++ + base::FilePath DWriteFontLookupTableBuilder::TableCacheFilePath() { + if (!EnsureCacheDirectory(cache_directory_)) + return base::FilePath(); +@@ -287,18 +309,38 @@ DWriteFontLookupTableBuilder::CallbackOnTaskRunner::CallbackOnTaskRunner( + DWriteFontLookupTableBuilder::CallbackOnTaskRunner::~CallbackOnTaskRunner() = + default; + ++void DWriteFontLookupTableBuilder::QueueShareMemoryRegionWhenReadyImpl( ++ scoped_refptr task_runner, ++ blink::mojom::DWriteFontProxy::GetUniqueNameLookupTableCallback callback) { ++ DCHECK_CALLED_ON_VALID_SEQUENCE(callbacks_access_sequence_checker_); ++ ++ // Don't queue but post response directly if the table is already ready for ++ // sharing with renderers to cover the condition in which the font table ++ // becomes ready briefly after a renderer asking for ++ // GetUniqueNameLookupTableIfAvailable(), receiving the information that it ++ // wasn't ready. (https://crbug.com/977283) ++ if (font_table_built_.IsSignaled()) { ++ task_runner->PostTask(FROM_HERE, base::BindOnce(std::move(callback), ++ DuplicateMemoryRegion())); ++ return; ++ } ++ ++ pending_callbacks_.push_back( ++ CallbackOnTaskRunner(std::move(task_runner), std::move(callback))); ++} ++ + void DWriteFontLookupTableBuilder::QueueShareMemoryRegionWhenReady( + scoped_refptr task_runner, + blink::mojom::DWriteFontProxy::GetUniqueNameLookupTableCallback callback) { + TRACE_EVENT0("dwrite,fonts", + "DWriteFontLookupTableBuilder::QueueShareMemoryRegionWhenReady"); + DCHECK(!HasDWriteUniqueFontLookups()); +- pending_callbacks_.emplace_back(std::move(task_runner), std::move(callback)); +- // Cover for the condition in which the font table becomes ready briefly after +- // a renderer asking for GetUniqueNameLookupTableIfAvailable(), receiving the +- // information that it wasn't ready. +- if (font_table_built_.IsSignaled()) +- PostCallbacks(); ++ CHECK(callbacks_access_task_runner_); ++ callbacks_access_task_runner_->PostTask( ++ FROM_HERE, ++ base::BindOnce( ++ &DWriteFontLookupTableBuilder::QueueShareMemoryRegionWhenReadyImpl, ++ base::Unretained(this), std::move(task_runner), std::move(callback))); + } + + bool DWriteFontLookupTableBuilder::FontUniqueNameTableReady() { +@@ -665,6 +707,7 @@ void DWriteFontLookupTableBuilder::ResetLookupTableForTesting() { + font_indexing_timeout_ = kFontIndexingTimeoutDefault; + font_table_memory_ = base::MappedReadOnlyRegion(); + caching_enabled_ = true; ++ ResetCallbacksAccessTaskRunner(); + font_table_built_.Reset(); + } + +diff --git a/content/browser/renderer_host/dwrite_font_lookup_table_builder_win.h b/content/browser/renderer_host/dwrite_font_lookup_table_builder_win.h +index 4d23f8ac3dada73b486b19bdfd033824140e10f6..3d19c8a8d36a9e263ff9373f489d38e7489368ff 100644 +--- a/content/browser/renderer_host/dwrite_font_lookup_table_builder_win.h ++++ b/content/browser/renderer_host/dwrite_font_lookup_table_builder_win.h +@@ -168,6 +168,21 @@ class CONTENT_EXPORT DWriteFontLookupTableBuilder { + // constructed protobuf to disk. + void FinalizeFontTable(); + ++ // Internal implementation of adding a callback request to the list in order ++ // to sequentialise access to pending_callbacks_. ++ void QueueShareMemoryRegionWhenReadyImpl( ++ scoped_refptr task_runner, ++ blink::mojom::DWriteFontProxy::GetUniqueNameLookupTableCallback callback); ++ ++ // Internal implementation of posting the callbacks, running on the sequence ++ // that sequentialises access to pending_callbacks_. ++ void PostCallbacksImpl(); ++ ++ // Resets the internal task runner guarding access to pending_callbacks_, used ++ // in unit tests, as the TaskEnvironment used in tests tears down and resets ++ // the ThreadPool between tests, and the TaskRunner depends on it. ++ void ResetCallbacksAccessTaskRunner(); ++ + void OnTimeout(); + + bool IsFontUniqueNameTableValid(); +@@ -221,6 +236,8 @@ class CONTENT_EXPORT DWriteFontLookupTableBuilder { + }; + + std::vector pending_callbacks_; ++ scoped_refptr callbacks_access_task_runner_; ++ SEQUENCE_CHECKER(callbacks_access_sequence_checker_); + + DISALLOW_COPY_AND_ASSIGN(DWriteFontLookupTableBuilder); + };