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

chrome/browser/sync/profile_sync_service_factory.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/sync/profile_sync_service_factory.h"
 
 #include <utility>
 
 #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"
 #include "chrome/browser/autofill/personal_data_manager_factory.h"
 #include "chrome/browser/bookmarks/bookmark_model_factory.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/defaults.h"
 #include "chrome/browser/gcm/gcm_profile_service_factory.h"
 #include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/invalidation/profile_invalidation_provider_factory.h"
 #include "chrome/browser/password_manager/password_store_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
 #include "chrome/browser/signin/about_signin_internals_factory.h"
 #include "chrome/browser/signin/chrome_signin_client_factory.h"
 #include "chrome/browser/signin/gaia_cookie_manager_service_factory.h"
 #include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
 #include "chrome/browser/sync/chrome_sync_client.h"
 #include "chrome/browser/sync/supervised_user_signin_manager_wrapper.h"
 #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"
 #include "components/network_time/network_time_tracker.h"
 #include "components/signin/core/browser/profile_oauth2_token_service.h"
 #include "components/signin/core/browser/signin_manager.h"
 #include "components/sync/driver/signin_manager_wrapper.h"
 #include "components/sync/driver/startup_controller.h"
 #include "components/sync/driver/sync_util.h"
 #include "content/public/browser/browser_thread.h"
@@ skipping 115 matching lines @@
           blocking_pool->GetSequenceToken(),
           base::SequencedWorkerPool::SKIP_ON_SHUTDOWN);
 
   bool local_sync_backend_enabled = false;
 
 // Since the local sync backend is currently only supported on Windows don't
 // even check the pref on other os-es.
 #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
     // Default profile should get this treatment or all profile as is the case
     // 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());
     init_params.local_sync_backend_folder =
         init_params.local_sync_backend_folder.Append(
             kLoopbackServerBackendFilename);
 
     init_params.start_behavior = ProfileSyncService::AUTO_START;
   }
 #endif  // defined(OS_WIN)
@@ skipping 49 matching lines @@
 
 // static
 void ProfileSyncServiceFactory::SetSyncClientFactoryForTest(
     SyncClientFactory* client_factory) {
   client_factory_ = client_factory;
 }
 
 // static
 ProfileSyncServiceFactory::SyncClientFactory*
     ProfileSyncServiceFactory::client_factory_ = nullptr;