Remove the usage of BrowserThread::FILE in the shell_integration* files

chrome/browser/shell_integration.cc

 // Copyright (c) 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/shell_integration.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/files/file_util.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/task_scheduler/post_task.h"
 #include "base/threading/thread_restrictions.h"
 #include "build/build_config.h"
 #include "chrome/browser/policy/policy_path_parser.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "components/prefs/pref_service.h"
 #include "components/version_info/version_info.h"
 #include "content/public/browser/browser_thread.h"
 
 #if defined(OS_CHROMEOS)
@@ skipping 105 matching lines @@
     return l10n_util::GetStringUTF16(IDS_APP_SHORTCUTS_SUBDIR_NAME_CANARY);
   return l10n_util::GetStringUTF16(IDS_APP_SHORTCUTS_SUBDIR_NAME);
 }
 #endif  // !defined(OS_WIN)
 
 ///////////////////////////////////////////////////////////////////////////////
 // DefaultWebClientWorker
 //
 
 void DefaultWebClientWorker::StartCheckIsDefault() {
-  BrowserThread::PostTask(
-      BrowserThread::FILE, FROM_HERE,
-      base::Bind(&DefaultWebClientWorker::CheckIsDefault, this, false));
+  base::CreateSequencedTaskRunnerWithTraits({base::MayBlock()})

[gab, 2017/05/26 18:44:05]

Why does this need to be called on a sequence which isn't re-used? Does it use
SequencedTaskRunnerHandle or something?

If there are multiple check default calls, shouldn't they be sequenced between
themselves? And with SetAsDefault calls as well?

[Patrick Monette, 2017/05/29 18:54:14]

I decided to go with sequencing everything

+      ->PostTask(FROM_HERE, base::Bind(&DefaultWebClientWorker::CheckIsDefault,
+                                       this, false));
 }
 
 void DefaultWebClientWorker::StartSetAsDefault() {
-  BrowserThread::PostTask(
-      BrowserThread::FILE, FROM_HERE,
-      base::Bind(&DefaultWebClientWorker::SetAsDefault, this));
+  GetSetDefaultTaskRunner()->PostTask(
+      FROM_HERE, base::Bind(&DefaultWebClientWorker::SetAsDefault, this));
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // DefaultWebClientWorker, protected:
 
 DefaultWebClientWorker::DefaultWebClientWorker(
     const DefaultWebClientWorkerCallback& callback,
     const char* worker_name)
     : callback_(callback), worker_name_(worker_name) {}
 
 DefaultWebClientWorker::~DefaultWebClientWorker() = default;
 
 void DefaultWebClientWorker::OnCheckIsDefaultComplete(
     DefaultWebClientState state,
     bool is_following_set_as_default) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   UpdateUI(state);
 
   if (is_following_set_as_default)
     ReportSetDefaultResult(state);
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // DefaultWebClientWorker, private:
 
+scoped_refptr<base::SequencedTaskRunner>
+DefaultWebClientWorker::GetSetDefaultTaskRunner() {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+#if defined(OS_WIN)
+  if (base::win::GetVersion() >= base::win::VERSION_WIN10) {
+    // On Windows 10, all the calls to SetAsDefault() must be done on the same
+    // SequencedTaskRunner (see OpenSystemSettingsHelper). Because the callers
+    // don't know each others, a global is shared between them.
+    //
+    // TODO(pmonette): Windows 10's implementation uses a base::Timer which
+    // currently still requires a SingleThreadTaskRunner. Change this to a
+    // SequencedTaskRunner when crbug.com/552633 is fixed.

[gab, 2017/05/26 18:44:05]

Feels like we might as well just sequence all attempts to SetAsDefault() on all
platforms. Feels wrong to SetAsDefault() in parallel on any platform.

[Patrick Monette, 2017/05/29 18:54:14]

Sounds good.

+    static base::SingleThreadTaskRunner* task_runner = nullptr;

[gab, 2017/05/26 18:44:05]

Make this a scoped_refptr<> assign the result of base::Create...() directly into
it. Will avoid the whole AddRef() dance.

[Patrick Monette, 2017/05/29 18:54:14]

Done.

+
+    if (!task_runner) {
+      scoped_refptr<base::SingleThreadTaskRunner> new_task_runner =
+          base::CreateSingleThreadTaskRunnerWithTraits({base::MayBlock()});
+
+      new_task_runner->AddRef();
+      task_runner = new_task_runner.get();
+    }
+
+    return task_runner;
+  }
+#endif  // defined(OS_WIN)

[gab, 2017/05/26 18:44:05]

#else?

[Patrick Monette, 2017/05/29 18:54:14]

Done.

+  return base::CreateSequencedTaskRunnerWithTraits({base::MayBlock()});
+#endif  // !defined(OS_WIN)

[gab, 2017/05/26 18:44:05]

#endif  // defined(OS_WIN)

           ^ don't add '!', keep it the same as initial statement even in the
"else" step

[Patrick Monette, 2017/05/29 18:54:14]

Done.

+}
+
 void DefaultWebClientWorker::CheckIsDefault(bool is_following_set_as_default) {
-  DCHECK_CURRENTLY_ON(BrowserThread::FILE);
+  base::ThreadRestrictions::AssertIOAllowed();
+
   DefaultWebClientState state = CheckIsDefaultImpl();
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       base::Bind(&DefaultBrowserWorker::OnCheckIsDefaultComplete, this, state,
                  is_following_set_as_default));
 }
 
 void DefaultWebClientWorker::SetAsDefault() {
-  DCHECK_CURRENTLY_ON(BrowserThread::FILE);
+  base::ThreadRestrictions::AssertIOAllowed();
 
   // SetAsDefaultImpl will make sure the callback is executed exactly once.
   SetAsDefaultImpl(
       base::Bind(&DefaultWebClientWorker::CheckIsDefault, this, true));
 }
 
 void DefaultWebClientWorker::ReportSetDefaultResult(
     DefaultWebClientState state) {
   base::LinearHistogram::FactoryGet(
       base::StringPrintf("%s.SetDefaultResult2", worker_name_), 1,
@@ skipping 114 matching lines @@
             return;
         }
       }
 #endif  // defined(OS_WIN)
       break;
   }
   on_finished_callback.Run();
 }
 
 }  // namespace shell_integration