[sync] Clean up path generation for the local sync database.

chrome/browser/sync/profile_sync_service_factory.cc

Index: chrome/browser/sync/profile_sync_service_factory.cc
diff --git a/chrome/browser/sync/profile_sync_service_factory.cc b/chrome/browser/sync/profile_sync_service_factory.cc
index 468c41ea34161d54da9a13f80a640e06b8e94516..1d80ed35a45922425056f1d11547e729fe87810b 100644
--- a/chrome/browser/sync/profile_sync_service_factory.cc
+++ b/chrome/browser/sync/profile_sync_service_factory.cc
@@ -8,6 +8,9 @@
 
 #include "base/memory/ptr_util.h"
 #include "base/memory/singleton.h"
+#include "base/metrics/histogram_macros.h"
+#include "base/path_service.h"
+#include "base/syslog_logging.h"
 #include "base/threading/sequenced_worker_pool.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
@@ -32,6 +35,7 @@
 #include "chrome/browser/themes/theme_service_factory.h"
 #include "chrome/browser/web_data_service_factory.h"
 #include "chrome/common/channel_info.h"
+#include "chrome/common/chrome_paths.h"
 #include "components/browser_sync/profile_sync_components_factory_impl.h"
 #include "components/browser_sync/profile_sync_service.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
@@ -167,6 +171,17 @@ KeyedService* ProfileSyncServiceFactory::BuildServiceInstanceFor(
 #if defined(OS_WIN)
   syncer::SyncPrefs prefs(profile->GetPrefs());
   local_sync_backend_enabled = prefs.IsLocalSyncEnabled();
+  init_params.local_sync_backend_folder = prefs.GetLocalSyncBackendDir();
+  if (init_params.local_sync_backend_folder.empty()) {
+    if (!base::PathService::Get(chrome::DIR_ROAMING_USER_DATA,
+                                &init_params.local_sync_backend_folder)) {
+      SYSLOG(WARNING) << "Local sync can not get the roaming profile folder.";

[Nicolas Zea, 2017/02/21 23:58:40]

Out of curiosity why SYSLOG instead of LOG?

[pastarmovj, 2017/02/22 14:23:14]

I think this is an actionable error and the admin needs to know about it. The
reason I decided to surface it because initially I had a CHECK around the Get
and it started crashing in the wild (see
https://bugs.chromium.org/p/chromium/issues/detail?id=687882 ). It turned out
that on some (virus infested) machines trying to get the roaming user directory
can fail. I decided that this is not a reason to crash Chrome but only to
disable local sync. However it is important info that can help the admin
diagnose and fix the issue. And they don't have to run with --enable-logging
which might not be possible on a user machine.

+      UMA_HISTOGRAM_BOOLEAN("Sync.Local.RoamingProfileUnavailable", false);

[Nicolas Zea, 2017/02/21 23:58:40]

It seems like this should probably be part of a larger histogram that just
captures roaming profile startup result, with this specific scenario being one
of several possible startup errors recorded.

As it is, it's not clear to me how useful this count would be with no point of
reference for the success.

[pastarmovj, 2017/02/22 14:23:13]

Sorry this one got broken when I was moving the code around. Of course it was
meant to measure success vs. failure.

+      local_sync_backend_enabled = false;

[Nicolas Zea, 2017/02/21 23:58:40]

Should we just return a nullptr here?

[pastarmovj, 2017/02/22 14:23:14]

Great idea! I forgot a service doesn't have to be created.

+    }
+  }
+  UMA_HISTOGRAM_BOOLEAN("Sync.Local.Enabled", local_sync_backend_enabled);
+
   if (local_sync_backend_enabled) {
     // This code as it is now will assume the same profile order is present on
     // all machines, which is not a given. It is to be defined if only the
@@ -174,7 +189,7 @@ KeyedService* ProfileSyncServiceFactory::BuildServiceInstanceFor(
     // now. The solution for now will be to assume profiles are created in the
     // same order on all machines and in the future decide if only the Default
     // one should be considered roamed.
-    init_params.local_sync_backend_folder = prefs.GetLocalSyncBackendDir();
+    // See http://crbug.com/674928.
     init_params.local_sync_backend_folder =
         init_params.local_sync_backend_folder.Append(
             init_params.base_directory.BaseName());