Device info datatype should be moved to components/sync_driver.

This change prepares DeviceInfo class (and all other sync
classes that depend on DeviceInfo and implement DEVICE_INFO
syncable type) to be moved to components/sync_driver library
by removing any dependencies on chrome/browser.

To achieve this all chrome/browser specific code was moved to
LocalDeviceInfoProviderImpl which will stay in browser/sync/glue.
All other classes including DeviceInfo and LocalDeviceInfoProvider
should now depend only on base or components. I'll move
them to components/sync_driver in the next change.

Details:

1) In ui_thread_search_terms_data.cc DeviceInfo::GetLocalDeviceType()
was used just to find out if the device is an Android phone.
Since this code has nothing to do with Sync I replaced these
with direct calls to ui::GetDeviceFormFactor() - that exactly
what DeviceInfo::GetLocalDeviceType() does internally.

2) In sessions_sync_manager.cc - calling DeviceInfo::GetLocalDeviceType()
isn't necessary anymore because the local device type can be
accessed via the owned LocalDeviceInfoProvider instance.
This allowed me to move GetLocalDeviceType() to LocalDeviceInfoProviderImpl
and make it private.

3) DeviceInfo::MakeUserAgentForSyncApi() is called only
from the code that will stay in browser/sync/glue.
I moved the implemented to LocalDeviceInfoProviderImpl class.

BUG=396136

Review URL: https://codereview.chromium.org/594643003

Cr-Commit-Position: refs/heads/master@{#296267}

chrome/browser/search_engines/ui_thread_search_terms_data.cc

@@ -15,7 +15,6 @@
 #include "chrome/browser/search/instant_service.h"
 #include "chrome/browser/search/instant_service_factory.h"
 #include "chrome/browser/search/search.h"
-#include "chrome/browser/sync/glue/device_info.h"
 #include "chrome/browser/themes/theme_service.h"
 #include "chrome/browser/themes/theme_service_factory.h"
 #include "chrome/common/chrome_switches.h"
@@ -25,7 +24,7 @@
 #include "components/omnibox/omnibox_field_trial.h"
 #include "components/search/search.h"
 #include "content/public/browser/browser_thread.h"
-#include "sync/protocol/sync.pb.h"
+#include "ui/base/device_form_factor.h"
 #include "url/gurl.h"
 
 #if defined(ENABLE_RLZ)
@@ -103,10 +102,8 @@ std::string UIThreadSearchTermsData::GetSuggestClient() const {
   DCHECK(!BrowserThread::IsThreadInitialized(BrowserThread::UI) ||
       BrowserThread::CurrentlyOn(BrowserThread::UI));
 #if defined(OS_ANDROID)
-  sync_pb::SyncEnums::DeviceType device_type =
-      browser_sync::DeviceInfo::GetLocalDeviceType();
-  return device_type == sync_pb::SyncEnums_DeviceType_TYPE_PHONE ?
-    "chrome" : "chrome-omni";
+  return ui::GetDeviceFormFactor() == ui::DEVICE_FORM_FACTOR_PHONE ?
+      "chrome" : "chrome-omni";
 #else
   return chrome::IsInstantExtendedAPIEnabled() ? "chrome-omni" : "chrome";
 #endif
@@ -116,9 +113,7 @@ std::string UIThreadSearchTermsData::GetSuggestRequestIdentifier() const {
   DCHECK(!BrowserThread::IsThreadInitialized(BrowserThread::UI) ||
       BrowserThread::CurrentlyOn(BrowserThread::UI));
 #if defined(OS_ANDROID)
-  sync_pb::SyncEnums::DeviceType device_type =
-      browser_sync::DeviceInfo::GetLocalDeviceType();
-  if (device_type == sync_pb::SyncEnums_DeviceType_TYPE_PHONE) {
+  if (ui::GetDeviceFormFactor() == ui::DEVICE_FORM_FACTOR_PHONE) {
     return OmniboxFieldTrial::EnableAnswersInSuggest() ?
         "chrome-mobile-ext-ansg" : "chrome-mobile-ext";
   }

chrome/browser/sync/glue/device_info.cc

@@ -2,48 +2,13 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/sync/glue/device_info.h"
-
-#include "base/command_line.h"
-#include "base/threading/sequenced_worker_pool.h"
 #include "base/values.h"
-#include "chrome/common/chrome_version_info.h"
-#include "chrome/common/chrome_switches.h"
+#include "chrome/browser/sync/glue/device_info.h"
 #include "content/public/browser/browser_thread.h"
 #include "sync/util/get_session_name.h"
-#include "ui/base/device_form_factor.h"
 
 namespace browser_sync {
 
-namespace {
-
-#if defined(OS_ANDROID)
-bool IsTabletUI() {
-  return ui::GetDeviceFormFactor() == ui::DEVICE_FORM_FACTOR_TABLET;
-}
-#endif
-
-// Converts VersionInfo::Channel to string for user-agent string.
-std::string ChannelToString(chrome::VersionInfo::Channel channel) {
-  switch (channel) {
-    case chrome::VersionInfo::CHANNEL_UNKNOWN:
-      return "unknown";
-    case chrome::VersionInfo::CHANNEL_CANARY:
-      return "canary";
-    case chrome::VersionInfo::CHANNEL_DEV:
-      return "dev";
-    case chrome::VersionInfo::CHANNEL_BETA:
-      return "beta";
-    case chrome::VersionInfo::CHANNEL_STABLE:
-      return "stable";
-    default:
-      NOTREACHED();
-      return "unknown";
-  }
-}
-
-}  // namespace
-
 DeviceInfo::DeviceInfo(const std::string& guid,
                        const std::string& client_name,
                        const std::string& chrome_version,
@@ -133,65 +98,6 @@ bool DeviceInfo::Equals(const DeviceInfo& other) const {
          this->signin_scoped_device_id() == other.signin_scoped_device_id();
 }
 
-// static.
-sync_pb::SyncEnums::DeviceType DeviceInfo::GetLocalDeviceType() {
-#if defined(OS_CHROMEOS)
-  return sync_pb::SyncEnums_DeviceType_TYPE_CROS;
-#elif defined(OS_LINUX)
-  return sync_pb::SyncEnums_DeviceType_TYPE_LINUX;
-#elif defined(OS_MACOSX)
-  return sync_pb::SyncEnums_DeviceType_TYPE_MAC;
-#elif defined(OS_WIN)
-  return sync_pb::SyncEnums_DeviceType_TYPE_WIN;
-#elif defined(OS_ANDROID)
-  return IsTabletUI() ?
-      sync_pb::SyncEnums_DeviceType_TYPE_TABLET :
-      sync_pb::SyncEnums_DeviceType_TYPE_PHONE;
-#else
-  return sync_pb::SyncEnums_DeviceType_TYPE_OTHER;
-#endif
-}
-
-// static
-std::string DeviceInfo::MakeUserAgentForSyncApi(
-    const chrome::VersionInfo& version_info) {
-  std::string user_agent;
-  user_agent = "Chrome ";
-#if defined(OS_WIN)
-  user_agent += "WIN ";
-#elif defined(OS_CHROMEOS)
-  user_agent += "CROS ";
-#elif defined(OS_ANDROID)
-  if (IsTabletUI())
-    user_agent += "ANDROID-TABLET ";
-  else
-    user_agent += "ANDROID-PHONE ";
-#elif defined(OS_LINUX)
-  user_agent += "LINUX ";
-#elif defined(OS_FREEBSD)
-  user_agent += "FREEBSD ";
-#elif defined(OS_OPENBSD)
-  user_agent += "OPENBSD ";
-#elif defined(OS_MACOSX)
-  user_agent += "MAC ";
-#endif
-  if (!version_info.is_valid()) {
-    DLOG(ERROR) << "Unable to create chrome::VersionInfo object";
-    return user_agent;
-  }
-
-  user_agent += version_info.Version();
-  user_agent += " (" + version_info.LastChange() + ")";
-  if (!version_info.IsOfficialBuild()) {
-    user_agent += "-devel";
-  } else {
-    user_agent += " channel(" +
-        ChannelToString(version_info.GetChannel()) + ")";
-  }
-
-  return user_agent;
-}
-
 base::DictionaryValue* DeviceInfo::ToValue() {
   base::DictionaryValue* value = new base::DictionaryValue();
   value->SetString("name", client_name_);
@@ -207,47 +113,8 @@ void DeviceInfo::set_public_id(std::string id) {
 }
 
 // static.
-void DeviceInfo::CreateLocalDeviceInfo(
-    const std::string& guid,
-    const std::string& signin_scoped_device_id,
-    base::Callback<void(const DeviceInfo& local_info)> callback) {
-  GetClientName(base::Bind(&DeviceInfo::CreateLocalDeviceInfoContinuation,
-                           guid,
-                           signin_scoped_device_id,
-                           callback));
-}
-
-// static.
-void DeviceInfo::GetClientName(
-    base::Callback<void(const std::string& client_name)> callback) {
-  syncer::GetSessionName(
-      content::BrowserThread::GetBlockingPool(),
-      base::Bind(&DeviceInfo::GetClientNameContinuation,
-                 callback));
-}
-
-void DeviceInfo::GetClientNameContinuation(
-    base::Callback<void(const std::string& local_info)> callback,
-    const std::string& session_name) {
-  callback.Run(session_name);
-}
-
-// static.
-void DeviceInfo::CreateLocalDeviceInfoContinuation(
-    const std::string& guid,
-    const std::string& signin_scoped_device_id,
-    base::Callback<void(const DeviceInfo& local_info)> callback,
-    const std::string& session_name) {
-  chrome::VersionInfo version_info;
-
-  DeviceInfo local_info(guid,
-                        session_name,
-                        version_info.CreateVersionString(),
-                        MakeUserAgentForSyncApi(version_info),
-                        GetLocalDeviceType(),
-                        signin_scoped_device_id);
-
-  callback.Run(local_info);
+void DeviceInfo::GetClientName(const GetClientNameCallback& callback) {
+  syncer::GetSessionName(content::BrowserThread::GetBlockingPool(), callback);
 }
 
 }  // namespace browser_sync

chrome/browser/sync/glue/device_info.h

@@ -8,22 +8,20 @@
 #include <string>
 
 #include "base/basictypes.h"
-#include "base/bind.h"
+#include "base/callback.h"
 #include "sync/protocol/sync.pb.h"
 
 namespace base {
 class DictionaryValue;
 }
 
-namespace chrome {
-class VersionInfo;
-}
-
 namespace browser_sync {
 
 // A class that holds information regarding the properties of a device.
 class DeviceInfo {
  public:
+  typedef base::Callback<void(const std::string&)> GetClientNameCallback;
+
   DeviceInfo(const std::string& guid,
              const std::string& client_name,
              const std::string& chrome_version,
@@ -46,7 +44,7 @@ class DeviceInfo {
 
   // The user agent is the combination of OS type, chrome version and which
   // channel of chrome(stable or beta). For more information see
-  // |DeviceInfo::MakeUserAgentForSyncApi|.
+  // |LocalDeviceInfoProviderImpl::MakeUserAgentForSyncApi|.
   const std::string& sync_user_agent() const;
 
   // Third party visible id for the device. See |public_id_| for more details.
@@ -78,36 +76,10 @@ class DeviceInfo {
   // which extension APIs can expose to third party apps.
   base::DictionaryValue* ToValue();
 
-  static sync_pb::SyncEnums::DeviceType GetLocalDeviceType();
-
-  // Creates a |DeviceInfo| object representing the local device and passes
-  // it as parameter to the callback.
-  static void CreateLocalDeviceInfo(
-      const std::string& guid,
-      const std::string& signin_scoped_device_id,
-      base::Callback<void(const DeviceInfo& local_info)> callback);
-
   // Gets the local device name and passes it as a parameter to callback.
-  static void GetClientName(
-      base::Callback<void(const std::string& local_info)> callback);
-
-  // Helper to construct a user agent string (ASCII) suitable for use by
-  // the syncapi for any HTTP communication. This string is used by the sync
-  // backend for classifying client types when calculating statistics.
-  static std::string MakeUserAgentForSyncApi(
-      const chrome::VersionInfo& version_info);
+  static void GetClientName(const GetClientNameCallback& callback);
 
  private:
-  static void GetClientNameContinuation(
-      base::Callback<void(const std::string& local_info)> callback,
-      const std::string& session_name);
-
-  static void CreateLocalDeviceInfoContinuation(
-      const std::string& guid,
-      const std::string& signin_scoped_device_id,
-      base::Callback<void(const DeviceInfo& local_info)> callback,
-      const std::string& session_name);
-
   const std::string guid_;
 
   const std::string client_name_;

chrome/browser/sync/glue/local_device_info_provider_impl.cc

@@ -2,10 +2,59 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/bind.h"
 #include "chrome/browser/sync/glue/local_device_info_provider_impl.h"
+#include "chrome/common/chrome_version_info.h"
+#include "ui/base/device_form_factor.h"
 
 namespace browser_sync {
 
+namespace {
+
+// Converts VersionInfo::Channel to string for user-agent string.
+std::string ChannelToString(chrome::VersionInfo::Channel channel) {
+  switch (channel) {
+    case chrome::VersionInfo::CHANNEL_UNKNOWN:
+      return "unknown";
+    case chrome::VersionInfo::CHANNEL_CANARY:
+      return "canary";
+    case chrome::VersionInfo::CHANNEL_DEV:
+      return "dev";
+    case chrome::VersionInfo::CHANNEL_BETA:
+      return "beta";
+    case chrome::VersionInfo::CHANNEL_STABLE:
+      return "stable";
+    default:
+      NOTREACHED();
+      return "unknown";
+  }
+}
+
+#if defined(OS_ANDROID)
+bool IsTabletUI() {
+  return ui::GetDeviceFormFactor() == ui::DEVICE_FORM_FACTOR_TABLET;
+}
+#endif
+
+sync_pb::SyncEnums::DeviceType GetLocalDeviceType() {
+#if defined(OS_CHROMEOS)
+  return sync_pb::SyncEnums_DeviceType_TYPE_CROS;
+#elif defined(OS_LINUX)
+  return sync_pb::SyncEnums_DeviceType_TYPE_LINUX;
+#elif defined(OS_MACOSX)
+  return sync_pb::SyncEnums_DeviceType_TYPE_MAC;
+#elif defined(OS_WIN)
+  return sync_pb::SyncEnums_DeviceType_TYPE_WIN;
+#elif defined(OS_ANDROID)
+  return IsTabletUI() ? sync_pb::SyncEnums_DeviceType_TYPE_TABLET
+                      : sync_pb::SyncEnums_DeviceType_TYPE_PHONE;
+#else
+  return sync_pb::SyncEnums_DeviceType_TYPE_OTHER;
+#endif
+}
+
+}  // namespace
+
 LocalDeviceInfoProviderImpl::LocalDeviceInfoProviderImpl()
     : weak_factory_(this) {
 }
@@ -13,6 +62,46 @@ LocalDeviceInfoProviderImpl::LocalDeviceInfoProviderImpl()
 LocalDeviceInfoProviderImpl::~LocalDeviceInfoProviderImpl() {
 }
 
+// static.
+std::string LocalDeviceInfoProviderImpl::MakeUserAgentForSyncApi(
+    const chrome::VersionInfo& version_info) {
+  std::string user_agent;
+  user_agent = "Chrome ";
+#if defined(OS_WIN)
+  user_agent += "WIN ";
+#elif defined(OS_CHROMEOS)
+  user_agent += "CROS ";
+#elif defined(OS_ANDROID)
+  if (IsTabletUI())
+    user_agent += "ANDROID-TABLET ";
+  else
+    user_agent += "ANDROID-PHONE ";
+#elif defined(OS_LINUX)
+  user_agent += "LINUX ";
+#elif defined(OS_FREEBSD)
+  user_agent += "FREEBSD ";
+#elif defined(OS_OPENBSD)
+  user_agent += "OPENBSD ";
+#elif defined(OS_MACOSX)
+  user_agent += "MAC ";
+#endif
+  if (!version_info.is_valid()) {
+    DLOG(ERROR) << "Unable to create chrome::VersionInfo object";
+    return user_agent;
+  }
+
+  user_agent += version_info.Version();
+  user_agent += " (" + version_info.LastChange() + ")";
+  if (!version_info.IsOfficialBuild()) {
+    user_agent += "-devel";
+  } else {
+    user_agent +=
+        " channel(" + ChannelToString(version_info.GetChannel()) + ")";
+  }
+
+  return user_agent;
+}
+
 const DeviceInfo*
 LocalDeviceInfoProviderImpl::GetLocalDeviceInfo() const {
   return local_device_info_.get();
@@ -33,25 +122,26 @@ void LocalDeviceInfoProviderImpl::Initialize(
     const std::string& cache_guid, const std::string& signin_scoped_device_id) {
   DCHECK(!cache_guid.empty());
   cache_guid_ = cache_guid;
-  DeviceInfo::CreateLocalDeviceInfo(
-      cache_guid_,
-      signin_scoped_device_id,
+
+  DeviceInfo::GetClientName(
       base::Bind(&LocalDeviceInfoProviderImpl::InitializeContinuation,
-                 weak_factory_.GetWeakPtr()));
+                 weak_factory_.GetWeakPtr(),
+                 cache_guid,
+                 signin_scoped_device_id));
 }
 
 void LocalDeviceInfoProviderImpl::InitializeContinuation(
-    const DeviceInfo& local_info) {
-  // Copy constructor is disallowed in DeviceInfo, construct a new one from
-  // the fields passed in local_info.
-  local_device_info_.reset(
-      new DeviceInfo(
-          local_info.guid(),
-          local_info.client_name(),
-          local_info.chrome_version(),
-          local_info.sync_user_agent(),
-          local_info.device_type(),
-          local_info.signin_scoped_device_id()));
+    const std::string& guid,
+    const std::string& signin_scoped_device_id,
+    const std::string& session_name) {
+  chrome::VersionInfo version_info;
+
+  local_device_info_.reset(new DeviceInfo(guid,
+                                          session_name,
+                                          version_info.CreateVersionString(),
+                                          MakeUserAgentForSyncApi(version_info),
+                                          GetLocalDeviceType(),
+                                          signin_scoped_device_id));
 
   // Notify observers.
   callback_list_.Notify();

chrome/browser/sync/glue/local_device_info_provider_impl.h

@@ -9,6 +9,10 @@
 #include "chrome/browser/sync/glue/device_info.h"
 #include "chrome/browser/sync/glue/local_device_info_provider.h"
 
+namespace chrome {
+class VersionInfo;
+}
+
 namespace browser_sync {
 
 class LocalDeviceInfoProviderImpl : public LocalDeviceInfoProvider {
@@ -25,8 +29,16 @@ class LocalDeviceInfoProviderImpl : public LocalDeviceInfoProvider {
   virtual scoped_ptr<Subscription> RegisterOnInitializedCallback(
     const base::Closure& callback) OVERRIDE;
 
+  // Helper to construct a user agent string (ASCII) suitable for use by
+  // the syncapi for any HTTP communication. This string is used by the sync
+  // backend for classifying client types when calculating statistics.
+  static std::string MakeUserAgentForSyncApi(
+      const chrome::VersionInfo& version_info);
+
  private:
-  void InitializeContinuation(const DeviceInfo& local_info);
+  void InitializeContinuation(const std::string& guid,
+                              const std::string& signin_scoped_device_id,
+                              const std::string& session_name);
 
   std::string cache_guid_;
   scoped_ptr<DeviceInfo> local_device_info_;

chrome/browser/sync/glue/local_device_info_provider_unittest.cc

@@ -2,9 +2,12 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/bind.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "chrome/browser/sync/glue/local_device_info_provider_impl.h"
+#include "chrome/common/chrome_version_info.h"
+#include "sync/util/get_session_name.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace browser_sync {
@@ -72,6 +75,13 @@ TEST_F(LocalDeviceInfoProviderTest, GetLocalDeviceInfo) {
   EXPECT_EQ(std::string(kLocalDeviceGuid), local_device_info->guid());
   EXPECT_EQ(std::string(kSigninScopedDeviceId),
             local_device_info->signin_scoped_device_id());
+  EXPECT_EQ(syncer::GetSessionNameSynchronouslyForTesting(),
+            local_device_info->client_name());
+
+  chrome::VersionInfo version_info;
+  EXPECT_EQ(browser_sync::LocalDeviceInfoProviderImpl::MakeUserAgentForSyncApi(
+                version_info),
+            local_device_info->sync_user_agent());
 }
 
 TEST_F(LocalDeviceInfoProviderTest, GetLocalSyncCacheGUID) {

chrome/browser/sync/glue/sync_backend_host_core.cc

@@ -6,8 +6,8 @@
 
 #include "base/files/file_util.h"
 #include "base/metrics/histogram.h"
-#include "chrome/browser/sync/glue/device_info.h"
 #include "chrome/browser/sync/glue/invalidation_adapter.h"
+#include "chrome/browser/sync/glue/local_device_info_provider_impl.h"
 #include "chrome/browser/sync/glue/sync_backend_registrar.h"
 #include "chrome/common/chrome_version_info.h"
 #include "components/invalidation/invalidation_util.h"
@@ -405,7 +405,7 @@ void SyncBackendHostCore::DoInitialize(
   // building the user agent may block on some platforms.
   chrome::VersionInfo version_info;
   options->http_bridge_factory->Init(
-      DeviceInfo::MakeUserAgentForSyncApi(version_info));
+      LocalDeviceInfoProviderImpl::MakeUserAgentForSyncApi(version_info));
 
   // Blow away the partial or corrupt sync data folder before doing any more
   // initialization, if necessary.

chrome/browser/sync/sessions/sessions_sync_manager.cc

@@ -122,7 +122,7 @@ syncer::SyncMergeResult SessionsSyncManager::MergeDataAndStartSyncing(
     base_specifics->set_session_tag(current_machine_tag());
     sync_pb::SessionHeader* header_s = base_specifics->mutable_header();
     header_s->set_client_name(current_session_name_);
-    header_s->set_device_type(DeviceInfo::GetLocalDeviceType());
+    header_s->set_device_type(local_device_info->device_type());
     syncer::SyncData data = syncer::SyncData::CreateLocalData(
         current_machine_tag(), current_session_name_, specifics);
     new_changes.push_back(syncer::SyncChange(
@@ -158,7 +158,11 @@ void SessionsSyncManager::AssociateWindows(
   SyncedSession* current_session = session_tracker_.GetSession(local_tag);
   current_session->modified_time = base::Time::Now();
   header_s->set_client_name(current_session_name_);
-  header_s->set_device_type(DeviceInfo::GetLocalDeviceType());
+  // SessionDataTypeController ensures that the local device info
+  // is available before activating this datatype.
+  DCHECK(local_device_);
+  const DeviceInfo* local_device_info = local_device_->GetLocalDeviceInfo();
+  header_s->set_device_type(local_device_info->device_type());
 
   session_tracker_.ResetSessionTracking(local_tag);
   std::set<SyncedWindowDelegate*> windows =