Migrate jumplist update task from FILE thread to base/task_scheduler

chrome/browser/win/jumplist.cc

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/win/jumplist.h"
 
+#include <utility>
+
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/files/file_util.h"
 #include "base/macros.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/path_service.h"
-#include "base/sequenced_task_runner.h"
+#include "base/single_thread_task_runner.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/task_scheduler/post_task.h"
 #include "base/threading/thread.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/trace_event/trace_event.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/favicon/favicon_service_factory.h"
 #include "chrome/browser/history/top_sites_factory.h"
 #include "chrome/browser/metrics/jumplist_metrics_win.h"
@@ skipping 196 matching lines @@
     return false;
 
   // Commit this transaction and send the updated JumpList to Windows.
   if (!jumplist_updater.CommitUpdate())
     return false;
 
   return true;
 }
 
 // Updates the jumplist, once all the data has been fetched.
-void RunUpdateOnFileThread(
-    IncognitoModePrefs::Availability incognito_availability,
-    const std::wstring& app_id,
-    const base::FilePath& icon_dir,
-    base::RefCountedData<JumpListData>* ref_counted_data,
-    const scoped_refptr<base::SequencedTaskRunner>& sequenced_task_runner) {
+void RunUpdateJumpList(IncognitoModePrefs::Availability incognito_availability,
+                       const std::wstring& app_id,
+                       const base::FilePath& icon_dir,
+                       base::RefCountedData<JumpListData>* ref_counted_data) {
   JumpListData* data = &ref_counted_data->data;
   ShellLinkItemList local_most_visited_pages;
   ShellLinkItemList local_recently_closed_pages;
 
   {
     base::AutoLock auto_lock(data->list_lock_);
     // Make sure we are not out of date: if icon_urls_ is not empty, then
     // another notification has been received since we processed this one
     if (!data->icon_urls_.empty())
       return;
@@ skipping 34 matching lines @@
     // Create icon files for shortcuts in the "Recently Closed"
     // category.
     CreateIconFiles(icon_dir, local_recently_closed_pages);
   }
 
   // Create a new JumpList and replace the current JumpList with it. The
   // jumplist links are updated anyway, while the jumplist icons may not as
   // mentioned above.
   UpdateJumpList(app_id.c_str(), local_most_visited_pages,
                  local_recently_closed_pages, incognito_availability);
-
-  // Post a background task to delete JumpListIconsOld folder if it exists and
-  // log the delete results to UMA.
-  base::FilePath icon_dir_old = icon_dir.DirName().Append(
-      icon_dir.BaseName().value() + FILE_PATH_LITERAL("Old"));
-
-  if (base::DirectoryExists(icon_dir_old)) {
-    sequenced_task_runner->PostTask(
-        FROM_HERE, base::Bind(&DeleteDirectoryAndLogResults, icon_dir_old,
-                              kFileDeleteLimit));
-  }
 }
 
 }  // namespace
 
 JumpList::JumpListData::JumpListData() {}
 
 JumpList::JumpListData::~JumpListData() {}
 
 JumpList::JumpList(Profile* profile)
     : RefcountedKeyedService(content::BrowserThread::GetTaskRunnerForThread(
           content::BrowserThread::UI)),
       profile_(profile),
       jumplist_data_(new base::RefCountedData<JumpListData>),
       task_id_(base::CancelableTaskTracker::kBadTaskId),
-      sequenced_task_runner_(base::CreateSequencedTaskRunnerWithTraits(
+      single_thread_task_runner_(base::CreateCOMSTATaskRunnerWithTraits(
           base::TaskTraits()
               .WithPriority(base::TaskPriority::BACKGROUND)
               .WithShutdownBehavior(
                   base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN)
               .MayBlock())),
       weak_ptr_factory_(this) {
   DCHECK(Enabled());
   // To update JumpList when a tab is added or removed, we add this object to
   // the observer list of the TabRestoreService class.
   // When we add this object to the observer list, we save the pointer to this
@@ skipping 197 matching lines @@
     waiting_for_icons = !data->icon_urls_.empty();
     if (waiting_for_icons) {
       // Ask FaviconService if it has a favicon of a URL.
       // When FaviconService has one, it will call OnFaviconDataAvailable().
       url = GURL(data->icon_urls_.front().first);
     }
   }
 
   if (!waiting_for_icons) {
     // No more favicons are needed by the application JumpList. Schedule a
-    // RunUpdateOnFileThread call.
+    // RunUpdateJumpList call.
     PostRunUpdate();
     return;
   }
 
   favicon::FaviconService* favicon_service =
       FaviconServiceFactory::GetForProfile(profile_,
                                            ServiceAccessType::EXPLICIT_ACCESS);
   task_id_ = favicon_service->GetFaviconImageForPageURL(
       url,
       base::Bind(&JumpList::OnFaviconDataAvailable, base::Unretained(this)),
       &cancelable_task_tracker_);
 }
 
 void JumpList::OnFaviconDataAvailable(
     const favicon_base::FaviconImageResult& image_result) {
   DCHECK(CalledOnValidThread());
 
   // If there is currently a favicon request in progress, it is now outdated,
   // as we have received another, so nullify the handle from the old request.
   task_id_ = base::CancelableTaskTracker::kBadTaskId;
   // Lock the list to set icon data and pop the url.
   {
     JumpListData* data = &jumplist_data_->data;
     base::AutoLock auto_lock(data->list_lock_);
     // Attach the received data to the ShellLinkItem object.
-    // This data will be decoded by the RunUpdateOnFileThread method.
+    // This data will be decoded by the RunUpdateJumpList method.
     if (!image_result.image.IsEmpty() && !data->icon_urls_.empty() &&
         data->icon_urls_.front().second.get()) {
       gfx::ImageSkia image_skia = image_result.image.AsImageSkia();
       image_skia.EnsureRepsForSupportedScales();
       std::unique_ptr<gfx::ImageSkia> deep_copy(image_skia.DeepCopy());
       data->icon_urls_.front().second->set_icon_image(*deep_copy);
     }
 
     if (!data->icon_urls_.empty())
       data->icon_urls_.pop_front();
@@ skipping 37 matching lines @@
 
 void JumpList::DeferredRunUpdate() {
   DCHECK(CalledOnValidThread());
 
   TRACE_EVENT0("browser", "JumpList::DeferredRunUpdate");
   // Check if incognito windows (or normal windows) are disabled by policy.
   IncognitoModePrefs::Availability incognito_availability =
       profile_ ? IncognitoModePrefs::GetAvailability(profile_->GetPrefs())
                : IncognitoModePrefs::ENABLED;
 
-  BrowserThread::PostTask(
-      BrowserThread::FILE, FROM_HERE,
-      base::Bind(&RunUpdateOnFileThread, incognito_availability, app_id_,
-                 icon_dir_, base::RetainedRef(jumplist_data_),
-                 sequenced_task_runner_));
+  // Post a task to update the jumplist used by the shell.
+  single_thread_task_runner_->PostTask(

[robliao, 2017/04/05 00:27:50]

Is this expected to be called a lot? If not, you can grab the
CreateCOMSTATaskRunnerWithTraits here on demand and avoid persisting a
single_thread_task_runner_.

[chengx, 2017/04/05 03:25:50]

It will be called each time a Chrome tab is closed. I think that's a lot. 

[gab, 2017/04/05 20:53:27]

Yeah agreed that's a lot.

@robliao: I'm also seeing other single-threaded things that wouldn't mind
sharing a thread. Maybe we need to split our create methods to declare whether
you need exclusion or don't mind sharing...

[robliao, 2017/04/05 23:02:29]

Yeah. I'm pondering if the API should expose that kind of control to the caller
or not (e.g. will people know which one to choose).

We can discuss within the team.

+      FROM_HERE, base::Bind(&RunUpdateJumpList, incognito_availability, app_id_,
+                            icon_dir_, base::RetainedRef(jumplist_data_)));
+
+  // Post a task to delete JumpListIconsOld folder if it exists and log the
+  // delete results to UMA.
+  base::FilePath icon_dir_old = icon_dir_.DirName().Append(
+      icon_dir_.BaseName().value() + FILE_PATH_LITERAL("Old"));
+
+  single_thread_task_runner_->PostTask(
+      FROM_HERE, base::Bind(&DeleteDirectoryAndLogResults,
+                            std::move(icon_dir_old), kFileDeleteLimit));
 }
 
 void JumpList::TopSitesLoaded(history::TopSites* top_sites) {
 }
 
 void JumpList::TopSitesChanged(history::TopSites* top_sites,
                                ChangeReason change_reason) {
   top_sites->GetMostVisitedURLs(
       base::Bind(&JumpList::OnMostVisitedURLsAvailable,
                  weak_ptr_factory_.GetWeakPtr()),
       false);
 }