Remove usage of SequencedWorkerPool::GetNamedSequenceToken from GetKeyboardLayoutsForLocale.

chrome/browser/ui/webui/chromeos/login/l10n_util.cc

 // Copyright 2014 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/ui/webui/chromeos/login/l10n_util.h"
 
 #include <stddef.h>
 
 #include <iterator>
 #include <map>
 #include <memory>
 #include <set>
 #include <utility>
 
 #include "base/bind.h"
 #include "base/i18n/rtl.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/sequenced_task_runner.h"
 #include "base/stl_util.h"
 #include "base/strings/string16.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/task_runner_util.h"
-#include "base/threading/sequenced_worker_pool.h"
-#include "base/threading/thread_task_runner_handle.h"
+#include "base/task_scheduler/post_task.h"
+#include "base/threading/sequenced_task_runner_handle.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/customization/customization_document.h"
 #include "chrome/browser/chromeos/input_method/input_method_util.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/grit/generated_resources.h"
 #include "content/public/browser/browser_thread.h"
 #include "ui/base/ime/chromeos/component_extension_ime_manager.h"
 #include "ui/base/ime/chromeos/input_method_descriptor.h"
 #include "ui/base/ime/chromeos/input_method_manager.h"
 #include "ui/base/l10n/l10n_util.h"
 
 namespace chromeos {
 
 const char kMostRelevantLanguagesDivider[] = "MOST_RELEVANT_LANGUAGES_DIVIDER";
 
 namespace {
 
-const char kSequenceToken[] = "chromeos_login_l10n_util";
-
 std::unique_ptr<base::DictionaryValue> CreateInputMethodsEntry(
     const input_method::InputMethodDescriptor& method,
     const std::string selected) {
   input_method::InputMethodUtil* util =
       input_method::InputMethodManager::Get()->GetInputMethodUtil();
   const std::string& ime_id = method.id();
   std::unique_ptr<base::DictionaryValue> input_method(
       new base::DictionaryValue);
   input_method->SetString("value", ime_id);
   input_method->SetString(
@@ skipping 279 matching lines @@
   std::string resolved_locale;
   if (!l10n_util::CheckAndResolveLocale(requested_locale, &resolved_locale))
     return loaded_locale;
 
   if (resolved_locale == loaded_locale)
     return requested_locale;
 
   return loaded_locale;
 }
 
-void ResolveLanguageListOnBlockingPool(
+void ResolveLanguageListInThreadPool(
     std::unique_ptr<chromeos::locale_util::LanguageSwitchResult>
         language_switch_result,
     const scoped_refptr<base::TaskRunner> task_runner,
     const UILanguageListResolvedCallback& resolved_callback) {
-  // DCHECK(task_runner->RunsTasksInCurrentSequence());

[achuithb, 2017/05/18 21:36:51]

Is there an appropriate DCHECK to assert the right thread that we could use
here?

   base::ThreadRestrictions::AssertIOAllowed();

[emaxx, 2017/05/19 17:58:16]

nit: While you're here - please add #include
"base/threading/thread_restrictions.h"

[fdoray, 2017/05/19 18:34:28]

Done.

 
   std::string selected_language;
   if (!language_switch_result) {
     selected_language =
         StartupCustomizationDocument::GetInstance()->initial_locale_default();
   } else {
     if (language_switch_result->success) {
       if (language_switch_result->requested_locale ==
           language_switch_result->loaded_locale) {
         selected_language = language_switch_result->requested_locale;
@@ skipping 55 matching lines @@
 }
 
 }  // namespace
 
 void ResolveUILanguageList(
     std::unique_ptr<chromeos::locale_util::LanguageSwitchResult>
         language_switch_result,
     const UILanguageListResolvedCallback& callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
-  content::BrowserThread::GetBlockingPool()->PostTask(
-      FROM_HERE, base::Bind(&ResolveLanguageListOnBlockingPool,
-                            base::Passed(&language_switch_result),
-                            base::ThreadTaskRunnerHandle::Get(), callback));
+  base::PostTaskWithTraits(
+      FROM_HERE, {base::MayBlock()},
+      base::BindOnce(&ResolveLanguageListInThreadPool,
+                     base::Passed(&language_switch_result),
+                     base::SequencedTaskRunnerHandle::Get(), callback));
 }
 
 std::unique_ptr<base::ListValue> GetMinimalUILanguageList() {
   const std::string application_locale =
       g_browser_process->GetApplicationLocale();
   base::string16 language_native_display_name =
       l10n_util::GetDisplayNameForLocale(
           application_locale, application_locale, true);
 
   std::unique_ptr<base::ListValue> language_list(new base::ListValue());
@@ skipping 126 matching lines @@
     input_methods_list->Append(
         CreateInputMethodsEntry(*us_eng_descriptor, selected));
     manager->GetActiveIMEState()->EnableInputMethod(us_keyboard_id);
   }
   return input_methods_list;
 }
 
 void GetKeyboardLayoutsForLocale(
     const GetKeyboardLayoutsForLocaleCallback& callback,
     const std::string& locale) {
-  base::SequencedWorkerPool* worker_pool =
-      content::BrowserThread::GetBlockingPool();
-  scoped_refptr<base::SequencedTaskRunner> background_task_runner =
-      worker_pool->GetSequencedTaskRunnerWithShutdownBehavior(
-          worker_pool->GetNamedSequenceToken(kSequenceToken),
-          base::SequencedWorkerPool::SKIP_ON_SHUTDOWN);

[emaxx, 2017/05/19 17:58:16]

Is there a reason for not keeping the SKIP_ON_SHUTDOWN flag?

[fdoray, 2017/05/19 18:34:28]

No. Added it back.

-
   // Resolve |locale| on a background thread, then continue on the current
   // thread.
   std::string (*get_application_locale)(const std::string&, bool) =
       &l10n_util::GetApplicationLocale;
-  base::PostTaskAndReplyWithResult(
-      background_task_runner.get(), FROM_HERE,
-      base::Bind(get_application_locale, locale, false /* set_icu_locale */),
-      base::Bind(&GetKeyboardLayoutsForResolvedLocale, locale, callback));
+  base::PostTaskWithTraitsAndReplyWithResult(

[fdoray, 2017/05/17 17:52:16]

Is it important to have mutual exclusion between tasks posted by different
GetKeyboardLayoutsForLocale() calls? Previously, this mutual exclusion was
provided by using a SequencedTaskRunner. In the TaskScheduler world, we prefer
not to create sequences for no reason.

[emaxx, 2017/05/19 17:58:16]

Looking at implementation of l10n_util::GetApplicationLocale, I don't think
there's any kind of global state used by it that can lead to concurrency issues.
This function doesn't modify the ICU default locale as long as the
set_icu_locale parameter is passed as false. It also uses some singletons
(base::i18n::IllegalCharacters, ResourceBundle), but they seem to be fine.

+      FROM_HERE, {base::MayBlock()},
+      base::BindOnce(get_application_locale, locale,
+                     false /* set_icu_locale */),
+      base::BindOnce(&GetKeyboardLayoutsForResolvedLocale, locale, callback));
 }
 
 std::unique_ptr<base::DictionaryValue> GetCurrentKeyboardLayout() {
   const input_method::InputMethodDescriptor current_input_method =
       input_method::InputMethodManager::Get()
           ->GetActiveIMEState()
           ->GetCurrentInputMethod();
   return CreateInputMethodsEntry(current_input_method,
                                  current_input_method.id());
 }
 
 }  // namespace chromeos