implement sign in dialog

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

Index: chrome/browser/extensions/api/identity/identity_api.cc
===================================================================
--- chrome/browser/extensions/api/identity/identity_api.cc	(revision 146190)
+++ chrome/browser/extensions/api/identity/identity_api.cc	(working copy)
@@ -12,9 +12,14 @@
 #include "chrome/browser/signin/token_service.h"
 #include "chrome/browser/signin/token_service_factory.h"
 #include "chrome/browser/ui/browser.h"
-#include "chrome/browser/ui/tab_contents/tab_contents.h"
+#include "chrome/browser/ui/browser_navigator.h"
+#include "chrome/browser/ui/webui/signin/login_ui_service.h"
+#include "chrome/browser/ui/webui/signin/login_ui_service_factory.h"
 #include "chrome/common/extensions/extension.h"
+#include "chrome/common/url_constants.h"
+#include "content/public/common/page_transition_types.h"
 #include "googleurl/src/gurl.h"
+#include "webkit/glue/window_open_disposition.h"
 
 namespace extensions {
 
@@ -26,6 +31,7 @@
 const char kAuthFailure[] = "OAuth2 request failed: ";
 const char kNoGrant[] = "OAuth2 not granted or revoked.";
 const char kUserRejected[] = "The user did not approve access.";
+const char kUserNotSignedIn[] = "The user is not signed in.";
 
 }  // namespace
 
@@ -38,12 +44,19 @@
   arg->GetBoolean("interactive", &interactive_);
 
   // Balanced in OnIssueAdviceSuccess|OnMintTokenSuccess|OnMintTokenFailure|
-  // InstallUIAbort.
+  // InstallUIAbort|OnLoginUIClosed.
   AddRef();
 
-  if (StartFlow(ExtensionInstallPrompt::ShouldAutomaticallyApproveScopes() ?
-                    OAuth2MintTokenFlow::MODE_MINT_TOKEN_FORCE :
-                    OAuth2MintTokenFlow::MODE_MINT_TOKEN_NO_FORCE)) {
+  if (!HasLoginToken()) {
+    if (StartLogin()) {
+      return true;
+    } else {
+      Release();
+      return false;
+    }
+  }
+
+  if (StartFlow(GetTokenFlowMode())) {

[Mihai Parparita -not on Chrome, 2012/07/12 00:39:21]

Rather than having to always call StartToken with GetTokenFlowMode, can you just
make it parameter-less and call GetTokenFlowMode() inside of it, when creating
the parameters?

[Munjal (Google), 2012/07/12 18:41:56]

Actually ,that is what I first tried until I realized that StartFlow is
sometimes called with a specific parameter also. It is not always determined via
a command line flag.

So I either do this or I overload StartFlow. But I thought latter was
inappropriate since the only reason I would add an overload is to not duplicate
the code of determining the mode from command line flag, which I can do via a
helper instead of overloading StartFlow.

[Mihai Parparita -not on Chrome, 2012/07/12 23:31:32]

Ah, I'd missed the StartFlow(OAuth2MintTokenFlow::MODE_MINT_TOKEN_FORCE)
callsite. Leaving it as-is is fine.

     return true;
   } else {
     Release();
@@ -79,6 +92,20 @@
   }
 }
 
+void GetAuthTokenFunction::OnLoginUIShown(LoginUIService::LoginUI* ui) {
+  // Do nothing when login ui is shown.
+}
+
+void GetAuthTokenFunction::OnLoginUIClosed(LoginUIService::LoginUI* ui) {
+  LoginUIService* login_ui_service =
+      LoginUIServiceFactory::GetForProfile(profile());
+  login_ui_service->RemoveObserver(this);
+  if (!StartFlow(GetTokenFlowMode())) {
+    SendResponse(false);
+    Release();
+  }
+}
+
 void GetAuthTokenFunction::InstallUIProceed() {
   DCHECK(install_ui_->record_oauth2_grant());
   // The user has accepted the scopes, so we may now force (recording a grant
@@ -107,6 +134,11 @@
     return false;
   }
 
+  if (!HasLoginToken()) {
+    error_ = kUserNotSignedIn;
+    return false;
+  }
+
   TokenService* token_service = TokenServiceFactory::GetForProfile(profile());
   flow_.reset(new OAuth2MintTokenFlow(
       profile()->GetRequestContext(),
@@ -121,6 +153,44 @@
   return true;
 }
 
+bool GetAuthTokenFunction::StartLogin() {
+  if (!interactive_) {
+    error_ = kUserNotSignedIn;
+    return false;
+  }
+
+  LoginUIService* login_ui_service =
+      LoginUIServiceFactory::GetForProfile(profile());
+  login_ui_service->AddObserver(this);
+
+  LoginUIService::LoginUI* login_ui = login_ui_service->current_login_ui();
+  if (login_ui) {
+    // Focus existing UI.

[Mihai Parparita -not on Chrome, 2012/07/12 00:39:21]

Nit: This comment (and the one a couple of lines below) are just stating what
the code does, and so don't really seem necessary.

[Munjal (Google), 2012/07/12 18:41:56]

Done.

+    login_ui->FocusUI();
+  } else {
+    // Open login UI in a new popup.
+    chrome::NavigateParams params(NULL,
+                                  GURL(chrome::kChromeUISyncPromoURL),

[Mihai Parparita -not on Chrome, 2012/07/12 00:39:21]

Can knowledge of this page and its URL be encapsulated into LoginUIService? It
already has ShowLoginUI, perhaps it have an overloaded variant that does't
require a Browser which opens the UI in a popup.

[Munjal (Google), 2012/07/12 18:41:56]

Actually I had it exactly that way. But in talking to Andrew (who wrote
LoginUIService), we agreed that it is better to do i here because:
- He is going to remove ShowLoginUI method in LoginUIService
- This is the only place that needs to show login UI popup.

So we agreed that it is better to not add another method to LoginUIService.

[Mihai Parparita -not on Chrome, 2012/07/12 23:31:32]

OK

+                                  content::PAGE_TRANSITION_START_PAGE);
+    params.profile = profile();
+    params.disposition = NEW_POPUP;
+    chrome::Navigate(&params);
+  }
+
+  return true;
+}
+
+bool GetAuthTokenFunction::HasLoginToken() const {
+  TokenService* token_service = TokenServiceFactory::GetForProfile(profile());
+  return token_service->HasOAuthLoginToken();
+}
+
+OAuth2MintTokenFlow::Mode GetAuthTokenFunction::GetTokenFlowMode() const {
+  return ExtensionInstallPrompt::ShouldAutomaticallyApproveScopes() ?
+             OAuth2MintTokenFlow::MODE_MINT_TOKEN_FORCE :
+             OAuth2MintTokenFlow::MODE_MINT_TOKEN_NO_FORCE;
+}
+
 LaunchWebAuthFlowFunction::LaunchWebAuthFlowFunction() {}
 LaunchWebAuthFlowFunction::~LaunchWebAuthFlowFunction() {}