Cleanup VersionInfo after componentization.

Cleanup VersionInfo after componentization.

Remove wrapper methods around version_info free functions and directly
call them instead and fixes #include if only the component is required.

Convert VersionInfo methods to free functions (as they do not have
state) and rename GetVersionStringModifier to GetChannelString.

Rename chrome/common/{chrome_version_info => channel_info}*.

Forward-declare version_info::Channel in channel_info.h and add the
missing #include of components/version_info/version_info.h were needed.

BUG=514562
Committed: https://crrev.com/9fb6769d7d92d0f3ea0cd67be95f7450dec6c2b3
Cr-Commit-Position: refs/heads/master@{#341568}

[sdefresne, 2015-07-30 16:30:14]

Patchset #1 (id:1) has been deleted

[sdefresne, 2015-07-30 16:30:16]

Patchset #1 (id:1) has been deleted

[sdefresne, 2015-07-30 16:40:41]

sdefresne@chromium.org changed reviewers:
+ brettw@chromium.org

[sdefresne, 2015-07-30 16:40:42]

Please take a look.

This is a followup to https://codereview.chromium.org/1257633002 as requested.

Please concentrate on chrome/common/channel_info* (formerly called
chrome/common/chrome_version_info*). The rest is just changing the client code
to adapt to the API change and fixing #include.

[brettw, 2015-07-31 21:13:16]

It seems like a bunch of files include the channel_info header but don't need
it. I marked these when I saw one. These should be deleted instead of renamed,
if my understanding is correct.

LGTM with that ands one removal of a c_str() call, thanks!

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/app_mode/startup_app_launcher.cc (right):

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/app_mode/startup_app_launcher.cc:27: #include
"chrome/common/channel_info.h"
This doesn't look like it's used in this file.

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/existing_user_controller.cc (right):

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/login/existing_user_controller.cc:50: #include
"chrome/common/channel_info.h"
Ditto

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/devtools...
File chrome/browser/devtools/devtools_targets_ui.cc (right):

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/devtools...
chrome/browser/devtools/devtools_targets_ui.cc:18: #include
"chrome/common/channel_info.h"
Ditto

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/extensio...
File chrome/browser/extensions/api/cast_streaming/performance_test.cc (right):

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/api/cast_streaming/performance_test.cc:19: #include
"chrome/common/channel_info.h"
Ditto

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/extensio...
File
chrome/browser/extensions/api/notification_provider/notification_provider_api.cc
(right):

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/api/notification_provider/notification_provider_api.cc:17:
#include "chrome/common/channel_info.h"
Ditto

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/extensio...
File chrome/browser/extensions/api/notifications/notifications_api.cc (right):

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/api/notifications/notifications_api.cc:20: #include
"chrome/common/channel_info.h"
Ditto

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/extensio...
File chrome/browser/extensions/api/sync_file_system/sync_file_system_apitest.cc
(right):

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/api/sync_file_system/sync_file_system_apitest.cc:17:
#include "chrome/common/channel_info.h"
Ditto

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/extensio...
File chrome/browser/extensions/api/tab_capture/tab_capture_apitest.cc (right):

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/api/tab_capture/tab_capture_apitest.cc:18: #include
"chrome/common/channel_info.h"
Ditto

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/extensio...
File chrome/browser/extensions/api/tab_capture/tab_capture_performancetest.cc
(right):

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/api/tab_capture/tab_capture_performancetest.cc:15:
#include "chrome/common/channel_info.h"
Ditto

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/extensio...
File chrome/browser/extensions/default_apps.cc (right):

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/default_apps.cc:80:
version_info::GetVersionNumber().c_str());
Can you remove c_str() here? This just causes an unnecessary copy since the
function seems to take a std::string

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/extensio...
File chrome/browser/extensions/extension_message_bubble_controller_unittest.cc
(right):

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/extension_message_bubble_controller_unittest.cc:19:
#include "chrome/common/channel_info.h"
Ditto

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/extensio...
File chrome/browser/extensions/extension_system_impl.cc (right):

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/extension_system_impl.cc:31: #include
"chrome/common/channel_info.h"
Ditto

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/guest_vi...
File chrome/browser/guest_view/web_view/chrome_web_view_guest_delegate.cc
(right):

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/guest_vi...
chrome/browser/guest_view/web_view/chrome_web_view_guest_delegate.cc:13:
#include "chrome/common/channel_info.h"
Ditto

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/history/...
File chrome/browser/history/chrome_history_client.cc (right):

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/history/...
chrome/browser/history/chrome_history_client.cc:13: #include
"chrome/common/channel_info.h"
Ditto

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/media/me...
File chrome/browser/media/media_capture_devices_dispatcher.cc (right):

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/media/me...
chrome/browser/media/media_capture_devices_dispatcher.cc:22: #include
"chrome/common/channel_info.h"
Ditto

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/metrics/...
File chrome/browser/metrics/variations/variations_service.h (right):

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/metrics/...
chrome/browser/metrics/variations/variations_service.h:19: #include
"chrome/common/channel_info.h"
Ditto

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/profiles...
File chrome/browser/profiles/profile_impl.cc (right):

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/profiles...
chrome/browser/profiles/profile_impl.cc:75: #include
"chrome/common/channel_info.h"
Ditto

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/safe_bro...
File chrome/browser/safe_browsing/protocol_manager.cc (right):

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/safe_bro...
chrome/browser/safe_browsing/protocol_manager.cc:18: #include
"chrome/common/channel_info.h"
Ditto

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/sync/pro...
File chrome/browser/sync/profile_sync_components_factory_impl.cc (right):

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/sync/pro...
chrome/browser/sync/profile_sync_components_factory_impl.cc:43: #include
"chrome/common/channel_info.h"
Ditto

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/sync/pro...
File chrome/browser/sync/profile_sync_components_factory_impl_unittest.cc
(right):

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/sync/pro...
chrome/browser/sync/profile_sync_components_factory_impl_unittest.cc:16:
#include "chrome/common/channel_info.h"
Ditto

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/web_appl...
File chrome/browser/web_applications/web_app.cc (right):

https://codereview.chromium.org/1269773002/diff/20001/chrome/browser/web_appl...
chrome/browser/web_applications/web_app.cc:20: #include
"chrome/common/channel_info.h"
Ditto

[sdefresne, 2015-08-03 12:29:16]

sdefresne@chromium.org changed reviewers:
+ derat@chromium.org

[sdefresne, 2015-08-03 12:29:17]

brettw: thank you for the review, all comments addressed (and some more #include
removed)

derat: please take a look as OWNERS of extensions/browser/DEPS. Note that the
components/version_info is already a dependency of chrome/common that is a
dependency of extensions/browser.

[Daniel Erat, 2015-08-03 14:33:57]

On 2015/08/03 12:29:17, sdefresne wrote:
> brettw: thank you for the review, all comments addressed (and some more
#include
> removed)
> 
> derat: please take a look as OWNERS of extensions/browser/DEPS. Note that the
> components/version_info is already a dependency of chrome/common that is a
> dependency of extensions/browser.

lgtm for making extensions/browser depend on components/version_info.

(but extensions/browser doesn't appear to, and shouldn't, depend on
chrome/common, apart from some tests. am i missing something?)

[sdefresne, 2015-08-03 15:17:56]

On 2015/08/03 at 14:33:57, derat wrote:
> On 2015/08/03 12:29:17, sdefresne wrote:
> > brettw: thank you for the review, all comments addressed (and some more
#include
> > removed)
> > 
> > derat: please take a look as OWNERS of extensions/browser/DEPS. Note that
the
> > components/version_info is already a dependency of chrome/common that is a
> > dependency of extensions/browser.
> 
> lgtm for making extensions/browser depend on components/version_info.
> 
> (but extensions/browser doesn't appear to, and shouldn't, depend on
chrome/common, apart from some tests. am i missing something?)

The file extensions/browser/api/app_window/app_window_apitest.cc uses
version_info::Channel enumeration. It used to be chrome::VersionInfo::Channel
defined in //chrome/common/chrome_version_info.h (I've componentized this in
https://codereview.chromium.org/1260033003 last week). I guess that file may
have an (indirect) dependency on chrome/common, and that's why I assumed that
extensions/browser had a dependency on chrome/common. As you say this is only
for some tests.

Thank you for the review.

[sdefresne, 2015-08-03 15:43:29]

The CQ bit was checked by sdefresne@chromium.org

[sdefresne, 2015-08-03 15:43:30]

The patchset sent to the CQ was uploaded after l-g-t-m from brettw@chromium.org
Link to the patchset: https://codereview.chromium.org/1269773002/#ps40001
(title: "Rebase, remove unnecessary #include & c_str() call")

[commit-bot: I haz the power, 2015-08-03 15:43:51]

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

[sdefresne, 2015-08-03 16:23:12]

The CQ bit was unchecked by sdefresne@chromium.org

[sdefresne, 2015-08-03 16:52:29]

The CQ bit was checked by sdefresne@chromium.org

[sdefresne, 2015-08-03 16:52:30]

The patchset sent to the CQ was uploaded after l-g-t-m from brettw@chromium.org,
derat@chromium.org
Link to the patchset: https://codereview.chromium.org/1269773002/#ps60001
(title: "Rebase")

[commit-bot: I haz the power, 2015-08-03 16:52:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1269773002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1269773002/60001

[commit-bot: I haz the power, 2015-08-03 18:48:52]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2015-08-03 18:49:38]

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