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 5f7fb306267a4372e837ea45a33e6e37620349e6..66cfab6cbb3edbdb744782e5a688e23917ed8d57 100644 |
--- a/chrome/browser/chromeos/settings/device_oauth2_token_service_factory.cc |
+++ b/chrome/browser/chromeos/settings/device_oauth2_token_service_factory.cc |
@@ -4,11 +4,14 @@ |
#include "chrome/browser/chromeos/settings/device_oauth2_token_service_factory.h" |
+#include "base/bind.h" |
+#include "base/callback.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 "chromeos/cryptohome/system_salt_getter.h" |
#include "content/public/browser/browser_thread.h" |
namespace chromeos { |
@@ -19,10 +22,9 @@ DeviceOAuth2TokenServiceFactory* g_factory = NULL; |
} // namespace |
DeviceOAuth2TokenServiceFactory::DeviceOAuth2TokenServiceFactory() |
- : token_service_(new DeviceOAuth2TokenService( |
- g_browser_process->system_request_context(), |
- g_browser_process->local_state(), |
- new CryptohomeTokenEncryptor)) { |
+ : initialized_(false), |
+ token_service_(NULL), |
+ weak_ptr_factory_(this) { |
} |
DeviceOAuth2TokenServiceFactory::~DeviceOAuth2TokenServiceFactory() { |
@@ -33,30 +35,15 @@ DeviceOAuth2TokenServiceFactory::~DeviceOAuth2TokenServiceFactory() { |
void DeviceOAuth2TokenServiceFactory::Get(const GetCallback& callback) { |
DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)); |
- DeviceOAuth2TokenService* token_service = NULL; |
- if (g_factory) |
- token_service = g_factory->token_service_; |
- |
- // TODO(satorux): Implement async initialization logic for |
- // DeviceOAuth2TokenService. crbug.com/309959. |
- // Here's how that should work: |
- // |
- // if token_service is ready: |
- // run callback asynchronously via MessageLoop |
- // return |
- // |
- // add callback to the pending callback list |
- // |
- // if there is only one pending callback: |
- // start getting the system salt asynchronously... |
- // |
- // 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)); |
+ if (!g_factory) { |
+ base::MessageLoop::current()->PostTask( |
+ FROM_HERE, |
+ base::Bind(callback, |
+ static_cast<DeviceOAuth2TokenService*>(NULL))); |
+ return; |
+ } |
+ |
+ g_factory->RunAsync(callback); |
} |
// static |
@@ -65,6 +52,7 @@ void DeviceOAuth2TokenServiceFactory::Initialize() { |
DCHECK(!g_factory); |
g_factory = new DeviceOAuth2TokenServiceFactory; |
+ g_factory->CreateTokenService(); |
} |
// static |
@@ -77,4 +65,56 @@ void DeviceOAuth2TokenServiceFactory::Shutdown() { |
} |
} |
+void DeviceOAuth2TokenServiceFactory::CreateTokenService() { |
+ DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)); |
+ |
+ SystemSaltGetter::Get()->GetSystemSalt( |
+ base::Bind(&DeviceOAuth2TokenServiceFactory::DidGetSystemSalt, |
+ weak_ptr_factory_.GetWeakPtr())); |
+} |
+ |
+void DeviceOAuth2TokenServiceFactory::DidGetSystemSalt( |
+ const std::string& system_salt) { |
+ DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)); |
+ DCHECK(!token_service_); |
+ |
+ if (system_salt.empty()) { |
+ LOG(ERROR) << "Failed to get the system salt"; |
+ } else { |
+ token_service_= new DeviceOAuth2TokenService( |
+ g_browser_process->system_request_context(), |
+ g_browser_process->local_state(), |
+ new CryptohomeTokenEncryptor(system_salt)); |
+ } |
+ |
+ // Run callbacks regardless of whether token_service_ is created or not, |
+ // but don't run callbacks immediately. This is tricky, but running |
+ // callbacks directly here can introduce a subtle bug, as these callbacks |
+ // may call Get(), which in turn appends new callbacks to |
+ // pending_callbacks_, while the vector is being accessed here. |
+ for (size_t i = 0; i < pending_callbacks_.size(); ++i) { |
pastarmovj
2013/10/24 09:43:20
You can create a local empty vector here and swap(
satorux1
2013/10/24 10:23:14
I considered, but it's still complicated, as we'll
satorux1
2013/10/24 10:53:01
oops. I misunderstood your comment. If we are to s
pastarmovj
2013/10/24 11:28:49
How are they going to be run with the current setu
satorux1
2013/10/24 13:15:52
New callbacks are posted to MessageLoop in RunAsyn
pastarmovj
2013/10/24 13:21:30
This makes sense. Thanks for clarifying this! :)
satorux1
2013/10/24 13:22:00
Let me drop "by the time they are added"...
pneubeck (no reviews)
2013/10/24 19:44:13
nit: use vector<>::const_iterator instead of an in
satorux1
2013/10/25 02:36:02
IMHO that's verbose for little benefit. :) but I c
|
+ base::MessageLoop::current()->PostTask( |
+ FROM_HERE, |
+ base::Bind(pending_callbacks_[i], token_service_)); |
+ } |
+ pending_callbacks_.clear(); |
+ // Also mark that the factory is initialized. |
+ initialized_ = true; |
pneubeck (no reviews)
2013/10/24 19:44:13
you could move this "initialized_=true" to the mor
satorux1
2013/10/25 02:36:02
Thank you for the suggestion. Moved initialized=tr
|
+} |
+ |
+void DeviceOAuth2TokenServiceFactory::RunAsync(const GetCallback& callback) { |
+ DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)); |
+ |
+ if (initialized_) { |
+ // No pending callbacks should be left. |
+ DCHECK(pending_callbacks_.empty()); |
+ base::MessageLoop::current()->PostTask( |
+ FROM_HERE, |
+ base::Bind(callback, token_service_)); |
+ return; |
+ } |
+ |
+ pending_callbacks_.push_back(callback); |
+} |
+ |
} // namespace chromeos |