[OSCrypt][Password Manager] Introduce TaskRunner for libsecret

A SequencedTaskRunner will be used to guarantee no race conditions
between the two clients of libsecret.

Bug: 782851
Change-Id: I6549f867a14378d019404429bab42008fca548a7
Reviewed-on: https://chromium-review.googlesource.com/803954
Commit-Queue: Christos Froussios <cfroussios@chromium.org>
Reviewed-by: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523409}

chrome/browser/password_manager/native_backend_libsecret.cc

@@ -20,6 +20,7 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
+#include "components/os_crypt/libsecret_task_runner_linux.h"
 #include "components/password_manager/core/browser/password_manager_metrics_util.h"
 #include "components/password_manager/core/browser/password_manager_util.h"
 #include "url/origin.h"
@@ -412,7 +413,7 @@ bool NativeBackendLibsecret::GetAllLogins(
 
 scoped_refptr<base::SequencedTaskRunner>
 NativeBackendLibsecret::GetBackgroundTaskRunner() {
-  return nullptr;
+  return os_crypt::GetLibsecretTaskRunner();
 }
 
 bool NativeBackendLibsecret::GetLoginsList(

components/os_crypt/BUILD.gn

@@ -97,6 +97,8 @@ static_library("os_crypt") {
       sources += [
         "key_storage_libsecret.cc",
         "key_storage_libsecret.h",
+        "libsecret_task_runner_linux.cc",
+        "libsecret_task_runner_linux.h",
         "libsecret_util_linux.cc",
         "libsecret_util_linux.h",
       ]

components/os_crypt/key_storage_libsecret.cc

@@ -6,7 +6,9 @@
 
 #include "base/base64.h"
 #include "base/rand_util.h"
+#include "base/sequenced_task_runner.h"
 #include "base/strings/string_number_conversions.h"
+#include "components/os_crypt/libsecret_task_runner_linux.h"
 #include "components/os_crypt/libsecret_util_linux.h"
 
 namespace {
@@ -51,6 +53,10 @@ SecretValue* ToSingleSecret(GList* secret_items) {
 
 }  // namespace
 
+base::SequencedTaskRunner* KeyStorageLibsecret::GetTaskRunner() {
+  return os_crypt::GetLibsecretTaskRunner().get();
+}
+
 std::string KeyStorageLibsecret::AddRandomPasswordInLibsecret() {
   std::string password;
   base::Base64Encode(base::RandBytesAsString(16), &password);

components/os_crypt/key_storage_libsecret.h

@@ -10,6 +10,10 @@
 #include "base/macros.h"
 #include "components/os_crypt/key_storage_linux.h"
 
+namespace base {
+class SequencedTaskRunner;
+}
+
 // Specialisation of KeyStorageLinux that uses Libsecret.
 class KeyStorageLibsecret : public KeyStorageLinux {
  public:
@@ -18,6 +22,7 @@ class KeyStorageLibsecret : public KeyStorageLinux {
 
  protected:
   // KeyStorageLinux
+  base::SequencedTaskRunner* GetTaskRunner() override;
   bool Init() override;
   std::string GetKeyImpl() override;
 

components/os_crypt/key_storage_libsecret_unittest.cc

@@ -6,6 +6,7 @@
 
 #include "base/logging.h"
 #include "base/macros.h"
+#include "base/test/scoped_task_environment.h"
 #include "components/os_crypt/key_storage_libsecret.h"
 #include "components/os_crypt/libsecret_util_linux.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -215,6 +216,8 @@ class LibsecretTest : public testing::Test {
   void TearDown() override { MockLibsecretLoader::TearDown(); }
 
  private:
+  base::test::ScopedTaskEnvironment scoped_task_environment_;
+
   DISALLOW_COPY_AND_ASSIGN(LibsecretTest);
 };
 

components/os_crypt/libsecret_task_runner_linux.cc

+// Copyright 2017 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 "components/os_crypt/libsecret_task_runner_linux.h"
+
+#include "base/task_scheduler/lazy_task_runner.h"
+
+namespace os_crypt {
+
+namespace {
+
+// Use TaskPriority::USER_BLOCKING, because profile initialisation may block on
+// initialising OSCrypt, which in turn may contact libsecret.
+base::LazySingleThreadTaskRunner g_libsecret_thread_task_runner =
+    LAZY_SINGLE_THREAD_TASK_RUNNER_INITIALIZER(
+        base::TaskTraits(base::MayBlock(), base::TaskPriority::USER_BLOCKING),
+        base::SingleThreadTaskRunnerThreadMode::SHARED);
+
+}  // namespace
+
+// TODO(crbug.com/571003) Remove this if OSCrypt is the only client of keyring.
+scoped_refptr<base::SequencedTaskRunner> GetLibsecretTaskRunner() {
+  return g_libsecret_thread_task_runner.Get();
+}
+
+}  // namespace os_crypt

components/os_crypt/libsecret_task_runner_linux.h

+// Copyright 2017 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.
+
+#ifndef COMPONENTS_OS_CRYPT_LIBSECRET_TASK_RUNNER_LINUX_H_
+#define COMPONENTS_OS_CRYPT_LIBSECRET_TASK_RUNNER_LINUX_H_
+
+#include "base/memory/ref_counted.h"
+#include "base/sequenced_task_runner.h"
+
+// Concurrent calls to the libsecret library may sometimes cause erroneous
+// behaviour. Use this TaskRunner to remove race conditions between all
+// components interacting with libsecret.
+
+namespace os_crypt {
+
+scoped_refptr<base::SequencedTaskRunner> GetLibsecretTaskRunner();
+
+}  // namespace os_crypt
+
+#endif  // COMPONENTS_OS_CRYPT_LIBSECRET_TASK_RUNNER_LINUX_H_