Adding parameter to GetStatusLabels to indicate if links are acceptable, as some platforms cannot handle links in all the places these messages are shown.

Also adding unit tests for other functionality of GetStatusLabels.

BUG=none, discussed beforehand with akalin
TEST=HtmlNotIncludedInStatusIfNotRequested


Review URL: http://codereview.chromium.org/8383036

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

7 files changed, 253 insertions, 23 deletions

diff --git a/chrome/browser/sync/profile_sync_service.cc b/chrome/browser/sync/profile_sync_service.cc
index 3e9b99e..9ceef3f 100644
--- a/chrome/browser/sync/profile_sync_service.cc
+++ b/chrome/browser/sync/profile_sync_service.cc
@@ -996,6 +996,10 @@ SyncBackendHost::Status ProfileSyncService::QueryDetailedSyncStatus() {
   }
 }
 
+const GoogleServiceAuthError& ProfileSyncService::GetAuthError() const {
+  return last_auth_error_;
+}
+
 bool ProfileSyncService::SetupInProgress() const {
   return !HasSyncSetupCompleted() && WizardIsVisible();
 }
@@ -1023,6 +1027,10 @@ bool ProfileSyncService::unrecoverable_error_detected() const {
   return unrecoverable_error_detected_;
 }
 
+bool ProfileSyncService::UIShouldDepictAuthInProgress() const {
+  return is_auth_in_progress_;
+}
+
 bool ProfileSyncService::IsPassphraseRequired() const {
   return passphrase_required_reason_ !=
       sync_api::REASON_PASSPHRASE_NOT_REQUIRED;
diff --git a/chrome/browser/sync/profile_sync_service.h b/chrome/browser/sync/profile_sync_service.h
index 4854e06b..d87e796 100644
--- a/chrome/browser/sync/profile_sync_service.h
+++ b/chrome/browser/sync/profile_sync_service.h
@@ -247,9 +247,7 @@ class ProfileSyncService : public browser_sync::SyncFrontend,
   browser_sync::SyncBackendHost::StatusSummary QuerySyncStatusSummary();
   virtual browser_sync::SyncBackendHost::Status QueryDetailedSyncStatus();
 
-  const GoogleServiceAuthError& GetAuthError() const {
-    return last_auth_error_;
-  }
+  virtual const GoogleServiceAuthError& GetAuthError() const;
 
   // Displays a dialog for the user to enter GAIA credentials and attempt
   // re-authentication, and returns true if it actually opened the dialog.
@@ -301,9 +299,7 @@ class ProfileSyncService : public browser_sync::SyncFrontend,
     return unrecoverable_error_location_;
   }
 
-  bool UIShouldDepictAuthInProgress() const {
-    return is_auth_in_progress_;
-  }
+  virtual bool UIShouldDepictAuthInProgress() const;
 
   // Returns true if OnPassphraseRequired has been called for any reason.
   virtual bool IsPassphraseRequired() const;
diff --git a/chrome/browser/sync/profile_sync_service_mock.h b/chrome/browser/sync/profile_sync_service_mock.h
index 2c39061..0868f5c 100644
--- a/chrome/browser/sync/profile_sync_service_mock.h
+++ b/chrome/browser/sync/profile_sync_service_mock.h
@@ -15,6 +15,7 @@
 #include "chrome/browser/sync/profile_sync_service.h"
 #include "chrome/browser/sync/protocol/sync_protocol_error.h"
 #include "chrome/browser/sync/syncable/model_type.h"
+#include "chrome/common/net/gaia/google_service_auth_error.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
 class ProfileSyncServiceMock : public ProfileSyncService {
@@ -59,8 +60,11 @@ class ProfileSyncServiceMock : public ProfileSyncService {
   MOCK_CONST_METHOD0(GetLastSessionSnapshot,
                      const browser_sync::sessions::SyncSessionSnapshot*());
 
+  MOCK_CONST_METHOD0(UIShouldDepictAuthInProgress, bool());
   MOCK_METHOD0(QueryDetailedSyncStatus,
                browser_sync::SyncBackendHost::Status());
+  MOCK_CONST_METHOD0(GetAuthError, const GoogleServiceAuthError&());
+  MOCK_CONST_METHOD0(SetupInProgress, bool());
   MOCK_CONST_METHOD0(GetLastSyncedTimeString, string16());
   MOCK_CONST_METHOD0(unrecoverable_error_detected, bool());
   MOCK_METHOD1(OnActionableError, void(
diff --git a/chrome/browser/sync/sync_global_error_unittest.cc b/chrome/browser/sync/sync_global_error_unittest.cc
index 0215151..53374b2 100644
--- a/chrome/browser/sync/sync_global_error_unittest.cc
+++ b/chrome/browser/sync/sync_global_error_unittest.cc
@@ -13,6 +13,7 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 using ::testing::Return;
+using ::testing::ReturnRef;
 using ::testing::NiceMock;
 using content::BrowserThread;
 
@@ -25,9 +26,6 @@ void VerifySyncGlobalErrorResult(NiceMock<ProfileSyncServiceMock>* service,
                                  GoogleServiceAuthError::State error_state,
                                  bool is_signed_in,
                                  bool is_error) {
-  GoogleServiceAuthError auth_error(error_state);
-  service->UpdateAuthErrorState(auth_error);
-
   EXPECT_CALL(*service, HasSyncSetupCompleted())
               .WillRepeatedly(Return(is_signed_in));
   if (error_state == GoogleServiceAuthError::SERVICE_UNAVAILABLE) {
@@ -38,6 +36,9 @@ void VerifySyncGlobalErrorResult(NiceMock<ProfileSyncServiceMock>* service,
                 .WillRepeatedly(Return(UTF8ToUTF16("foo")));
   }
 
+  GoogleServiceAuthError auth_error(error_state);
+  EXPECT_CALL(*service, GetAuthError()).WillRepeatedly(ReturnRef(auth_error));
+
   error->OnStateChanged();
 
   // If there is an error then a wrench button badge, menu item, and bubble view
diff --git a/chrome/browser/sync/sync_ui_util.cc b/chrome/browser/sync/sync_ui_util.cc
index 2bfd6c6..39c3ea2 100644
--- a/chrome/browser/sync/sync_ui_util.cc
+++ b/chrome/browser/sync/sync_ui_util.cc
@@ -132,16 +132,28 @@ void GetStatusLabelsForAuthError(const AuthError& auth_error,
 // Returns the message that should be displayed when the user is authenticated
 // and can connect to the sync server. If the user hasn't yet authenticated, an
 // empty string is returned.
-string16 GetSyncedStateStatusLabel(ProfileSyncService* service) {
+string16 GetSyncedStateStatusLabel(ProfileSyncService* service,
+                                   StatusLabelStyle style) {
   string16 label;
   string16 user_name(service->GetAuthenticatedUsername());
   if (user_name.empty())
     return label;
 
-  return l10n_util::GetStringFUTF16(
-      IDS_SYNC_ACCOUNT_SYNCING_TO_USER,
-      user_name,
-      ASCIIToUTF16(chrome::kSyncGoogleDashboardURL));
+  // Message may also carry additional advice with an HTML link, if acceptable.
+  switch (style) {
+    case PLAIN_TEXT:
+      return l10n_util::GetStringFUTF16(
+          IDS_SYNC_ACCOUNT_SYNCING_TO_USER,
+          user_name);
+    case WITH_HTML:
+      return l10n_util::GetStringFUTF16(
+          IDS_SYNC_ACCOUNT_SYNCING_TO_USER_WITH_MANAGE_LINK,
+          user_name,
+          ASCIIToUTF16(chrome::kSyncGoogleDashboardURL));
+    default:
+      NOTREACHED();
+      return NULL;
+  }
 }
 
 void GetStatusForActionableError(
@@ -175,6 +187,7 @@ void GetStatusForActionableError(
 
 // status_label and link_label must either be both NULL or both non-NULL.
 MessageType GetStatusInfo(ProfileSyncService* service,
+                          StatusLabelStyle style,
                           string16* status_label,
                           string16* link_label) {
   DCHECK_EQ(status_label == NULL, link_label == NULL);
@@ -237,7 +250,7 @@ MessageType GetStatusInfo(ProfileSyncService* service,
         // current synced status.  Return SYNC_PROMO so that
         // the configure link will still be shown.
         if (status_label && link_label) {
-          status_label->assign(GetSyncedStateStatusLabel(service));
+          status_label->assign(GetSyncedStateStatusLabel(service, style));
           link_label->assign(
               l10n_util::GetStringUTF16(IDS_SYNC_PASSWORD_SYNC_ATTENTION));
         }
@@ -247,7 +260,7 @@ MessageType GetStatusInfo(ProfileSyncService* service,
 
     // There is no error. Display "Last synced..." message.
     if (status_label)
-      status_label->assign(GetSyncedStateStatusLabel(service));
+      status_label->assign(GetSyncedStateStatusLabel(service, style));
     return SYNCED;
   } else {
     // Either show auth error information with a link to re-login, auth in prog,
@@ -329,17 +342,18 @@ MessageType GetStatusInfoForNewTabPage(ProfileSyncService* service,
   }
 
   // Fallback to default.
-  return GetStatusInfo(service, status_label, link_label);
+  return GetStatusInfo(service, WITH_HTML, status_label, link_label);
 }
 
 }  // namespace
 
 MessageType GetStatusLabels(ProfileSyncService* service,
+                            StatusLabelStyle style,
                             string16* status_label,
                             string16* link_label) {
   DCHECK(status_label);
   DCHECK(link_label);
-  return sync_ui_util::GetStatusInfo(service, status_label, link_label);
+  return sync_ui_util::GetStatusInfo(service, style, status_label, link_label);
 }
 
 MessageType GetStatusLabelsForNewTabPage(ProfileSyncService* service,
@@ -390,7 +404,7 @@ void GetStatusLabelsForSyncGlobalError(ProfileSyncService* service,
 }
 
 MessageType GetStatus(ProfileSyncService* service) {
-  return sync_ui_util::GetStatusInfo(service, NULL, NULL);
+  return sync_ui_util::GetStatusInfo(service, WITH_HTML, NULL, NULL);
 }
 
 string16 GetSyncMenuLabel(ProfileSyncService* service) {
diff --git a/chrome/browser/sync/sync_ui_util.h b/chrome/browser/sync/sync_ui_util.h
index 3498a63..a729ebf 100644
--- a/chrome/browser/sync/sync_ui_util.h
+++ b/chrome/browser/sync/sync_ui_util.h
@@ -32,13 +32,19 @@ enum MessageType {
                // attention, but not as an error.
 };
 
+enum StatusLabelStyle {
+  PLAIN_TEXT,  // Label will be plain-text only.
+  WITH_HTML    // Label may contain an HTML-formatted link.
+};
+
 // TODO(akalin): audit the use of ProfileSyncService* service below,
 // and use const ProfileSyncService& service where possible.
 
 // Create status and link labels for the current status labels and link text
 // by querying |service|.
-// |status_label| may contain an HTML-formatted link.
+// |style| sets the link properties, see |StatusLabelStyle|.
 MessageType GetStatusLabels(ProfileSyncService* service,
+                            StatusLabelStyle style,
                             string16* status_label,
                             string16* link_label);
 
diff --git a/chrome/browser/sync/sync_ui_util_unittest.cc b/chrome/browser/sync/sync_ui_util_unittest.cc
index b392ed6..8fba233 100644
--- a/chrome/browser/sync/sync_ui_util_unittest.cc
+++ b/chrome/browser/sync/sync_ui_util_unittest.cc
@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <set>
 #include "base/basictypes.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/sync/profile_sync_service_mock.h"
@@ -12,9 +13,23 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 using ::testing::Return;
+using ::testing::ReturnRef;
 using ::testing::NiceMock;
 using content::BrowserThread;
 
+// A number of distinct states of the ProfileSyncService can be generated for
+// tests.
+enum DistinctState {
+  STATUS_CASE_SETUP_IN_PROGRESS,
+  STATUS_CASE_SETUP_ERROR,
+  STATUS_CASE_AUTHENTICATING,
+  STATUS_CASE_AUTH_ERROR,
+  STATUS_CASE_PROTOCOL_ERROR,
+  STATUS_CASE_PASSPHRASE_ERROR,
+  STATUS_CASE_SYNCED,
+  NUMBER_OF_STATUS_CASES
+};
+
 namespace {
 
 // Utility function to test that GetStatusLabelsForSyncGlobalError returns
@@ -23,11 +38,9 @@ void VerifySyncGlobalErrorResult(NiceMock<ProfileSyncServiceMock>* service,
                                  GoogleServiceAuthError::State error_state,
                                  bool is_signed_in,
                                  bool is_error) {
-  GoogleServiceAuthError auth_error(error_state);
-  service->UpdateAuthErrorState(auth_error);
-
   EXPECT_CALL(*service, HasSyncSetupCompleted())
               .WillRepeatedly(Return(is_signed_in));
+
   if (error_state == GoogleServiceAuthError::SERVICE_UNAVAILABLE) {
     EXPECT_CALL(*service, GetAuthenticatedUsername())
                 .WillRepeatedly(Return(UTF8ToUTF16("")));
@@ -36,6 +49,9 @@ void VerifySyncGlobalErrorResult(NiceMock<ProfileSyncServiceMock>* service,
                 .WillRepeatedly(Return(UTF8ToUTF16("foo")));
   }
 
+  GoogleServiceAuthError auth_error(error_state);
+  EXPECT_CALL(*service, GetAuthError()).WillRepeatedly(ReturnRef(auth_error));
+
   string16 label1, label2, label3;
   sync_ui_util::GetStatusLabelsForSyncGlobalError(
       service, &label1, &label2, &label3);
@@ -63,6 +79,9 @@ TEST(SyncUIUtilTest, ConstructAboutInformationWithUnrecoverableErrorTest) {
   EXPECT_CALL(service, QueryDetailedSyncStatus())
               .WillOnce(Return(status));
 
+  GoogleServiceAuthError auth_error(GoogleServiceAuthError::NONE);
+  EXPECT_CALL(service, GetAuthError()).WillOnce(ReturnRef(auth_error));
+
   EXPECT_CALL(service, unrecoverable_error_detected())
              .WillOnce(Return(true));
 
@@ -125,3 +144,185 @@ TEST(SyncUIUtilTest, AuthStateGlobalError) {
         &service, table[i].error_state, false, false);
   }
 }
+// Loads a ProfileSyncServiceMock to emulate one of a number of distinct cases
+// in order to perform tests on the generated messages.
+void GetDistinctCase(ProfileSyncServiceMock& service,
+                     GoogleServiceAuthError** auth_error,
+                     int caseNumber) {
+  // Auth Error object is returned by reference in mock and needs to stay in
+  // scope throughout test, so it is owned by calling method. However it is
+  // immutable so can only be allocated in this method.
+  switch (caseNumber) {
+    case STATUS_CASE_SETUP_IN_PROGRESS: {
+      EXPECT_CALL(service, HasSyncSetupCompleted())
+                  .WillOnce(Return(false));
+      EXPECT_CALL(service, SetupInProgress())
+                  .WillOnce(Return(true));
+      browser_sync::SyncBackendHost::Status status;
+      status.summary = browser_sync::SyncBackendHost::Status::READY;
+      EXPECT_CALL(service, QueryDetailedSyncStatus())
+                  .WillOnce(Return(status));
+      *auth_error = new GoogleServiceAuthError(GoogleServiceAuthError::NONE);
+      EXPECT_CALL(service, GetAuthError())
+                  .WillOnce(ReturnRef(**auth_error));
+      EXPECT_CALL(service, UIShouldDepictAuthInProgress())
+                  .WillOnce(Return(false));
+      return;
+    }
+   case STATUS_CASE_SETUP_ERROR: {
+      EXPECT_CALL(service, HasSyncSetupCompleted())
+                  .WillOnce(Return(false));
+      EXPECT_CALL(service, SetupInProgress())
+                  .WillOnce(Return(false));
+      EXPECT_CALL(service, unrecoverable_error_detected())
+                  .WillOnce(Return(true));
+      browser_sync::SyncBackendHost::Status status;
+      status.summary = browser_sync::SyncBackendHost::Status::READY;
+      EXPECT_CALL(service, QueryDetailedSyncStatus())
+                  .WillOnce(Return(status));
+      return;
+    }
+    case STATUS_CASE_AUTHENTICATING: {
+      EXPECT_CALL(service, HasSyncSetupCompleted())
+                  .WillOnce(Return(true));
+      browser_sync::SyncBackendHost::Status status;
+      status.summary = browser_sync::SyncBackendHost::Status::READY;
+      EXPECT_CALL(service, QueryDetailedSyncStatus())
+                  .WillOnce(Return(status));
+      EXPECT_CALL(service, UIShouldDepictAuthInProgress())
+                  .WillOnce(Return(true));
+      *auth_error = new GoogleServiceAuthError(GoogleServiceAuthError::NONE);
+      EXPECT_CALL(service, GetAuthError())
+                  .WillOnce(ReturnRef(**auth_error));
+      return;
+    }
+    case STATUS_CASE_AUTH_ERROR: {
+      EXPECT_CALL(service, HasSyncSetupCompleted())
+                  .WillOnce(Return(true));
+      browser_sync::SyncBackendHost::Status status;
+      status.summary = browser_sync::SyncBackendHost::Status::READY;
+      EXPECT_CALL(service, QueryDetailedSyncStatus())
+                  .WillOnce(Return(status));
+      *auth_error = new GoogleServiceAuthError(
+         GoogleServiceAuthError::SERVICE_UNAVAILABLE);
+      EXPECT_CALL(service, GetAuthenticatedUsername())
+                  .WillOnce(Return(ASCIIToUTF16("")));
+      EXPECT_CALL(service, GetAuthError())
+                  .WillOnce(ReturnRef(**auth_error));
+      EXPECT_CALL(service, UIShouldDepictAuthInProgress())
+                  .WillOnce(Return(false));
+      return;
+    }
+    case STATUS_CASE_PROTOCOL_ERROR: {
+      EXPECT_CALL(service, HasSyncSetupCompleted())
+                  .WillOnce(Return(true));
+      browser_sync::SyncProtocolError protocolError;
+      protocolError.action = browser_sync::STOP_AND_RESTART_SYNC;
+      browser_sync::SyncBackendHost::Status status;
+      status.summary = browser_sync::SyncBackendHost::Status::READY;
+      status.sync_protocol_error = protocolError;
+      EXPECT_CALL(service, QueryDetailedSyncStatus())
+                  .WillOnce(Return(status));
+      *auth_error = new GoogleServiceAuthError(GoogleServiceAuthError::NONE);
+      EXPECT_CALL(service, GetAuthError())
+                  .WillOnce(ReturnRef(**auth_error));
+      EXPECT_CALL(service, UIShouldDepictAuthInProgress())
+                  .WillOnce(Return(false));
+      return;
+    }
+    case STATUS_CASE_PASSPHRASE_ERROR: {
+      EXPECT_CALL(service, HasSyncSetupCompleted())
+                  .WillOnce(Return(true));
+      browser_sync::SyncBackendHost::Status status;
+      status.summary = browser_sync::SyncBackendHost::Status::READY;
+      EXPECT_CALL(service, QueryDetailedSyncStatus())
+                  .WillOnce(Return(status));
+      *auth_error = new GoogleServiceAuthError(GoogleServiceAuthError::NONE);
+      EXPECT_CALL(service, GetAuthError())
+                  .WillOnce(ReturnRef(**auth_error));
+      EXPECT_CALL(service, GetAuthenticatedUsername())
+                  .WillOnce(Return(ASCIIToUTF16("example@example.com")));
+      EXPECT_CALL(service, UIShouldDepictAuthInProgress())
+                  .WillOnce(Return(false));
+      EXPECT_CALL(service, IsPassphraseRequired())
+                  .WillOnce(Return(true));
+      EXPECT_CALL(service, IsPassphraseRequiredForDecryption())
+                  .WillOnce(Return(true));
+      return;
+    }
+    case STATUS_CASE_SYNCED: {
+      EXPECT_CALL(service, HasSyncSetupCompleted())
+              .WillOnce(Return(true));
+      browser_sync::SyncBackendHost::Status status;
+      status.summary = browser_sync::SyncBackendHost::Status::READY;
+      EXPECT_CALL(service, QueryDetailedSyncStatus())
+                  .WillOnce(Return(status));
+      *auth_error = new GoogleServiceAuthError(GoogleServiceAuthError::NONE);
+      EXPECT_CALL(service, GetAuthError())
+                  .WillOnce(ReturnRef(**auth_error));
+      EXPECT_CALL(service, GetAuthenticatedUsername())
+                  .WillOnce(Return(ASCIIToUTF16("example@example.com")));
+      EXPECT_CALL(service, UIShouldDepictAuthInProgress())
+                  .WillOnce(Return(false));
+      EXPECT_CALL(service, IsPassphraseRequired())
+                  .WillOnce(Return(false));
+      return;
+    }
+    default:
+      NOTREACHED();
+  }
+}
+
+// This test ensures that a each distinctive ProfileSyncService statuses
+// will return a unique combination of status and link messages from
+// GetStatusLabels().
+TEST(SyncUIUtilTest, DistinctCasesReportUniqueMessageSets) {
+  ProfileSyncServiceMock service;
+
+  std::set<string16> messages;
+  for (int idx = 0; idx != NUMBER_OF_STATUS_CASES; idx++) {
+    ProfileSyncServiceMock service;
+    GoogleServiceAuthError* auth_error = NULL;
+    GetDistinctCase(service, &auth_error, idx);
+    string16 status_label;
+    string16 link_label;
+    sync_ui_util::GetStatusLabels(&service,
+                                  sync_ui_util::WITH_HTML,
+                                  &status_label,
+                                  &link_label);
+    // If the status and link message combination is already present in the set
+    // of messages already seen, this is a duplicate rather than a unique
+    // message, and the test has failed.
+    string16 combined_label =
+        status_label + string16(ASCIIToUTF16("#")) + link_label;
+    EXPECT_TRUE(messages.find(combined_label) == messages.end());
+    messages.insert(combined_label);
+    if (auth_error)
+      delete auth_error;
+  }
+}
+
+// This test ensures that the html_links parameter on GetStatusLabels() is
+// honored.
+TEST(SyncUIUtilTest, HtmlNotIncludedInStatusIfNotRequested) {
+  ProfileSyncServiceMock service;
+  for (int idx = 0; idx != NUMBER_OF_STATUS_CASES; idx++) {
+    ProfileSyncServiceMock service;
+    GoogleServiceAuthError* auth_error = NULL;
+    GetDistinctCase(service, &auth_error, idx);
+    string16 status_label;
+    string16 link_label;
+    sync_ui_util::GetStatusLabels(&service,
+                                  sync_ui_util::PLAIN_TEXT,
+                                  &status_label,
+                                  &link_label);
+    // Ensures a search for string 'href' (found in links, not a string to be
+    // found in an English language message) fails when links are excluded from
+    // the status label.
+    EXPECT_EQ(status_label.find(string16(ASCIIToUTF16("href"))),
+              string16::npos);
+    if (auth_error) {
+      delete auth_error;
+    }
+  }
+}