components/component_updater/component_updater_url_constants.cc - Issue 1281913002: Componentize ComponentUpdaterConfigurator

Side by Side Diff: components/component_updater/component_updater_url_constants.cc

Issue 1281913002: Componentize ComponentUpdaterConfigurator (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: Fix iOS compile Created 5 years, 4 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

« no previous file with comments | « components/component_updater/component_updater_url_constants.h ('k') | components/component_updater/configurator_impl.h » ('j') | no next file with comments »

OLD	NEW
1 // Copyright 2015 The Chromium Authors. All rights reserved.	1 // Copyright 2015 The Chromium Authors. All rights reserved.

2 // Use of this source code is governed by a BSD-style license that can be	2 // Use of this source code is governed by a BSD-style license that can be

3 // found in the LICENSE file.	3 // found in the LICENSE file.

4	4

5 #include "chrome/browser/component_updater/component_updater_url_constants.h"	5 #include "components/component_updater/component_updater_url_constants.h"

6	6

7 namespace component_updater {	7 namespace component_updater {

8	8

9 // The alternative URL for the v3 protocol service endpoint.	9 // The alternative URL for the v3 protocol service endpoint.

10 const char kUpdaterAltUrl[] = "http://clients2.google.com/service/update2";	10 const char kUpdaterAltUrl[] = "http://clients2.google.com/service/update2";

11	11

12 // The default URL for the v3 protocol service endpoint. In some cases, the	12 // The default URL for the v3 protocol service endpoint. In some cases, the

13 // component updater is allowed to fall back to and alternate URL source, if	13 // component updater is allowed to fall back to and alternate URL source, if

14 // the request to the default URL source fails.	14 // the request to the default URL source fails.

15 // The value of \|kDefaultUrlSource\| can be overridden with	15 // The value of \|kDefaultUrlSource\| can be overridden with

16 // --component-updater=url-source=someurl.	16 // --component-updater=url-source=someurl.

17 const char kUpdaterDefaultUrl[] = "https://clients2.google.com/service/update2";	17 const char kUpdaterDefaultUrl[] = "https://clients2.google.com/service/update2";

18	18
	asargent_no_longer_on_chrome 2015/08/18 17:54:25 Does it really make sense for these google-specifi Does it really make sense for these google-specific URLs to be in the components/ namespace? What if some other product built on top of chromium source (eg opera, etc.) wanted to use component updater code to do something similar, but use their own URLs? I would have expected to see these constants stay defined somewhere in chrome/ and then some interface with virtual methods defined in components/, and a concrete implementation those methods returning these specific urls in chrome. Had this kind of approach already been considered and rejected for some reason? blundell 2015/08/18 18:02:38 We have discussed this kind of issue in the past, Show quoted text On 2015/08/18 17:54:25, Antony Sargent wrote: > Does it really make sense for these google-specific URLs to be in the > components/ namespace? What if some other product built on top of chromium > source (eg opera, etc.) wanted to use component updater code to do something > similar, but use their own URLs? > > I would have expected to see these constants stay defined somewhere in chrome/ > and then some interface with virtual methods defined in components/, and a > concrete implementation those methods returning these specific urls in chrome. > Had this kind of approach already been considered and rejected for some reason? > > We have discussed this kind of issue in the past, and here have been the conclusions: - The component is being created for the iOS port of Chrome to share code with the other ports. - As such, it is OK for the component to include Google-specific code, strings, constants, or whatnot. - If some other embedder later comes along and wants to share this code but with s/Google/<foo>, the code can easily be generalized at that point. TL;DR: The component is generalized only to the level that generalization is currently required.
19 } // namespace component_updater	19 } // namespace component_updater

OLD	NEW