Export a private webstore API to call into the new inline sign-in flow.

chrome/browser/extensions/api/webstore_private/webstore_private_api.cc

Index: chrome/browser/extensions/api/webstore_private/webstore_private_api.cc
diff --git a/chrome/browser/extensions/api/webstore_private/webstore_private_api.cc b/chrome/browser/extensions/api/webstore_private/webstore_private_api.cc
index 15a5f04c4c782077609fb5293d378bae7d7dd638..7da4c96c1d826cc01dcff1024929b134264e3880 100644
--- a/chrome/browser/extensions/api/webstore_private/webstore_private_api.cc
+++ b/chrome/browser/extensions/api/webstore_private/webstore_private_api.cc
@@ -24,26 +24,33 @@
 #include "chrome/browser/gpu/gpu_feature_checker.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/signin/signin_manager.h"
-#include "chrome/browser/signin/signin_manager_factory.h"
+#include "chrome/browser/signin/signin_promo.h"
 #include "chrome/browser/sync/profile_sync_service.h"
 #include "chrome/browser/sync/profile_sync_service_factory.h"
 #include "chrome/browser/ui/app_list/app_list_service.h"
 #include "chrome/browser/ui/app_list/app_list_util.h"
 #include "chrome/browser/ui/browser.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_constants.h"
 #include "chrome/common/extensions/extension_l10n_util.h"
 #include "chrome/common/pref_names.h"
+#include "chrome/common/profile_management_switches.h"
 #include "content/public/browser/gpu_data_manager.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/common/page_transition_types.h"
+#include "content/public/common/referrer.h"
 #include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/common/error_utils.h"
 #include "extensions/common/extension.h"
+#include "google_apis/gaia/google_service_auth_error.h"
 #include "grit/chromium_strings.h"
 #include "grit/generated_resources.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "url/gurl.h"
 
 using content::GpuDataManager;
 
@@ -58,6 +65,7 @@ namespace GetStoreLogin = api::webstore_private::GetStoreLogin;
 namespace GetWebGLStatus = api::webstore_private::GetWebGLStatus;
 namespace InstallBundle = api::webstore_private::InstallBundle;
 namespace IsInIncognitoMode = api::webstore_private::IsInIncognitoMode;
+namespace SignIn = api::webstore_private::SignIn;
 namespace SetStoreLogin = api::webstore_private::SetStoreLogin;
 
 namespace {
@@ -686,4 +694,117 @@ bool WebstorePrivateIsInIncognitoModeFunction::RunImpl() {
   return true;
 }
 
+WebstorePrivateSignInFunction::WebstorePrivateSignInFunction()
+    : signin_manager_(NULL) {}
+WebstorePrivateSignInFunction::~WebstorePrivateSignInFunction() {}
+
+bool WebstorePrivateSignInFunction::RunImpl() {
+  scoped_ptr<SignIn::Params> params = SignIn::Params::Create(*args_);
+  EXTENSION_FUNCTION_VALIDATE(params);
+
+  // The |continue_url| is required, and must be hosted on the same origin as
+  // the calling page.
+  // TODO(isherman): Is it better to pass the error message to the callback, or
+  // to set it as the last |error_|?

[Ilya Sherman, 2014/03/14 05:42:56]

^^^

[guohui, 2014/03/14 22:09:44]

i think it is better to pass the error to callback, otherwise we need another
function to expose the last error to webstore.

+  GURL continue_url(params->continue_url);
+  content::WebContents* web_contents = GetAssociatedWebContents();
+  if (!continue_url.is_valid() ||
+      continue_url.GetOrigin() !=
+          web_contents->GetLastCommittedURL().GetOrigin()) {
+    error_ = "invalid_continue_url";
+    SendResponse(false);
+    return false;
+  }
+
+  // If sign-in is disallowed, give up.
+  signin_manager_ = SigninManagerFactory::GetForProfile(GetProfile());
+  if (!signin_manager_ || !signin_manager_->IsSigninAllowed() ||
+      switches::IsEnableWebBasedSignin()) {
+    error_ = "signin_is_disallowed";
+    SendResponse(false);
+    return false;
+  }
+
+  // If the user is already signed in, there's nothing else to do.
+  if (!signin_manager_->GetAuthenticatedUsername().empty()) {
+    error_ = "user_is_already_signed_in";
+    SendResponse(false);
+    return false;
+  }
+
+  // If an authentication is currently in progress, wait for it to complete.
+  // TODO(isherman): Is this worth the added complexity, or would it be better
+  // to simply fail immediately and require the webstore to retry?  (Seems
+  // possibly tricky for the webstore to know when to retry.  I guess there's
+  // always the option of exponential backoff or something...)

[Ilya Sherman, 2014/03/14 05:42:56]

^^^

[guohui, 2014/03/14 22:09:44]

definitely better to wait for it to complete, as you said it is tricky for
webstore to know when to retry. and webstore probably should have a timeout on
their side to avoid waiting forever.

+  if (signin_manager_->AuthInProgress()) {
+    // TODO(isherman): Should we send a message to the page immediately to
+    // indicate that we're waiting?  (I'd prefer not to, since the API call is
+    // already asynchronous... but if it's possible that the reply will be
+    // significantly delayed, then the extra message is probably needed.)

[Ilya Sherman, 2014/03/14 05:42:56]

^^^

[guohui, 2014/03/14 22:09:44]

hmm i think webstore could have a timeout on their side, since this should be an
edge case, i think it is worth to send another reply.

+    SigninManagerFactory::GetInstance()->AddObserver(this);
+    signin_tracker_.reset(new SigninTracker(GetProfile(), this));
+    AddRef();  // Balanced in the sign-in observer methods below.
+    return true;
+  }
+
+  GURL signin_url =
+      signin::GetPromoURLWithContinueURL(signin::SOURCE_WEBSTORE_INSTALL,
+                                         false /* auto_close */,
+                                         false /* is_constrained */,
+                                         continue_url);
+  web_contents->GetController().LoadURL(signin_url,
+                                        content::Referrer(),
+                                        content::PAGE_TRANSITION_AUTO_TOPLEVEL,
+                                        std::string());
+
+  SendResponse(true);
+  return true;
+}
+
+void WebstorePrivateSignInFunction::SigninManagerShutdown(
+    SigninManagerBase* manager) {
+  SigninManagerFactory::GetInstance()->RemoveObserver(this);
+  // TODO(isherman): Probably a good idea to move these three lines into a
+  // shared function.  Holding off on that for now in case we actually want to
+  // send different error strings depending on the exact codepath hit or
+  // anything like that.

[Ilya Sherman, 2014/03/14 05:42:56]

Do you think we should try to have a smaller set of error strings that might be
less descriptive than possible in some cases; or would it be better to have a
distinct error string for each possible code path?

[guohui, 2014/03/14 22:09:44]

i think it is better to have a smaller set of error strings, since webstore has
to duplicate them in their code, it would be hard to process and to maintain a
large set of errors in two different code bases.

+  error_ = "signin_failed";
+  SendResponse(false);
+  Release();  // Balanced in RunImpl().
+}
+
+void WebstorePrivateSignInFunction::SigninFailed(
+    const GoogleServiceAuthError& error) {
+  // TODO(isherman): Would it be helpful to navigate to a sign-in page at this
+  // point, or is it better to have the Webstore re-issue the request if
+  // desired?

[Ilya Sherman, 2014/03/14 05:42:56]

^^^

[guohui, 2014/03/14 22:09:44]

i don't have a strong preference. Since this is an edge case of an edge case,
you may just return error for simplicity.

+  error_ = "signin_failed";
+  SendResponse(false);
+  Release();  // Balanced in RunImpl().
+}
+
+// TODO(isherman): Is this right?  Specifically:
+//   (a) Do we need to wait for the session to be merged, or are we ready to
+//       continue as soon as we get this sign-in success?
+//   (b) Is a sign-in success guaranteed to be followed by a merge session
+//       complete?  If not, is there a way to listen for a failure?

[Ilya Sherman, 2014/03/14 05:42:56]

^^^

[guohui, 2014/03/14 22:09:44]

yes we should wait for mergeSession to complete otherwise the gaia cookie would
not be set, and yes signinSuccess is guaranteed to be followed by a merge
session complete.

+void WebstorePrivateSignInFunction::SigninSuccess() {
+  // Nothing to do in this case. Keep waiting until MergeSessionComplete() is
+  // called.
+}
+
+void WebstorePrivateSignInFunction::MergeSessionComplete(
+    const GoogleServiceAuthError& error) {
+  // TODO(isherman): Am I handling the |error| parameter correctly?  Why can
+  // merging the session fail?

[Ilya Sherman, 2014/03/14 05:42:56]

^^^

[guohui, 2014/03/14 22:09:44]

you may return a specific error 'merge_session_failed', which means Chrome
signin is successful but gaia cookie in the content area is not updated
properly, and webstore could then navigate to the gaia webpage or show an error
accordingly.

+  if (error.state() == GoogleServiceAuthError::NONE) {
+    SendResponse(true);
+  } else {
+    error_ = "signin_failed";
+    SendResponse(false);
+  }
+  Release();  // Balanced in RunImpl().
+}
+
 }  // namespace extensions