From 49844b6e5b1d293653eac06380ba73779d7ada62 Mon Sep 17 00:00:00 2001 From: Yury Solovyov Date: Mon, 8 Jan 2018 13:21:22 +0300 Subject: [PATCH] Move the IconLoader to use the task scheduler. This follows https://codereview.chromium.org/2953633002 --- chromium_src/chrome/browser/icon_loader.cc | 20 +++++++++---------- chromium_src/chrome/browser/icon_loader.h | 13 +++++++++--- .../chrome/browser/icon_loader_auralinux.cc | 7 ++++--- .../chrome/browser/icon_loader_mac.mm | 6 ++++-- .../chrome/browser/icon_loader_win.cc | 7 +++++-- 5 files changed, 33 insertions(+), 20 deletions(-) diff --git a/chromium_src/chrome/browser/icon_loader.cc b/chromium_src/chrome/browser/icon_loader.cc index a0cca6379d4c..91d80e78727d 100644 --- a/chromium_src/chrome/browser/icon_loader.cc +++ b/chromium_src/chrome/browser/icon_loader.cc @@ -2,8 +2,12 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include + #include "chrome/browser/icon_loader.h" #include "base/bind.h" +#include "base/task_scheduler/post_task.h" +#include "base/task_scheduler/task_traits.h" #include "base/threading/thread_task_runner_handle.h" #include "content/public/browser/browser_thread.h" @@ -18,10 +22,10 @@ IconLoader* IconLoader::Create(const base::FilePath& file_path, void IconLoader::Start() { target_task_runner_ = base::ThreadTaskRunnerHandle::Get(); - BrowserThread::PostTaskAndReply( - BrowserThread::FILE, FROM_HERE, - base::Bind(&IconLoader::ReadGroup, base::Unretained(this)), - base::Bind(&IconLoader::OnReadGroup, base::Unretained(this))); + + base::PostTaskWithTraits( + FROM_HERE, traits(), + base::BindOnce(&IconLoader::ReadGroup, base::Unretained(this))); } IconLoader::IconLoader(const base::FilePath& file_path, @@ -33,10 +37,6 @@ IconLoader::~IconLoader() {} void IconLoader::ReadGroup() { group_ = GroupForFilepath(file_path_); -} - -void IconLoader::OnReadGroup() { - BrowserThread::PostTask( - ReadIconThreadID(), FROM_HERE, - base::Bind(&IconLoader::ReadIcon, base::Unretained(this))); + GetReadIconTaskRunner()->PostTask( + FROM_HERE, base::BindOnce(&IconLoader::ReadIcon, base::Unretained(this))); } diff --git a/chromium_src/chrome/browser/icon_loader.h b/chromium_src/chrome/browser/icon_loader.h index 837be967356c..fc91909d86df 100644 --- a/chromium_src/chrome/browser/icon_loader.h +++ b/chromium_src/chrome/browser/icon_loader.h @@ -12,6 +12,7 @@ #include "base/files/file_path.h" #include "base/macros.h" #include "base/single_thread_task_runner.h" +#include "base/task_scheduler/task_traits.h" #include "build/build_config.h" #include "content/public/browser/browser_thread.h" #include "ui/gfx/image/image.h" @@ -66,13 +67,19 @@ class IconLoader { // Given a file path, get the group for the given file. static IconGroup GroupForFilepath(const base::FilePath& file_path); - // The thread ReadIcon() should be called on. - static content::BrowserThread::ID ReadIconThreadID(); + // The TaskRunner that ReadIcon() must be called on. + static scoped_refptr GetReadIconTaskRunner(); void ReadGroup(); - void OnReadGroup(); void ReadIcon(); + // The traits of the tasks posted by this class. These operations may block, + // because they are fetching icons from the disk, yet the result will be seen + // by the user so they should be prioritized accordingly. + static constexpr base::TaskTraits traits() { + return {base::MayBlock(), base::TaskPriority::USER_VISIBLE}; + } + // The task runner object of the thread in which we notify the delegate. scoped_refptr target_task_runner_; diff --git a/chromium_src/chrome/browser/icon_loader_auralinux.cc b/chromium_src/chrome/browser/icon_loader_auralinux.cc index 9f9187588657..484f6888c17c 100644 --- a/chromium_src/chrome/browser/icon_loader_auralinux.cc +++ b/chromium_src/chrome/browser/icon_loader_auralinux.cc @@ -17,10 +17,11 @@ IconLoader::IconGroup IconLoader::GroupForFilepath( } // static -content::BrowserThread::ID IconLoader::ReadIconThreadID() { +scoped_refptr IconLoader::GetReadIconTaskRunner() { // ReadIcon() calls into views::LinuxUI and GTK2 code, so it must be on the UI // thread. - return content::BrowserThread::UI; + return content::BrowserThread::GetTaskRunnerForThread( + content::BrowserThread::UI); } void IconLoader::ReadIcon() { @@ -50,6 +51,6 @@ void IconLoader::ReadIcon() { } target_task_runner_->PostTask( - FROM_HERE, base::Bind(callback_, base::Passed(&image), group_)); + FROM_HERE, base::BindOnce(callback_, base::Passed(&image), group_)); delete this; } diff --git a/chromium_src/chrome/browser/icon_loader_mac.mm b/chromium_src/chrome/browser/icon_loader_mac.mm index 392f2b22b253..f10b47d27a6e 100644 --- a/chromium_src/chrome/browser/icon_loader_mac.mm +++ b/chromium_src/chrome/browser/icon_loader_mac.mm @@ -11,6 +11,7 @@ #include "base/memory/ptr_util.h" #include "base/message_loop/message_loop.h" #include "base/strings/sys_string_conversions.h" +#include "base/task_scheduler/post_task.h" #include "base/threading/thread.h" #include "ui/gfx/image/image_skia.h" #include "ui/gfx/image/image_skia_util_mac.h" @@ -22,8 +23,9 @@ IconLoader::IconGroup IconLoader::GroupForFilepath( } // static -content::BrowserThread::ID IconLoader::ReadIconThreadID() { - return content::BrowserThread::FILE; +scoped_refptr IconLoader::GetReadIconTaskRunner() { + // NSWorkspace is thread-safe. + return base::CreateTaskRunnerWithTraits(traits()); } void IconLoader::ReadIcon() { diff --git a/chromium_src/chrome/browser/icon_loader_win.cc b/chromium_src/chrome/browser/icon_loader_win.cc index 035c759e42bf..1d0912bd5185 100644 --- a/chromium_src/chrome/browser/icon_loader_win.cc +++ b/chromium_src/chrome/browser/icon_loader_win.cc @@ -10,6 +10,7 @@ #include "base/bind.h" #include "base/memory/ptr_util.h" #include "base/message_loop/message_loop.h" +#include "base/task_scheduler/post_task.h" #include "base/threading/thread.h" #include "third_party/skia/include/core/SkBitmap.h" #include "ui/display/win/dpi.h" @@ -30,8 +31,10 @@ IconLoader::IconGroup IconLoader::GroupForFilepath( } // static -content::BrowserThread::ID IconLoader::ReadIconThreadID() { - return content::BrowserThread::FILE; +scoped_refptr IconLoader::GetReadIconTaskRunner() { + // Technically speaking, only a thread with COM is needed, not one that has + // a COM STA. However, this is what is available for now. + return base::CreateCOMSTATaskRunnerWithTraits(traits()); } void IconLoader::ReadIcon() {