settings: Make DeviceOAuth2TokenServiceFactory::Get() async

chrome/browser/extensions/api/identity/identity_api.cc

Index: chrome/browser/extensions/api/identity/identity_api.cc
diff --git a/chrome/browser/extensions/api/identity/identity_api.cc b/chrome/browser/extensions/api/identity/identity_api.cc
index 485711e8be8eebae06df1ae73b4a274d067a56d3..5287ffe31d40124caa5ad74cc08e2305856886c4 100644
--- a/chrome/browser/extensions/api/identity/identity_api.cc
+++ b/chrome/browser/extensions/api/identity/identity_api.cc
@@ -393,8 +393,13 @@ void IdentityGetAuthTokenFunction::OnGetTokenFailure(
 
 #if defined(OS_CHROMEOS)
 void IdentityGetAuthTokenFunction::StartDeviceLoginAccessTokenRequest() {
-  chromeos::DeviceOAuth2TokenService* service =
-      chromeos::DeviceOAuth2TokenServiceFactory::Get();
+  chromeos::DeviceOAuth2TokenServiceFactory::Get(
+      base::Bind(&IdentityGetAuthTokenFunction::DidGetTokenService,
+                 this));

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

should this really rely on the ref-counting and potentially extend the lifetime
of this object?

alternatively use a WeakPtr

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

Inside API functions, relying on ref counting is a common practice, so I'd just
go with this, until this becomes a problem.

+}
+
+void IdentityGetAuthTokenFunction::DidGetTokenService(
+    chromeos::DeviceOAuth2TokenService* service) {

[hashimoto, 2013/10/22 08:49:51]

nit: No need to have a NULL check here?

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

The original code didn't have one, but sounds like a good idea. will add it.

[Michael Courage, 2013/10/23 00:22:47]

When would the factory return NULL? Is this going to be a DCHECK or some new
error case developers might have to interpret? 

[satorux1, 2013/10/23 04:23:27]

Per the header file [1], it can return NULL during the browser startup and
shutdown. I think it can also return NULL in unit tests where
DeviceOAuth2TokenServiceFactory is not initialized.  I think it's a good
practice to check NULL.

[1]
https://codereview.chromium.org/34523003/diff/40001/chrome/browser/chromeos/s...

[satorux1, 2013/10/23 06:35:44]

added:

  if (!service) {
    CompleteFunctionWithError(identity_constants::kAuthFailure);
    return;
  }
 
I thought about doing DCHECK(service) instead as this function won't be called
during browse startup and shutdown in production, but I decided to check NULL,
as this function may be called from tests where DeviceOAuth2TokenServiceFactory
is not initialized

   // Since robot account refresh tokens are scoped down to [any-api] only,
   // request access token for [any-api] instead of login.
   OAuth2TokenService::ScopeSet scopes;