Device info datatype should be moved to components/sync_driver.

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
Committed: https://crrev.com/1052875a076568fc4232b506d19d1d029e9814e6
Cr-Commit-Position: refs/heads/master@{#296267}

[stanisc, 2014-09-23 02:06:24]

stanisc@chromium.org changed reviewers:
+ maniscalco@chromium.org, pkasting@chromium.org

[stanisc, 2014-09-23 02:06:25]

pkasting@chromium.org: Please review changes in chrome/browser/search_engines.

maniscalco@chromium.org: Please review changes in chrome/browser/sync.

[Peter Kasting, 2014-09-23 04:06:31]

LGTM

https://codereview.chromium.org/594643003/diff/20001/chrome/browser/search_en...
File chrome/browser/search_engines/ui_thread_search_terms_data.cc (right):

https://codereview.chromium.org/594643003/diff/20001/chrome/browser/search_en...
chrome/browser/search_engines/ui_thread_search_terms_data.cc:107: :
"chrome-omni";
Nit: Please wrap the ?: like the old code did

[maniscalco, 2014-09-23 16:28:27]

LGTM with a couple nits.  Nice change description BTW.

https://codereview.chromium.org/594643003/diff/20001/chrome/browser/sync/glue...
File chrome/browser/sync/glue/device_info.cc (right):

https://codereview.chromium.org/594643003/diff/20001/chrome/browser/sync/glue...
chrome/browser/sync/glue/device_info.cc:117: const base::Callback<void(const
std::string& client_name)> callback) {
nit: any reason for the parameter name in the callback to differ between
declaration and definition? (client_name vs local_info).

Better yet, how about introducing a typedef

typedef base::Callback<void(const std::string& client_name)>
GetClientNameCallback;

?

https://codereview.chromium.org/594643003/diff/20001/chrome/browser/sync/glue...
File chrome/browser/sync/glue/device_info.h (right):

https://codereview.chromium.org/594643003/diff/20001/chrome/browser/sync/glue...
chrome/browser/sync/glue/device_info.h:79: const base::Callback<void(const
std::string& local_info)> callback);
You can make callback a reference parameter.

[stanisc, 2014-09-23 21:30:33]

Addressed code review feedback.

https://codereview.chromium.org/594643003/diff/20001/chrome/browser/search_en...
File chrome/browser/search_engines/ui_thread_search_terms_data.cc (right):

https://codereview.chromium.org/594643003/diff/20001/chrome/browser/search_en...
chrome/browser/search_engines/ui_thread_search_terms_data.cc:107: :
"chrome-omni";
On 2014/09/23 04:06:31, Peter Kasting wrote:
> Nit: Please wrap the ?: like the old code did
Done. By the way this formatting was done by running "git cl format".

https://codereview.chromium.org/594643003/diff/20001/chrome/browser/sync/glue...
File chrome/browser/sync/glue/device_info.cc (right):

https://codereview.chromium.org/594643003/diff/20001/chrome/browser/sync/glue...
chrome/browser/sync/glue/device_info.cc:117: const base::Callback<void(const
std::string& client_name)> callback) {
On 2014/09/23 16:28:27, maniscalco wrote:
> nit: any reason for the parameter name in the callback to differ between
> declaration and definition? (client_name vs local_info).
> 
> Better yet, how about introducing a typedef
> 
> typedef base::Callback<void(const std::string& client_name)>
> GetClientNameCallback;
> 
> ?

Done. Introduced the typedef you suggested. The name "local_info" ended up in
the callback definition as a result of copy&paste from the old code. Deleted it.

https://codereview.chromium.org/594643003/diff/20001/chrome/browser/sync/glue...
File chrome/browser/sync/glue/device_info.h (right):

https://codereview.chromium.org/594643003/diff/20001/chrome/browser/sync/glue...
chrome/browser/sync/glue/device_info.h:79: const base::Callback<void(const
std::string& local_info)> callback);
On 2014/09/23 16:28:27, maniscalco wrote:
> You can make callback a reference parameter.

Done.

[stanisc, 2014-09-23 21:33:26]

The CQ bit was checked by stanisc@chromium.org

[commit-bot: I haz the power, 2014-09-23 21:34:25]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/594643003/40001

[commit-bot: I haz the power, 2014-09-23 21:52:30]

Committed patchset #3 (id:40001) as 7667c7da392e5fffc39c4da09fa6ac4877832520

[commit-bot: I haz the power, 2014-09-23 21:52:55]

Patchset 3 (id:??) landed as
https://crrev.com/1052875a076568fc4232b506d19d1d029e9814e6
Cr-Commit-Position: refs/heads/master@{#296267}