settings: Make DeviceOAuth2TokenServiceFactory::Get() async

chrome/browser/chromeos/settings/device_oauth2_token_service_factory.cc

Index: chrome/browser/chromeos/settings/device_oauth2_token_service_factory.cc
diff --git a/chrome/browser/chromeos/settings/device_oauth2_token_service_factory.cc b/chrome/browser/chromeos/settings/device_oauth2_token_service_factory.cc
index 57b6efb1dae9df33137b45a784612bd260fd7b5c..ed88fd803951815816f42f3a1e5807e6591c0fba 100644
--- a/chrome/browser/chromeos/settings/device_oauth2_token_service_factory.cc
+++ b/chrome/browser/chromeos/settings/device_oauth2_token_service_factory.cc
@@ -4,40 +4,77 @@
 
 #include "chrome/browser/chromeos/settings/device_oauth2_token_service_factory.h"
 
+#include "base/message_loop/message_loop.h"
+#include "base/tracked_objects.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/settings/device_oauth2_token_service.h"
 #include "chrome/browser/chromeos/settings/token_encryptor.h"
 #include "content/public/browser/browser_thread.h"
 
 namespace chromeos {
+namespace {
 
-static DeviceOAuth2TokenService* g_device_oauth2_token_service_ = NULL;
+DeviceOAuth2TokenServiceFactory* g_factory = NULL;
 
-DeviceOAuth2TokenServiceFactory::DeviceOAuth2TokenServiceFactory() {
+}  // namespace
+
+DeviceOAuth2TokenServiceFactory::DeviceOAuth2TokenServiceFactory()
+    : token_service_(new DeviceOAuth2TokenService(
+        g_browser_process->system_request_context(),
+        g_browser_process->local_state(),
+        new CryptohomeTokenEncryptor)) {
+}
+
+DeviceOAuth2TokenServiceFactory::~DeviceOAuth2TokenServiceFactory() {
+  delete token_service_;
 }
 
 // static
-DeviceOAuth2TokenService* DeviceOAuth2TokenServiceFactory::Get() {
+void DeviceOAuth2TokenServiceFactory::Get(const GetCallback& callback) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
-  return g_device_oauth2_token_service_;
+
+  DeviceOAuth2TokenService* token_service = NULL;
+  if (g_factory)
+    token_service = g_factory->token_service_;
+
+  // TODO(satorux): Implement async initialization logic for
+  // DeviceOAuth2TokenService. Here's how that should work:
+  //
+  // if token_service is ready:
+  //   run callback asynchronously

[pneubeck (no reviews), 2013/10/22 09:05:03]

do you mean "immediately" or "synchronously" instead of "asynchronously" here?

[satorux1, 2013/10/22 09:56:43]

I meant asynchronously. we could run callback.Run() here, but running it through
MessageLoop would be a better behavior, as the caller may not expect callback to
be run immediately

+  //   return
+  //
+  // if there is no pending callback:

[satorux1, 2013/10/22 09:56:43]

FWIW, I meant "if there is any pending callback"

+  //   add callback to the pending callback list
+  //   return
+  //
+  // add callback to the pending callback list
+  // start getting the system salt asynchronously...

[pneubeck (no reviews), 2013/10/22 09:05:03]

Why should we do this lazily triggered by the Get() function?
Couldn't we instead start getting the system salt always and as early as
possible? I.e. either in DeviceOAuth2TokenServiceFactory::Initialize() or in
CryptohomeClient ?

In anyways, putting most of the complexity described here into CryptohomeClient,
would have the advantage that it might simplify the implementation everywhere
where the system salt is required and not only solve the problem
DeviceOAuth2TokenService.

[satorux1, 2013/10/22 09:56:43]

Thanks. Will explore the idea, when I implement the logic outlined here.
Yes. hashimoto@ has just added retry logic in SystemSaltGetter, which helps for
everything.

https://codereview.chromium.org/34303002/

+  //
+  // upon receiving the system salt:
+  //   create CryptohomeTokenEncryptor with that key
+  //   create DeviceOAuth2TokenService
+  //   run all the pending callbacks
+  base::MessageLoop::current()->PostTask(
+      FROM_HERE,
+      base::Bind(callback, token_service));
 }
 
 // static
 void DeviceOAuth2TokenServiceFactory::Initialize() {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
-  DCHECK(!g_device_oauth2_token_service_);
-  g_device_oauth2_token_service_ = new DeviceOAuth2TokenService(
-      g_browser_process->system_request_context(),
-      g_browser_process->local_state(),
-      new CryptohomeTokenEncryptor);
+
+  DCHECK(!g_factory);
+  g_factory = new DeviceOAuth2TokenServiceFactory;
 }
 
 // static
 void DeviceOAuth2TokenServiceFactory::Shutdown() {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
-  if (g_device_oauth2_token_service_) {
-    delete g_device_oauth2_token_service_;
-    g_device_oauth2_token_service_ = NULL;
+
+  if (g_factory) {
+    delete g_factory;
+    g_factory = NULL;
   }
 }