Experiment: Schedule font unique name lookup table build at startup

Modify DWriteFontLookupTableBuilder to support off-main-thread
scheduling of the font unique name lookup table construction.

For now, schedule this as a USER_BLOCKING-prioritized startup task in a
separate SequencedTaskRunner. Synchronize using a WaitableEvent that the
DWriteFontProxyImpl can wait on when it receives an incoming Mojo
request from a renderer for this table.

Once the UpdatePriority() API for SequencedTaskRunners becomes
available, downgrade the priority to BEST_EFFORT, then bump it once the
renderer requests the table. This TODO is tracked in [1].

Background for font unique name local lookups in the design review doc
in https://crbug.com/828317. See also startup job scheduling discussion
in [2].

[1] https://crbug.com/931366
[2] https://groups.google.com/a/chromium.org/d/msg/chromium-dev/iDClDAsZnrE/bb5eZXkUFAAJ

Bug: 889864
Change-Id: I8e19d34520422fc9404ca01cb8357221d7a11495
Reviewed-on: https://chromium-review.googlesource.com/c/1459635
Commit-Queue: Dominik Röttsches <drott@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#632008}

base/threading/thread_restrictions.h

@@ -133,6 +133,7 @@ class BrowserShutdownProfileDumper;
 class BrowserTestBase;
 class CategorizedWorkerPool;
 class DesktopCaptureDevice;
+class DWriteFontLookupTableBuilder;
 class GpuProcessTransportFactory;
 class NestedMessagePumpAndroid;
 class SandboxHostLinux;
@@ -374,6 +375,7 @@ class BASE_EXPORT ScopedAllowBaseSyncPrimitives {
   friend class base::GetAppOutputScopedAllowBaseSyncPrimitives;
   friend class content::BrowserMainLoop;
   friend class content::BrowserProcessSubThread;
+  friend class content::DWriteFontLookupTableBuilder;
   friend class content::ServiceWorkerContextClient;
   friend class content::SessionStorageDatabase;
   friend class functions::ExecScriptScopedAllowBaseSyncPrimitives;

content/browser/browser_main_loop.cc

@@ -188,6 +188,7 @@
 #include <shellapi.h>
 
 #include "base/memory/memory_pressure_monitor_win.h"
+#include "content/browser/renderer_host/dwrite_font_lookup_table_builder_win.h"
 #include "net/base/winsock_init.h"
 #include "services/service_manager/sandbox/win/sandbox_win.h"
 #include "ui/display/win/screen_win.h"
@@ -1440,6 +1441,11 @@ int BrowserMainLoop::BrowserThreadsStarted() {
           switches::kDisableGpuProcessForDX12VulkanInfoCollection)) {
     GpuDataManagerImpl::GetInstance()->RequestGpuSupportedRuntimeVersion();
   }
+
+  if (base::FeatureList::IsEnabled(features::kFontSrcLocalMatching)) {
+    content::DWriteFontLookupTableBuilder::GetInstance()
+        ->ScheduleBuildFontUniqueNameTable();
+  }
 #endif
 
   if (MediaKeysListenerManager::IsMediaKeysListenerManagerEnabled()) {

content/browser/renderer_host/dwrite_font_lookup_table_builder_win.h

@@ -8,12 +8,18 @@
 #include <dwrite.h>
 #include <dwrite_2.h>
 #include <wrl.h>
+#include <memory>
+#include <string>
+#include <vector>
 
 #include "base/macros.h"
 #include "base/memory/read_only_shared_memory_region.h"
 #include "base/memory/singleton.h"
+#include "base/optional.h"
 #include "base/synchronization/waitable_event.h"
+#include "base/time/time.h"
 #include "content/common/content_export.h"
+#include "third_party/blink/public/common/font_unique_name_lookup/font_unique_name_table.pb.h"
 
 namespace base {
 template <typename T>
@@ -25,47 +31,92 @@ namespace content {
 // Singleton class which encapsulates building the font unique name table lookup
 // once, then serving the built table as a ReadOnlySharedMemoryRegion. Receives
 // requests for accessing this table from DWriteFontProxyImpl after Mojo IPC
-// calls from the renderer.
+// calls from the renderer. A method ScheduleBuildFontUniqueNameTable() is
+// provided to schedule building the font unique name lookup
+// structure. EnsureFontUniqueNameTable() can be called on any thread to wait
+// for the lookup table to be ready. After that, DuplicateMemoryRegion() can be
+// used to retrieve the lookup structure. Thread-safe when used as described
+// below.
 class CONTENT_EXPORT DWriteFontLookupTableBuilder {
  public:
   static DWriteFontLookupTableBuilder* GetInstance();
-  void SetSlowDownIndexingForTesting(bool);
-  // Needed to trigger rebuilding the lookup table, when testing using
-  // slowed-down indexing. Otherwise, the test methods would use the already
-  // cached lookup table.
-  void ResetLookupTableForTesting();
-
   // Retrieve the prepared memory region if it is available.
-  // EnsureFontUniqueNameTable() should be checked before. This method hits an
-  // assertion otherwise.
-  base::ReadOnlySharedMemoryRegion DuplicatedMemoryRegion();
+  // EnsureFontUniqueNameTable() must be checked before.
+  base::ReadOnlySharedMemoryRegion DuplicateMemoryRegion();
 
   // Wait for the internal WaitableEvent to be signaled if needed and return
-  // true if the font unique name lookup table was successfully constructed.
+  // true if the font unique name lookup table was successfully
+  // constructed. Call only after ScheduleBuildFontUniqueNameTable().
   bool EnsureFontUniqueNameTable();
 
+  // Posts a task to build the font unique name table index, should only be
+  // called once at browser startup, after that, use EnsureFontUniqueNameTable()
+  // and DuplicatedMemoryRegion() to retrieve the lookup structure buffer.
+  void ScheduleBuildFontUniqueNameTable();
+
+  enum class SlowDownMode { kDelayEachTask, kHangOneTask, kNoSlowdown };
+  // Slow down each family indexing step for testing the internal timeout.
+  void SetSlowDownIndexingForTesting(SlowDownMode slowdown_mode);
+  // Needed to trigger rebuilding the lookup table, when testing using
+  // slowed-down indexing. Otherwise, the test methods would use the already
+  // cached lookup table.
+  void ResetLookupTableForTesting();
+  // Signals hang_event_for_testing_ which is used in testing hanging one of the
+  // font name retrieval tasks.
+  void ResumeFromHangForTesting();
+
  private:
   friend class base::NoDestructor<DWriteFontLookupTableBuilder>;
 
+  struct FontFileWithUniqueNames {
+    FontFileWithUniqueNames(blink::FontUniqueNameTable_UniqueFont&& font,
+                            std::vector<std::string>&& names);
+    ~FontFileWithUniqueNames();
+    FontFileWithUniqueNames(
+        DWriteFontLookupTableBuilder::FontFileWithUniqueNames&& other);
+
+    FontFileWithUniqueNames(const FontFileWithUniqueNames&) = delete;
+    FontFileWithUniqueNames& operator=(const FontFileWithUniqueNames&) = delete;
+
+    blink::FontUniqueNameTable_UniqueFont font_entry;
+    std::vector<std::string> extracted_names;
+  };
+
+  using FamilyResult = std::vector<FontFileWithUniqueNames>;
+
   void BuildFontUniqueNameTable();
+  static FamilyResult ExtractPathAndNamesFromFamily(
+      Microsoft::WRL::ComPtr<IDWriteFontCollection> collection,
+      uint32_t family_index,
+      base::TimeTicks start_time,
+      SlowDownMode slow_down_mode,
+      base::WaitableEvent* hang_event_for_testing);
+  void AppendFamilyResultAndFinalizeIfNeeded(const FamilyResult& family_result);
+  void FinalizeFontTable();
+
+  void OnTimeout();
 
   bool IsFontUniqueNameTableValid();
 
-  // Checks if the unique font table has been built already, and if not, builds
-  // it by enumerating fonts from the collection, extracting their file
-  // locations, ttc indices and names.
   void InitializeDirectWrite();
+
   DWriteFontLookupTableBuilder();
   ~DWriteFontLookupTableBuilder();
 
-  // This can only be replaced from the construction sequence. Once
-  // font_table_built_ is signaled, it can be read from everywhere.
+  // Protobuf structure temporarily used and shared during table construction.
+  std::unique_ptr<blink::FontUniqueNameTable> font_unique_name_table_;
+
   base::MappedReadOnlyRegion font_table_memory_;
+  base::WaitableEvent font_table_built_;
 
   bool direct_write_initialized_ = false;
   Microsoft::WRL::ComPtr<IDWriteFontCollection> collection_;
   Microsoft::WRL::ComPtr<IDWriteFactory2> factory2_;
-  bool slow_down_indexing_for_testing_ = false;
+  SlowDownMode slow_down_mode_for_testing_ = SlowDownMode::kNoSlowdown;
+  uint32_t outstanding_family_results_ = 0;
+  base::TimeTicks start_time_;
+
+  base::Optional<base::WaitableEvent> hang_event_for_testing_;
 
   DISALLOW_COPY_AND_ASSIGN(DWriteFontLookupTableBuilder);
 };

content/browser/renderer_host/dwrite_font_lookup_table_builder_win_unittest.cc

@@ -9,7 +9,9 @@
 #include <vector>
 
 #include "base/files/file.h"
+#include "base/test/scoped_feature_list.h"
 #include "base/test/scoped_task_environment.h"
+#include "content/public/common/content_features.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/blink/public/common/font_unique_name_lookup/font_table_matcher.h"
 
@@ -25,57 +27,86 @@ std::vector<std::pair<std::string, uint32_t>> expected_test_fonts = {
 
 class DWriteFontLookupTableBuilderTest : public testing::Test {
  public:
-  DWriteFontLookupTableBuilderTest() = default;
+  DWriteFontLookupTableBuilderTest()
+      : scoped_task_environment_(
+            base::test::ScopedTaskEnvironment::ExecutionMode::ASYNC) {
+    feature_list_.InitAndEnableFeature(features::kFontSrcLocalMatching);
+  }
+
+  void SetUp() override {
+    font_lookup_table_builder_ = DWriteFontLookupTableBuilder::GetInstance();
+    font_lookup_table_builder_->ResetLookupTableForTesting();
+  }
+
+ protected:
+  DWriteFontLookupTableBuilder* font_lookup_table_builder_;
 
  private:
+  base::test::ScopedFeatureList feature_list_;
   base::test::ScopedTaskEnvironment scoped_task_environment_;
 };
 
+class DWriteFontLookupTableBuilderTimeoutTest
+    : public DWriteFontLookupTableBuilderTest,
+      public ::testing::WithParamInterface<
+          DWriteFontLookupTableBuilder::SlowDownMode> {};
+
 }  // namespace
 
 // Run a test similar to DWriteFontProxyImplUnitTest, TestFindUniqueFont but
 // without going through Mojo and running it on the DWRiteFontLookupTableBuilder
 // class directly.
 TEST_F(DWriteFontLookupTableBuilderTest, TestFindUniqueFontDirect) {
-  DWriteFontLookupTableBuilder* font_lookup_table_builder =
-      DWriteFontLookupTableBuilder::GetInstance();
-  font_lookup_table_builder->EnsureFontUniqueNameTable();
+  font_lookup_table_builder_->ScheduleBuildFontUniqueNameTable();
+  font_lookup_table_builder_->EnsureFontUniqueNameTable();
   base::ReadOnlySharedMemoryRegion font_table_memory =
-      font_lookup_table_builder->DuplicatedMemoryRegion();
+      font_lookup_table_builder_->DuplicateMemoryRegion();
   blink::FontTableMatcher font_table_matcher(font_table_memory.Map());
 
   for (auto& test_font_name_index : expected_test_fonts) {
     base::Optional<blink::FontTableMatcher::MatchResult> match_result =
         font_table_matcher.MatchName(test_font_name_index.first);
-    CHECK(match_result) << "No font matched for font name: "
-                        << test_font_name_index.first;
+    ASSERT_TRUE(match_result)
+        << "No font matched for font name: " << test_font_name_index.first;
     base::File unique_font_file(
         base::FilePath::FromUTF8Unsafe(match_result->font_path),
         base::File::FLAG_OPEN | base::File::FLAG_READ);
-    CHECK(unique_font_file.IsValid());
-    CHECK_GT(unique_font_file.GetLength(), 0);
-    CHECK_EQ(test_font_name_index.second, match_result->ttc_index);
+    ASSERT_TRUE(unique_font_file.IsValid());
+    ASSERT_GT(unique_font_file.GetLength(), 0);
+    ASSERT_EQ(test_font_name_index.second, match_result->ttc_index);
   }
 }
 
-TEST_F(DWriteFontLookupTableBuilderTest, TestTimeout) {
-  DWriteFontLookupTableBuilder* font_lookup_table_builder =
-      DWriteFontLookupTableBuilder::GetInstance();
-  font_lookup_table_builder->ResetLookupTableForTesting();
-  font_lookup_table_builder->SetSlowDownIndexingForTesting(true);
-  font_lookup_table_builder->EnsureFontUniqueNameTable();
+TEST_P(DWriteFontLookupTableBuilderTimeoutTest, TestTimeout) {
+  font_lookup_table_builder_->SetSlowDownIndexingForTesting(GetParam());
+  font_lookup_table_builder_->ScheduleBuildFontUniqueNameTable();
+  font_lookup_table_builder_->EnsureFontUniqueNameTable();
   base::ReadOnlySharedMemoryRegion font_table_memory =
-      font_lookup_table_builder->DuplicatedMemoryRegion();
+      font_lookup_table_builder_->DuplicateMemoryRegion();
   blink::FontTableMatcher font_table_matcher(font_table_memory.Map());
 
   for (auto& test_font_name_index : expected_test_fonts) {
     base::Optional<blink::FontTableMatcher::MatchResult> match_result =
         font_table_matcher.MatchName(test_font_name_index.first);
-    CHECK(!match_result);
+    ASSERT_TRUE(!match_result);
+  }
+  if (GetParam() == DWriteFontLookupTableBuilder::SlowDownMode::kHangOneTask)
+    font_lookup_table_builder_->ResumeFromHangForTesting();
+}
+
+INSTANTIATE_TEST_SUITE_P(
+    ,
+    DWriteFontLookupTableBuilderTimeoutTest,
+    ::testing::Values(
+        DWriteFontLookupTableBuilder::SlowDownMode::kDelayEachTask,
+        DWriteFontLookupTableBuilder::SlowDownMode::kHangOneTask));
+
+TEST_F(DWriteFontLookupTableBuilderTest, RepeatedScheduling) {
+  for (unsigned i = 0; i < 3; ++i) {
+    font_lookup_table_builder_->ResetLookupTableForTesting();
+    font_lookup_table_builder_->ScheduleBuildFontUniqueNameTable();
+    font_lookup_table_builder_->EnsureFontUniqueNameTable();
   }
-  // Need to reset the table again so that it can be rebuilt successfully
-  // without the artificial timeout when running the next test.
-  font_lookup_table_builder->ResetLookupTableForTesting();
 }
 
 }  // namespace content

content/browser/renderer_host/dwrite_font_proxy_impl_win.cc

@@ -27,6 +27,7 @@
 #include "base/time/time.h"
 #include "content/browser/renderer_host/dwrite_font_file_util_win.h"
 #include "content/browser/renderer_host/dwrite_font_uma_logging_win.h"
+#include "content/public/common/content_features.h"
 #include "mojo/public/cpp/bindings/callback_helpers.h"
 #include "mojo/public/cpp/bindings/strong_binding.h"
 #include "third_party/blink/public/common/font_unique_name_lookup/font_unique_name_table.pb.h"
@@ -393,15 +394,19 @@ void DWriteFontProxyImpl::MapCharacters(
 
 void DWriteFontProxyImpl::GetUniqueNameLookupTable(
     GetUniqueNameLookupTableCallback callback) {
+  DCHECK(base::FeatureList::IsEnabled(features::kFontSrcLocalMatching));
   InitializeDirectWrite();
   callback = mojo::WrapCallbackWithDefaultInvokeIfNotRun(
       std::move(callback), base::ReadOnlySharedMemoryRegion());
 
+  // ScheduleBuildFontUniqueNameTable() is called early in browser startup
+  // before EnsureFontUniqueNameTable() can be called. See
+  // BrowserMainLoop::BrowserThreadsStarted().
   if (!DWriteFontLookupTableBuilder::GetInstance()->EnsureFontUniqueNameTable())
     return;
 
   std::move(callback).Run(
-      DWriteFontLookupTableBuilder::GetInstance()->DuplicatedMemoryRegion());
+      DWriteFontLookupTableBuilder::GetInstance()->DuplicateMemoryRegion());
 }
 
 void DWriteFontProxyImpl::InitializeDirectWrite() {

content/browser/renderer_host/dwrite_font_proxy_impl_win_unittest.cc

@@ -12,9 +12,11 @@
 #include "base/files/file.h"
 #include "base/memory/ref_counted.h"
 #include "base/run_loop.h"
+#include "base/test/scoped_feature_list.h"
 #include "base/test/scoped_task_environment.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/win/windows_version.h"
+#include "content/public/common/content_features.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "mojo/public/cpp/bindings/binding.h"
 #include "services/service_manager/public/cpp/bind_source_info.h"
@@ -47,6 +49,20 @@ class DWriteFontProxyImplUnitTest : public testing::Test {
   mojo::Binding<blink::mojom::DWriteFontProxy> binding_;
 };
 
+class DWriteFontProxyUniqueNameMatchingTest
+    : public DWriteFontProxyImplUnitTest {
+ public:
+  DWriteFontProxyUniqueNameMatchingTest() {
+    feature_list_.InitAndEnableFeature(features::kFontSrcLocalMatching);
+    DWriteFontLookupTableBuilder::GetInstance()->ResetLookupTableForTesting();
+    DWriteFontLookupTableBuilder::GetInstance()
+        ->ScheduleBuildFontUniqueNameTable();
+  }
+
+ private:
+  base::test::ScopedFeatureList feature_list_;
+};
+
 TEST_F(DWriteFontProxyImplUnitTest, GetFamilyCount) {
   UINT32 family_count = 0;
   dwrite_font_proxy().GetFamilyCount(&family_count);
@@ -191,7 +207,7 @@ TEST_F(DWriteFontProxyImplUnitTest, TestCustomFontFiles) {
   }
 }
 
-TEST_F(DWriteFontProxyImplUnitTest, TestFindUniqueFont) {
+TEST_F(DWriteFontProxyUniqueNameMatchingTest, TestFindUniqueFont) {
   base::ReadOnlySharedMemoryRegion font_table_memory;
   dwrite_font_proxy().GetUniqueNameLookupTable(&font_table_memory);
   blink::FontTableMatcher font_table_matcher(font_table_memory.Map());
@@ -210,7 +226,6 @@ TEST_F(DWriteFontProxyImplUnitTest, TestFindUniqueFont) {
   }
 }
 
-
 }  // namespace
 
 }  // namespace content