Migration of font thread to sequence tasks runner

This CL is removing the thread and using a task runner instead.

I propose to continue this migration by renaming the thread
service since it's no longer a thread.

R=fdoray@chromium.org, gab@chromium.org
BUG=936569

Change-Id: If65dfd41406d13c9f69036d71e75242ef29f9b1d
Reviewed-on: https://chromium-review.googlesource.com/c/1492369Reviewed-by: François Doray <fdoray@chromium.org>
Reviewed-by: Dominik Röttsches <drott@chromium.org>
Commit-Queue: Etienne Bergeron <etienneb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#636527}

components/services/font/public/cpp/font_loader.cc

@@ -22,11 +22,6 @@ FontLoader::FontLoader(service_manager::Connector* connector) {
 
 FontLoader::~FontLoader() {}
 
-void FontLoader::Shutdown() {
-  thread_->Stop();
-  thread_ = nullptr;
-}
-
 bool FontLoader::matchFamilyName(const char family_name[],
                                  SkFontStyle requested,
                                  FontIdentity* out_font_identifier,

components/services/font/public/cpp/font_loader.h

@@ -37,9 +37,6 @@ class FontLoader : public SkFontConfigInterface,
   explicit FontLoader(service_manager::Connector* connector);
   ~FontLoader() override;
 
-  // Shuts down the background thread.
-  void Shutdown();
-
   // SkFontConfigInterface:
   bool matchFamilyName(const char family_name[],
                        SkFontStyle requested,

components/services/font/public/cpp/font_service_thread.cc

@@ -9,36 +9,35 @@
 #include "base/bind.h"
 #include "base/files/file.h"
 #include "base/synchronization/waitable_event.h"
+#include "base/task/post_task.h"
 #include "components/services/font/public/cpp/mapped_font_file.h"
 
 namespace font_service {
 namespace internal {
 
-namespace {
-const char kFontThreadName[] = "Font_Proxy_Thread";
-}  // namespace
-
 FontServiceThread::FontServiceThread(mojom::FontServicePtr font_service)
-    : base::Thread(kFontThreadName),
-      font_service_info_(font_service.PassInterface()),
+    : font_service_info_(font_service.PassInterface()),
+      task_runner_(base::CreateSequencedTaskRunnerWithTraits(
+          {base::TaskPriority::USER_VISIBLE, base::MayBlock()})),
       weak_factory_(this) {
-  DETACH_FROM_THREAD(thread_checker_);
-  Start();
+  task_runner_->PostTask(FROM_HERE, base::BindOnce(&FontServiceThread::Init,
+                                                   weak_factory_.GetWeakPtr()));
 }
 
+FontServiceThread::~FontServiceThread() {}
+
 bool FontServiceThread::MatchFamilyName(
     const char family_name[],
     SkFontStyle requested_style,
     SkFontConfigInterface::FontIdentity* out_font_identity,
     SkString* out_family_name,
     SkFontStyle* out_style) {
-  DCHECK_NE(GetThreadId(), base::PlatformThread::CurrentId());
-
+  DCHECK(!task_runner_->RunsTasksInCurrentSequence());
   bool out_valid = false;
   // This proxies to the other thread, which proxies to mojo. Only on the reply
   // from mojo do we return from this.
   base::WaitableEvent done_event;
-  task_runner()->PostTask(
+  task_runner_->PostTask(
       FROM_HERE,
       base::BindOnce(&FontServiceThread::MatchFamilyNameImpl, this, &done_event,
                      family_name, requested_style, &out_valid,
@@ -55,10 +54,10 @@ bool FontServiceThread::FallbackFontForCharacter(
     std::string* out_family_name,
     bool* out_is_bold,
     bool* out_is_italic) {
-  DCHECK_NE(GetThreadId(), base::PlatformThread::CurrentId());
+  DCHECK(!task_runner_->RunsTasksInCurrentSequence());
   bool out_valid = false;
   base::WaitableEvent done_event;
-  task_runner()->PostTask(
+  task_runner_->PostTask(
       FROM_HERE,
       base::BindOnce(&FontServiceThread::FallbackFontForCharacterImpl, this,
                      &done_event, character, std::move(locale), &out_valid,
@@ -76,10 +75,10 @@ bool FontServiceThread::FontRenderStyleForStrike(
     bool is_bold,
     float device_scale_factor,
     font_service::mojom::FontRenderStylePtr* out_font_render_style) {
-  DCHECK_NE(GetThreadId(), base::PlatformThread::CurrentId());
+  DCHECK(!task_runner_->RunsTasksInCurrentSequence());
   bool out_valid = false;
   base::WaitableEvent done_event;
-  task_runner()->PostTask(
+  task_runner_->PostTask(
       FROM_HERE,
       base::BindOnce(&FontServiceThread::FontRenderStyleForStrikeImpl, this,
                      &done_event, family, size, is_italic, is_bold,
@@ -91,10 +90,10 @@ bool FontServiceThread::FontRenderStyleForStrike(
 bool FontServiceThread::MatchFontByPostscriptNameOrFullFontName(
     std::string postscript_name_or_full_font_name,
     mojom::FontIdentityPtr* out_identity) {
-  DCHECK_NE(GetThreadId(), base::PlatformThread::CurrentId());
+  DCHECK(!task_runner_->RunsTasksInCurrentSequence());
   bool out_valid = false;
   base::WaitableEvent done_event;
-  task_runner()->PostTask(
+  task_runner_->PostTask(
       FROM_HERE,
       base::BindOnce(
           &FontServiceThread::MatchFontByPostscriptNameOrFullFontNameImpl, this,
@@ -111,9 +110,9 @@ void FontServiceThread::MatchFontWithFallback(
     uint32_t charset,
     uint32_t fallback_family_type,
     base::File* out_font_file_handle) {
-  DCHECK_NE(GetThreadId(), base::PlatformThread::CurrentId());
+  DCHECK(!task_runner_->RunsTasksInCurrentSequence());
   base::WaitableEvent done_event;
-  task_runner()->PostTask(
+  task_runner_->PostTask(
       FROM_HERE,
       base::BindOnce(&FontServiceThread::MatchFontWithFallbackImpl, this,
                      &done_event, std::move(family), is_bold, is_italic,
@@ -123,13 +122,13 @@ void FontServiceThread::MatchFontWithFallback(
 
 scoped_refptr<MappedFontFile> FontServiceThread::OpenStream(
     const SkFontConfigInterface::FontIdentity& identity) {
-  DCHECK_NE(GetThreadId(), base::PlatformThread::CurrentId());
+  DCHECK(!task_runner_->RunsTasksInCurrentSequence());
 
   base::File stream_file;
   // This proxies to the other thread, which proxies to mojo. Only on the
   // reply from mojo do we return from this.
   base::WaitableEvent done_event;
-  task_runner()->PostTask(
+  task_runner_->PostTask(
       FROM_HERE, base::BindOnce(&FontServiceThread::OpenStreamImpl, this,
                                 &done_event, &stream_file, identity.fID));
   done_event.Wait();
@@ -148,10 +147,6 @@ scoped_refptr<MappedFontFile> FontServiceThread::OpenStream(
   return mapped_font_file;
 }
 
-FontServiceThread::~FontServiceThread() {
-  Stop();
-}
-
 void FontServiceThread::MatchFamilyNameImpl(
     base::WaitableEvent* done_event,
     const char family_name[],
@@ -160,7 +155,7 @@ void FontServiceThread::MatchFamilyNameImpl(
     SkFontConfigInterface::FontIdentity* out_font_identity,
     SkString* out_family_name,
     SkFontStyle* out_style) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  DCHECK(task_runner_->RunsTasksInCurrentSequence());
 
   if (font_service_.encountered_error()) {
     *out_valid = false;
@@ -192,7 +187,8 @@ void FontServiceThread::OnMatchFamilyNameComplete(
     mojom::FontIdentityPtr font_identity,
     const std::string& family_name,
     mojom::TypefaceStylePtr style) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  DCHECK(task_runner_->RunsTasksInCurrentSequence());
+
   pending_waitable_events_.erase(done_event);
 
   *out_valid = !font_identity.is_null();
@@ -214,7 +210,8 @@ void FontServiceThread::OnMatchFamilyNameComplete(
 void FontServiceThread::OpenStreamImpl(base::WaitableEvent* done_event,
                                        base::File* output_file,
                                        const uint32_t id_number) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  DCHECK(task_runner_->RunsTasksInCurrentSequence());
+
   if (font_service_.encountered_error()) {
     done_event->Signal();
     return;
@@ -229,7 +226,8 @@ void FontServiceThread::OpenStreamImpl(base::WaitableEvent* done_event,
 void FontServiceThread::OnOpenStreamComplete(base::WaitableEvent* done_event,
                                              base::File* output_file,
                                              base::File file) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  DCHECK(task_runner_->RunsTasksInCurrentSequence());
+
   pending_waitable_events_.erase(done_event);
   *output_file = std::move(file);
   done_event->Signal();
@@ -244,7 +242,7 @@ void FontServiceThread::FallbackFontForCharacterImpl(
     std::string* out_family_name,
     bool* out_is_bold,
     bool* out_is_italic) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  DCHECK(task_runner_->RunsTasksInCurrentSequence());
 
   if (font_service_.encountered_error()) {
     *out_valid = false;
@@ -271,7 +269,8 @@ void FontServiceThread::OnFallbackFontForCharacterComplete(
     const std::string& family_name,
     bool is_bold,
     bool is_italic) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  DCHECK(task_runner_->RunsTasksInCurrentSequence());
+
   pending_waitable_events_.erase(done_event);
 
   *out_valid = !font_identity.is_null();
@@ -293,7 +292,7 @@ void FontServiceThread::FontRenderStyleForStrikeImpl(
     float device_scale_factor,
     bool* out_valid,
     mojom::FontRenderStylePtr* out_font_render_style) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  DCHECK(task_runner_->RunsTasksInCurrentSequence());
 
   if (font_service_.encountered_error()) {
     *out_valid = false;
@@ -313,7 +312,8 @@ void FontServiceThread::OnFontRenderStyleForStrikeComplete(
     bool* out_valid,
     mojom::FontRenderStylePtr* out_font_render_style,
     mojom::FontRenderStylePtr font_render_style) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  DCHECK(task_runner_->RunsTasksInCurrentSequence());
+
   pending_waitable_events_.erase(done_event);
 
   *out_valid = !font_render_style.is_null();
@@ -328,7 +328,7 @@ void FontServiceThread::MatchFontByPostscriptNameOrFullFontNameImpl(
     bool* out_valid,
     std::string postscript_name_or_full_font_name,
     mojom::FontIdentityPtr* out_font_identity) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  DCHECK(task_runner_->RunsTasksInCurrentSequence());
 
   if (font_service_.encountered_error()) {
     *out_valid = false;
@@ -349,7 +349,8 @@ void FontServiceThread::OnMatchFontByPostscriptNameOrFullFontNameComplete(
     bool* out_valid,
     mojom::FontIdentityPtr* out_font_identity,
     mojom::FontIdentityPtr font_identity) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  DCHECK(task_runner_->RunsTasksInCurrentSequence());
+
   pending_waitable_events_.erase(done_event);
 
   *out_valid = !font_identity.is_null();
@@ -367,7 +368,8 @@ void FontServiceThread::MatchFontWithFallbackImpl(
     uint32_t charset,
     uint32_t fallback_family_type,
     base::File* out_font_file_handle) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  DCHECK(task_runner_->RunsTasksInCurrentSequence());
+
   *out_font_file_handle = base::File();
   if (font_service_.encountered_error()) {
     done_event->Signal();
@@ -384,7 +386,8 @@ void FontServiceThread::OnMatchFontWithFallbackComplete(
     base::WaitableEvent* done_event,
     base::File* out_font_file_handle,
     base::File file) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  DCHECK(task_runner_->RunsTasksInCurrentSequence());
+
   pending_waitable_events_.erase(done_event);
 
   *out_font_file_handle = std::move(file);
@@ -405,9 +408,5 @@ void FontServiceThread::Init() {
                      weak_factory_.GetWeakPtr()));
 }
 
-void FontServiceThread::CleanUp() {
-  font_service_.reset();
-}
-
 }  // namespace internal
 }  // namespace font_service

components/services/font/public/cpp/font_service_thread.h

@@ -12,8 +12,6 @@
 #include "base/files/file.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
-#include "base/threading/thread.h"
-#include "base/threading/thread_checker.h"
 #include "components/services/font/public/interfaces/font_service.mojom.h"
 #include "third_party/skia/include/core/SkStream.h"
 #include "third_party/skia/include/core/SkTypeface.h"
@@ -31,8 +29,8 @@ class MappedFontFile;
 // of this mismatch, we create a thread which owns the mojo pipe, sends and
 // receives messages. The multiple threads which call through FontLoader class
 // do blocking message calls to this thread.
-class FontServiceThread : public base::Thread,
-                          public base::RefCountedThreadSafe<FontServiceThread> {
+// TODO(936569): Rename FontServiceThread since it's no longer a thread.
+class FontServiceThread : public base::RefCountedThreadSafe<FontServiceThread> {
  public:
   explicit FontServiceThread(mojom::FontServicePtr font_service);
 
@@ -72,7 +70,9 @@ class FontServiceThread : public base::Thread,
 
  private:
   friend class base::RefCountedThreadSafe<FontServiceThread>;
-  ~FontServiceThread() override;
+  virtual ~FontServiceThread();
+
+  void Init();
 
   // Methods which run on the FontServiceThread. The public MatchFamilyName
   // calls this method, this method calls the mojo interface, and sets up the
@@ -169,17 +169,13 @@ class FontServiceThread : public base::Thread,
   // thread.
   void OnFontServiceConnectionError();
 
-  // base::Thread
-  void Init() override;
-  void CleanUp() override;
-
   // This member is used to safely pass data from one thread to another. It is
   // set in the constructor and is consumed in Init().
-  mojo::InterfacePtrInfo<mojom::FontService> font_service_info_;
+  mojom::FontServicePtrInfo font_service_info_;
 
   // This member is set in Init(). It takes |font_service_info_|, which is
   // non-thread bound, and binds it to the newly created thread.
-  mojo::InterfacePtr<mojom::FontService> font_service_;
+  mojom::FontServicePtr font_service_;
 
   // All WaitableEvents supplied to OpenStreamImpl() and the other *Impl()
   // functions are added here while waiting on the response from the
@@ -190,8 +186,7 @@ class FontServiceThread : public base::Thread,
   // never received.
   std::set<base::WaitableEvent*> pending_waitable_events_;
 
-  THREAD_CHECKER(thread_checker_);
-
+  const scoped_refptr<base::SequencedTaskRunner> task_runner_;
   base::WeakPtrFactory<FontServiceThread> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(FontServiceThread);