[Password Manager] Fix crash in reporting sync stats.

chrome/browser/password_manager/sync_metrics.cc

Index: chrome/browser/password_manager/sync_metrics.cc
diff --git a/chrome/browser/password_manager/sync_metrics.cc b/chrome/browser/password_manager/sync_metrics.cc
index 43520536e3beba225f80f37151a74a8b22b1e95b..4eddf427ccd22db327b2fa421aaee66bd509056d 100644
--- a/chrome/browser/password_manager/sync_metrics.cc
+++ b/chrome/browser/password_manager/sync_metrics.cc
@@ -13,14 +13,7 @@
 
 namespace password_manager_sync_metrics {
 
-std::string GetPasswordSyncUsername(Profile* profile) {
-  ProfileSyncService* sync_service =
-      ProfileSyncServiceFactory::GetForProfile(profile);
-  if (!sync_service ||
-      !sync_service->HasSyncSetupCompleted() ||
-      !sync_service->GetActiveDataTypes().Has(syncer::PASSWORDS))
-    return "";

[Ilya Sherman, 2014/07/15 01:09:11]

Is this the code that you're saying causes Sync to be reinitialized?  If so, how
so?  (Also, the CL description says that the method name is
IsPasswordSyncUsername(), which doesn't seem to be one of the existing method
names... or maybe I'm just overlooking it.)

[Garrett Casto, 2014/07/15 05:39:03]

ProfileSyncServiceFactory::GetForProfile() will create a a ProfileSyncService if
one isn't associated with the given profile. As part of this process,
PasswordStoreFactory::GetForProfile() is called (see
SyncRegistrar::SyncRegistrar()). Creating a new PasswordStore will call this
function and we go around until the stack blows up. 
And the function name is a typo in the CL description. It should be
GetPasswordSyncUsername().

[Ilya Sherman, 2014/07/15 05:48:13]

Can you use ProfileSyncServiceFactory::HasProfileSyncService()?

[Garrett Casto, 2014/07/15 06:05:55]

I could. It just wasn't clearly useful to me, as I mentioned in the other
comment. That is, either this intermittently allows us to check if we are
syncing passwords, which seems like it may skew the data, or it's never possible
to check once password syncing start. I should verify tomorrow which one it is,
but it doesn't seem like in either scenario it's worth trying to extract this
information.

-
+std::string GetSyncUsername(Profile* profile) {
   SigninManagerBase* signin_manager =
       SigninManagerFactory::GetForProfile(profile);
   if (!signin_manager)
@@ -29,13 +22,13 @@ std::string GetPasswordSyncUsername(Profile* profile) {
   return signin_manager->GetAuthenticatedUsername();
 }
 
-bool IsPasswordSyncAccountCredential(Profile* profile,
-                                     const std::string& username,
-                                     const std::string& origin) {
+bool IsSyncAccountCredential(Profile* profile,
+                             const std::string& username,
+                             const std::string& origin) {
   if (origin != GaiaUrls::GetInstance()->gaia_url().GetOrigin().spec())
     return false;
 
-  return gaia::AreEmailsSame(username, GetPasswordSyncUsername(profile));
+  return gaia::AreEmailsSame(username, GetSyncUsername(profile));
 }
 
 }  // namespace password_manager_sync_metrics