Fix the app notify channel setup for the case where the user may have...

Fix the app notify channel setup for the case where the user may have explicitly revoked permission and hence the OAuth2
access token generation may fail. In that case, prompt the user.
add unit tests and modify existing tests.

TBR=asargent
Review URL: http://codereview.chromium.org/8822006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@113164 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/extensions/app_notify_channel_setup.cc

@@ -81,7 +81,8 @@ AppNotifyChannelSetup::AppNotifyChannelSetup(
       callback_id_(callback_id),
       delegate_(delegate),
       ui_(ui),
-      state_(INITIAL) {}
+      state_(INITIAL),
+      oauth2_access_token_failure_(false) {}
 
 AppNotifyChannelSetup::~AppNotifyChannelSetup() {}
 
@@ -147,12 +148,17 @@ URLFetcher* AppNotifyChannelSetup::CreateURLFetcher(
 bool AppNotifyChannelSetup::ShouldPromptForLogin() const {
   std::string username = profile_->GetPrefs()->GetString(
       prefs::kGoogleServicesUsername);
-  // Prompt for login if either the user has not logged in at all or
-  // if the user is logged in but there is no OAuth2 login token.
-  // The latter happens for users who are already logged in before the
-  // code to generate OAuth2 login token is released.
+  // Prompt for login if either:
+  // 1. the user has not logged in at all or
+  // 2. if the user is logged in but there is no OAuth2 login token.
+  //    The latter happens for users who are already logged in before the
+  //    code to generate OAuth2 login token is released.
+  // 3. If the OAuth2 login token does not work anymore.
+  //    This can happen if the user explicitly revoked access to Google Chrome
+  //    from Google Accounts page.
   return username.empty() ||
-         !profile_->GetTokenService()->HasOAuthLoginToken();
+         !profile_->GetTokenService()->HasOAuthLoginToken() ||
+         oauth2_access_token_failure_;
 }
 
 void AppNotifyChannelSetup::BeginLogin() {
@@ -198,6 +204,16 @@ void AppNotifyChannelSetup::EndGetAccessToken(bool success) {
   if (success) {
     state_ = FETCH_ACCESS_TOKEN_DONE;
     BeginRecordGrant();
+  } else if (!oauth2_access_token_failure_) {
+    oauth2_access_token_failure_ = true;
+    // If access token generation fails, then it means somehow the
+    // OAuth2 login scoped token became invalid. One way this cna happen
+    // is if a user explicitly revoked access to Google Chrome from
+    // Google Accounts page. In such a case, we should try to show the
+    // login setup again to the user, but only if we have not already
+    // done so once (to avoid infinite loop).
+    state_ = INITIAL;
+    BeginLogin();
   } else {
     state_ = ERROR_STATE;
     ReportResult("", kChannelSetupInternalError);

chrome/browser/extensions/app_notify_channel_setup.h

@@ -149,6 +149,11 @@ class AppNotifyChannelSetup
   scoped_ptr<AppNotifyChannelUI> ui_;
   State state_;
   std::string oauth2_access_token_;
+  // Keeps track of whether we have encountered failure in OAuth2 access
+  // token generation already. We use this to prevent us from doing an
+  // infinite loop of trying to generate access token, if that fails, try
+  // to login the user and generate access token, etc.
+  bool oauth2_access_token_failure_;
 
   DISALLOW_COPY_AND_ASSIGN(AppNotifyChannelSetup);
 };

chrome/browser/extensions/app_notify_channel_setup_unittest.cc

@@ -182,13 +182,13 @@ class AppNotifyChannelSetupTest : public testing::Test {
                                      delegate_.AsWeakPtr());
   }
 
-  virtual void SetupLogin(bool should_succeed) {
+  virtual void SetupLogin(bool should_prompt, bool should_succeed) {
     if (should_succeed) {
       SetLoggedInUser("user@gmail.com");
       profile_.SetTokenServiceHasTokenResult(true);
-    } else {
-      ui_->SetSyncSetupResult(false);
     }
+    if (should_prompt)
+      ui_->SetSyncSetupResult(should_succeed);
   }
 
   virtual void SetupFetchAccessToken(bool should_succeed) {
@@ -230,14 +230,16 @@ class AppNotifyChannelSetupTest : public testing::Test {
 };
 
 TEST_F(AppNotifyChannelSetupTest, LoginFailure) {
-  SetupLogin(false);
+  SetupLogin(true, false);
 
   scoped_refptr<AppNotifyChannelSetup> setup = CreateInstance();
   RunServerTest(setup, "", "canceled_by_user");
 }
 
-TEST_F(AppNotifyChannelSetupTest, FetchAccessTokenFailure) {
-  SetupLogin(true);
+TEST_F(AppNotifyChannelSetupTest, DoubleFetchAccessTokenFailure) {
+  SetupLogin(false, true);
+  SetupFetchAccessToken(false);
+  SetupLogin(true, true);
   SetupFetchAccessToken(false);
 
   scoped_refptr<AppNotifyChannelSetup> setup = CreateInstance();
@@ -245,7 +247,7 @@ TEST_F(AppNotifyChannelSetupTest, FetchAccessTokenFailure) {
 }
 
 TEST_F(AppNotifyChannelSetupTest, RecordGrantFailure) {
-  SetupLogin(true);
+  SetupLogin(false, true);
   SetupFetchAccessToken(true);
   SetupRecordGrant(false);
 
@@ -254,7 +256,7 @@ TEST_F(AppNotifyChannelSetupTest, RecordGrantFailure) {
 }
 
 TEST_F(AppNotifyChannelSetupTest, GetChannelIdFailure) {
-  SetupLogin(true);
+  SetupLogin(false, true);
   SetupFetchAccessToken(true);
   SetupRecordGrant(true);
   SetupGetChannelId(false);
@@ -263,8 +265,20 @@ TEST_F(AppNotifyChannelSetupTest, GetChannelIdFailure) {
   RunServerTest(setup, "", "internal_error");
 }
 
-TEST_F(AppNotifyChannelSetupTest, ServerSuccess) {
-  SetupLogin(true);
+TEST_F(AppNotifyChannelSetupTest, FirstFetchAccessTokenSuccess) {
+  SetupLogin(false, true);
+  SetupFetchAccessToken(true);
+  SetupRecordGrant(true);
+  SetupGetChannelId(true);
+
+  scoped_refptr<AppNotifyChannelSetup> setup = CreateInstance();
+  RunServerTest(setup, "dummy_do_not_use", "");
+}
+
+TEST_F(AppNotifyChannelSetupTest, SecondFetchAccessTokenSuccess) {
+  SetupLogin(false, true);
+  SetupFetchAccessToken(false);
+  SetupLogin(true, true);
   SetupFetchAccessToken(true);
   SetupRecordGrant(true);
   SetupGetChannelId(true);