Enable SDCH over HTTPS.

Enable SDCH over HTTPS.

BUG=357769
R=mef@chromium.org

Committed: https://crrev.com/ec06b7d8f7c2df2a5c4f1b5315a5c40c72eb5bb5
Cr-Commit-Position: refs/heads/master@{#296451}

[Randy Smith (Not in Mondays), 2014-09-22 21:33:36]

Misha: (Hopefully) quick review for you?

[mef, 2014-09-23 08:06:32]

https://codereview.chromium.org/593863002/diff/1/chrome/browser/chrome_browse...
File chrome/browser/chrome_browser_main.cc (right):

https://codereview.chromium.org/593863002/diff/1/chrome/browser/chrome_browse...
chrome/browser/chrome_browser_main.cc:1377: {
is empty block needed here? I don't see them elsewhere in this file,

https://codereview.chromium.org/593863002/diff/1/chrome/browser/chrome_browse...
chrome/browser/chrome_browser_main.cc:1391: if
(sdch_trial_group.starts_with(kEnabledAllGroupName)) {
Is kEnabledAllGroupName still needed?

https://codereview.chromium.org/593863002/diff/1/chrome/browser/chrome_browse...
chrome/browser/chrome_browser_main.cc:1394: } else if
(sdch_trial_group.starts_with(kEnabledHttpOnlyGroupName)) {
This is essentially "kDisabledHttps", right? Then it should have
EnableSecureSchemeSupport(false);

[Randy Smith (Not in Mondays), 2014-09-23 21:44:43]

Misha: Comments incorporated; PTAL?

https://codereview.chromium.org/593863002/diff/1/chrome/browser/chrome_browse...
File chrome/browser/chrome_browser_main.cc (right):

https://codereview.chromium.org/593863002/diff/1/chrome/browser/chrome_browse...
chrome/browser/chrome_browser_main.cc:1377: {
On 2014/09/23 08:06:32, mef wrote:
> is empty block needed here? I don't see them elsewhere in this file,

Done.

https://codereview.chromium.org/593863002/diff/1/chrome/browser/chrome_browse...
chrome/browser/chrome_browser_main.cc:1391: if
(sdch_trial_group.starts_with(kEnabledAllGroupName)) {
On 2014/09/23 08:06:32, mef wrote:
> Is kEnabledAllGroupName still needed? 

Done.

https://codereview.chromium.org/593863002/diff/1/chrome/browser/chrome_browse...
chrome/browser/chrome_browser_main.cc:1394: } else if
(sdch_trial_group.starts_with(kEnabledHttpOnlyGroupName)) {
On 2014/09/23 08:06:32, mef wrote:
> This is essentially "kDisabledHttps", right? Then it should have
> EnableSecureSchemeSupport(false);

Yeah, you're right.  That's probably the right way to go, and not add a new
group name.  Done.

[Randy Smith (Not in Mondays), 2014-09-23 21:45:09]

On 2014/09/23 21:44:43, rdsmith wrote:
> Misha: Comments incorporated; PTAL?
> 
>
https://codereview.chromium.org/593863002/diff/1/chrome/browser/chrome_browse...
> File chrome/browser/chrome_browser_main.cc (right):
> 
>
https://codereview.chromium.org/593863002/diff/1/chrome/browser/chrome_browse...
> chrome/browser/chrome_browser_main.cc:1377: {
> On 2014/09/23 08:06:32, mef wrote:
> > is empty block needed here? I don't see them elsewhere in this file,
> 
> Done.
> 
>
https://codereview.chromium.org/593863002/diff/1/chrome/browser/chrome_browse...
> chrome/browser/chrome_browser_main.cc:1391: if
> (sdch_trial_group.starts_with(kEnabledAllGroupName)) {
> On 2014/09/23 08:06:32, mef wrote:
> > Is kEnabledAllGroupName still needed? 
> 
> Done.
> 
>
https://codereview.chromium.org/593863002/diff/1/chrome/browser/chrome_browse...
> chrome/browser/chrome_browser_main.cc:1394: } else if
> (sdch_trial_group.starts_with(kEnabledHttpOnlyGroupName)) {
> On 2014/09/23 08:06:32, mef wrote:
> > This is essentially "kDisabledHttps", right? Then it should have
> > EnableSecureSchemeSupport(false);
> 
> Yeah, you're right.  That's probably the right way to go, and not add a new
> group name.  Done.

Actually, hold off for a patchset; I was moving too fast and forgot to fix a
problem.

[Randy Smith (Not in Mondays), 2014-09-23 21:54:22]

Ok, latest patchset should be good.  PTAL.

[mef, 2014-09-23 21:59:32]

lgtm

[Randy Smith (Not in Mondays), 2014-09-23 22:00:09]

On 2014/09/23 21:59:32, mef wrote:
> lgtm

Thanks!

[Randy Smith (Not in Mondays), 2014-09-24 15:15:47]

The CQ bit was checked by rdsmith@chromium.org

[commit-bot: I haz the power, 2014-09-24 15:17:13]

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

[commit-bot: I haz the power, 2014-09-24 15:24:28]

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

[commit-bot: I haz the power, 2014-09-24 15:24:29]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Randy Smith (Not in Mondays), 2014-09-24 16:07:05]

rdsmith@chromium.org changed reviewers:
+ sky@chromium.org

[Randy Smith (Not in Mondays), 2014-09-24 16:07:06]

Scott: Stamp?

[sky, 2014-09-24 17:19:22]

LGTM

[Randy Smith (Not in Mondays), 2014-09-24 17:29:04]

The CQ bit was checked by rdsmith@chromium.org

[commit-bot: I haz the power, 2014-09-24 17:30:35]

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

[commit-bot: I haz the power, 2014-09-24 17:34:39]

Committed patchset #3 (id:40001) as 551b3d72c28552cce899813e51e737b384e940c3

[commit-bot: I haz the power, 2014-09-24 17:35:22]

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