Fix compile issue for 1133315.

This is a reland for 1133315. A global function was mistakenly moved lower, and was not visible to calls that happened before it.

Refactor BrowserDMTokenStorage for easier platform implementations.

This CL refactors BrowserDMTokenStorage & BrowserDMTokenStorageWin to move code that is common to all platforms.

After this CL, adding a new platform requires implementing the following methods:

-  virtual std::string InitClientId();
-  virtual std::string InitEnrollmentToken();
-  virtual std::string InitDMToken();
-  virtual void SaveDMToken(const std::string& token);

Note that this CL keeps the existing Windows implementation, it only refactors the code (no functional change).

BUG=812641

TBR: pastarmovj
Change-Id: I51d6b4efef6066d602a59c514e9db88e414e4337
Reviewed-on: https://chromium-review.googlesource.com/1138539
Reviewed-by: Georges Khalil <georgesak@chromium.org>
Commit-Queue: Georges Khalil <georgesak@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575356}

chrome/browser/BUILD.gn

@@ -1056,6 +1056,7 @@ jumbo_split_static_library("browser") {
     "plugins/pdf_iframe_navigation_throttle.h",
     "plugins/pdf_plugin_placeholder_observer.cc",
     "plugins/pdf_plugin_placeholder_observer.h",
+    "policy/browser_dm_token_storage.cc",
     "policy/browser_dm_token_storage.h",
     "policy/browser_dm_token_storage_win.cc",
     "policy/browser_dm_token_storage_win.h",
@@ -3163,8 +3164,6 @@ jumbo_split_static_library("browser") {
   } else {
     # Non-Windows.
     sources += [
-      "policy/browser_dm_token_storage_stub.cc",
-      "policy/browser_dm_token_storage_stub.h",
       "profile_resetter/triggered_profile_resetter_stub.cc",
       "profiles/profile_shortcut_manager_stub.cc",
     ]

chrome/browser/policy/browser_dm_token_storage.cc

@@ -0,0 +1,143 @@
+// Copyright 2018 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/policy/browser_dm_token_storage.h"
+
+#include <memory>
+#include <utility>
+#include <vector>
+
+#include "base/base64.h"
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/callback.h"
+#include "base/logging.h"
+#include "base/no_destructor.h"
+#include "base/strings/string16.h"
+#include "base/strings/utf_string_conversions.h"
+#include "base/task_runner_util.h"
+#include "base/task_scheduler/post_task.h"
+#include "base/threading/sequenced_task_runner_handle.h"
+#include "base/threading/thread_task_runner_handle.h"
+#include "build/build_config.h"
+#include "content/public/browser/browser_thread.h"
+
+namespace policy {
+
+// static
+BrowserDMTokenStorage* BrowserDMTokenStorage::storage_for_testing_ = nullptr;
+
+// Static function that can't be overridden. Implementation is only compiled for
+// non-supported platforms.
+#if !defined(OS_WIN)
+// static
+BrowserDMTokenStorage* BrowserDMTokenStorage::Get() {
+  if (storage_for_testing_)
+    return storage_for_testing_;
+
+  static base::NoDestructor<BrowserDMTokenStorage> storage;
+  return storage.get();
+}
+#endif
+
+BrowserDMTokenStorage::BrowserDMTokenStorage() : is_initialized_(false) {
+  DETACH_FROM_SEQUENCE(sequence_checker_);
+
+  // We don't call InitIfNeeded() here so that the global instance can be
+  // created early during startup if needed. The tokens and client ID are read
+  // from the system as part of the first retrieve or store operation.
+}
+
+BrowserDMTokenStorage::~BrowserDMTokenStorage() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+}
+
+std::string BrowserDMTokenStorage::RetrieveClientId() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+  InitIfNeeded();
+  return client_id_;
+}
+
+std::string BrowserDMTokenStorage::RetrieveEnrollmentToken() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+  InitIfNeeded();
+  return enrollment_token_;
+}
+
+void BrowserDMTokenStorage::StoreDMToken(const std::string& dm_token,
+                                         StoreCallback callback) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  DCHECK(!store_callback_);
+  InitIfNeeded();
+
+  dm_token_ = dm_token;
+
+  store_callback_ = std::move(callback);
+
+  SaveDMToken(dm_token);
+}
+
+std::string BrowserDMTokenStorage::RetrieveDMToken() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  DCHECK(!store_callback_);
+
+  InitIfNeeded();
+  return dm_token_;
+}
+
+void BrowserDMTokenStorage::InitIfNeeded() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+  if (is_initialized_)
+    return;
+
+  is_initialized_ = true;
+  client_id_ = InitClientId();
+  DVLOG(1) << "Client ID = " << client_id_;
+  if (client_id_.empty())
+    return;
+
+  enrollment_token_ = InitEnrollmentToken();
+  DVLOG(1) << "Enrollment token = " << enrollment_token_;
+
+  dm_token_ = InitDMToken();
+  DVLOG(1) << "DM Token = " << dm_token_;
+}
+
+void BrowserDMTokenStorage::OnDMTokenStored(bool success) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  DCHECK(store_callback_);
+
+  if (!store_callback_.is_null())
+    std::move(store_callback_).Run(success);
+}
+
+// Stub implementation. This function will become virtual pure once Mac & Linux
+// implementations are done.
+std::string BrowserDMTokenStorage::InitClientId() {
+  return std::string();
+}
+
+// Stub implementation. This function will become virtual pure once Mac & Linux
+// implementations are done.
+std::string BrowserDMTokenStorage::InitEnrollmentToken() {
+  return std::string();
+}
+
+// Stub implementation. This function will become virtual pure once Mac & Linux
+// implementations are done.
+std::string BrowserDMTokenStorage::InitDMToken() {
+  return std::string();
+}
+
+// Stub implementation. This function will become virtual pure once Mac & Linux
+// implementations are done.
+void BrowserDMTokenStorage::SaveDMToken(const std::string& token) {
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::BindOnce(std::move(store_callback_), false));
+}
+
+}  // namespace policy

chrome/browser/policy/browser_dm_token_storage.h

@@ -10,6 +10,11 @@
 
 #include "base/callback_forward.h"
 #include "base/macros.h"
+#include "base/memory/scoped_refptr.h"
+#include "base/memory/weak_ptr.h"
+#include "base/no_destructor.h"
+#include "base/sequence_checker.h"
+#include "base/single_thread_task_runner.h"
 
 namespace policy {
 
@@ -25,19 +30,22 @@ class BrowserDMTokenStorage {
 
   // Returns the global singleton object. Must be called from the UI thread.
   static BrowserDMTokenStorage* Get();
-  // Returns a client ID unique to the machine.
-  virtual std::string RetrieveClientId() = 0;
-  // Returns the enrollment token, or an empty string if there is none.
-  virtual std::string RetrieveEnrollmentToken() = 0;
-  // Asynchronously stores |dm_token| in the registry and calls |callback| with
-  // a boolean to indicate success or failure. It is an error to attempt
-  // concurrent store operations.
+  // Returns a client ID unique to the machine. Virtual for tests.
+  virtual std::string RetrieveClientId();
+  // Returns the enrollment token, or an empty string if there is none. Virtual
+  // for tests.
+  virtual std::string RetrieveEnrollmentToken();
+  // Asynchronously stores |dm_token| and calls |callback| with a boolean to
+  // indicate success or failure. It is an error to attempt concurrent store
+  // operations. Virtual for tests.
   virtual void StoreDMToken(const std::string& dm_token,
-                            StoreCallback callback) = 0;
-  // Returns an already stored DM token from the registry or from the cache in
-  // memory. An empty token is returned if no DM token exists on the system or
-  // an error is encountered.
-  virtual std::string RetrieveDMToken() = 0;
+                            StoreCallback callback);
+  // Returns an already stored DM token. An empty token is returned if no DM
+  // token exists on the system or an error is encountered. Virtual for tests.
+  virtual std::string RetrieveDMToken();
+  // Must be called after the DM token is saved, to ensure that the callback is
+  // invoked.
+  void OnDMTokenStored(bool success);
 
   // Set the mock BrowserDMTokenStorage for testing. The caller owns the
   // instance of the storage.
@@ -46,12 +54,42 @@ class BrowserDMTokenStorage {
   }
 
  protected:
-  BrowserDMTokenStorage() = default;
-  virtual ~BrowserDMTokenStorage() = default;
+  friend class base::NoDestructor<BrowserDMTokenStorage>;
+
+  // Get the global singleton instance by calling BrowserDMTokenStorage::Get().
+  BrowserDMTokenStorage();
+  virtual ~BrowserDMTokenStorage();
 
  private:
   static BrowserDMTokenStorage* storage_for_testing_;
 
+  // Initializes the DMTokenStorage object and caches the ids and tokens. This
+  // is called the first time the BrowserDMTokenStorage is interacted with.
+  void InitIfNeeded();
+
+  // Gets the client ID and stores it in |client_id_|. This implementation is
+  // platform dependant.
+  virtual std::string InitClientId();
+  // Gets the enrollment token and stores it in |enrollment_token_|. This
+  // implementation is platform dependant.
+  virtual std::string InitEnrollmentToken();
+  // Gets the DM token and stores it in |dm_token_|. This implementation is
+  // platform dependant.
+  virtual std::string InitDMToken();
+  // Saves the DM token. This implementation is platform dependant.
+  virtual void SaveDMToken(const std::string& token);
+
+  // Will be called after the DM token is stored.
+  StoreCallback store_callback_;
+
+  bool is_initialized_;
+
+  std::string client_id_;
+  std::string enrollment_token_;
+  std::string dm_token_;
+
+  SEQUENCE_CHECKER(sequence_checker_);
+
   DISALLOW_COPY_AND_ASSIGN(BrowserDMTokenStorage);
 };