Adding handling of MCS Endpoints to GServicesSettings

Adding handling of MCS Endpoints to GServicesSettings

Removing mcs_hostname and mcs_secure_port from GServicesSettings.
Adding mcs_main_endpoint and mcs_fallback_enpoint.
Constructing the main endpoint based on g-settings from the checkin.
Constructing the fallback endpoint based on mcs hostname and default
fallback port.

R=jianli@chromium.org,zea@chromium.org
BUG=359254
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=269632

[fgorski, 2014-05-08 18:39:21]

PTAL.
This one closes the G-services settings V2.

[jianli, 2014-05-08 18:59:37]

https://codereview.chromium.org/270403006/diff/1/google_apis/gcm/engine/gserv...
File google_apis/gcm/engine/gservices_settings.cc (right):

https://codereview.chromium.org/270403006/diff/1/google_apis/gcm/engine/gserv...
google_apis/gcm/engine/gservices_settings.cc:27: const int kDefaultMCSSecurePort
= 5228;
nit: kDefaultMCSMainSecurePort

https://codereview.chromium.org/270403006/diff/1/google_apis/gcm/engine/gserv...
google_apis/gcm/engine/gservices_settings.cc:28: const int
kDefaultMCSFallbackPort = 443;
nit: better to add "Secure" into the name, like kDefaultMCSFallbackSecurePort

https://codereview.chromium.org/270403006/diff/1/google_apis/gcm/engine/gserv...
google_apis/gcm/engine/gservices_settings.cc:184: mcs_main_endpoint_ =
GURL(MakeMCSEndpoint(new_mcs_hostname,
Can you validate the URL?

[fgorski, 2014-05-08 19:21:03]

PTAL

https://codereview.chromium.org/270403006/diff/1/google_apis/gcm/engine/gserv...
File google_apis/gcm/engine/gservices_settings.cc (right):

https://codereview.chromium.org/270403006/diff/1/google_apis/gcm/engine/gserv...
google_apis/gcm/engine/gservices_settings.cc:27: const int kDefaultMCSSecurePort
= 5228;
On 2014/05/08 18:59:37, jianli wrote:
> nit: kDefaultMCSMainSecurePort

Done.

https://codereview.chromium.org/270403006/diff/1/google_apis/gcm/engine/gserv...
google_apis/gcm/engine/gservices_settings.cc:28: const int
kDefaultMCSFallbackPort = 443;
On 2014/05/08 18:59:37, jianli wrote:
> nit: better to add "Secure" into the name, like kDefaultMCSFallbackSecurePort

Done.

https://codereview.chromium.org/270403006/diff/1/google_apis/gcm/engine/gserv...
google_apis/gcm/engine/gservices_settings.cc:184: mcs_main_endpoint_ =
GURL(MakeMCSEndpoint(new_mcs_hostname,
On 2014/05/08 18:59:37, jianli wrote:
> Can you validate the URL?

Done.

[jianli, 2014-05-08 20:49:35]

lgtm

[Nicolas Zea, 2014-05-08 22:00:52]

https://codereview.chromium.org/270403006/diff/20001/google_apis/gcm/engine/g...
File google_apis/gcm/engine/gservices_settings.cc (right):

https://codereview.chromium.org/270403006/diff/20001/google_apis/gcm/engine/g...
google_apis/gcm/engine/gservices_settings.cc:34: return
base::StringPrintf(kMCSEnpointTemplate, mcs_hostname.c_str(), port);
Should we be doing any kind of verification that this is a valid google
endpoint? (x.google.com) Otherwise we don't know if the SSL certificate key
pinning will be enabled.

[fgorski, 2014-05-09 17:54:41]

https://codereview.chromium.org/270403006/diff/20001/google_apis/gcm/engine/g...
File google_apis/gcm/engine/gservices_settings.cc (right):

https://codereview.chromium.org/270403006/diff/20001/google_apis/gcm/engine/g...
google_apis/gcm/engine/gservices_settings.cc:34: return
base::StringPrintf(kMCSEnpointTemplate, mcs_hostname.c_str(), port);
On 2014/05/08 22:00:52, Nicolas Zea wrote:
> Should we be doing any kind of verification that this is a valid google
> endpoint? (http://x.google.com) Otherwise we don't know if the SSL certificate
key
> pinning will be enabled.

Based on our conversation, we don't want to make a change like that right now.

[Nicolas Zea, 2014-05-09 17:55:36]

lgtm

[fgorski, 2014-05-09 20:03:13]

The CQ bit was checked by fgorski@chromium.org

[commit-bot: I haz the power, 2014-05-09 20:04:13]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/fgorski@chromium.org/270403006/20001

[commit-bot: I haz the power, 2014-05-09 21:26:18]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-05-09 23:00:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-05-09 23:00:59]

Try jobs failed on following builders:
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)

[fgorski, 2014-05-10 18:21:26]

The CQ bit was checked by fgorski@chromium.org

[commit-bot: I haz the power, 2014-05-10 18:24:47]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/fgorski@chromium.org/270403006/20001

[commit-bot: I haz the power, 2014-05-10 20:40:19]

Change committed as 269632