Add experimental versions of the SSL interstitial

Add experimental versions of the SSL interstitial

This adds a few experimental versions of the new SSL interstitial:
1. The old layout (V1)
2. The new layout (V2)
3. The new layout, with a yellow background
4. The new layout, with a crossing guard image
5. The old layout with the new layout's text

Binary images in: https://codereview.chromium.org/340173002

BUG=386033
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=278969

[felt, 2014-06-18 17:59:20]

palmer, dbeam, PTAL.

[felt, 2014-06-20 18:43:21]

friendly ping

[palmer, 2014-06-20 19:31:49]

lgtm

[Dan Beam, 2014-06-20 21:28:20]

this is officially the most advance interstitial known to humankind

https://codereview.chromium.org/341653006/diff/40001/chrome/browser/resources...
File chrome/browser/resources/ssl/interstitial_v2.css (right):

https://codereview.chromium.org/341653006/diff/40001/chrome/browser/resources...
chrome/browser/resources/ssl/interstitial_v2.css:121: .safe-browsing small-link
{
nit: .small-link, probably? (unless you're actually using a <small-link>)

also

  .safe-browsing :-webkit-any(
      a, #details, #details-button, h1, small-link) {
    ...
  }

https://codereview.chromium.org/341653006/diff/40001/chrome/browser/resources...
chrome/browser/resources/ssl/interstitial_v2.css:159:
url('images/2x/brokenssl_guard.png') 2x) !important;
!important :(

https://codereview.chromium.org/341653006/diff/40001/chrome/browser/resources...
chrome/browser/resources/ssl/interstitial_v2.css:159:
url('images/2x/brokenssl_guard.png') 2x) !important;
url('...') -> url(...) everywhere

https://codereview.chromium.org/341653006/diff/40001/chrome/browser/resources...
chrome/browser/resources/ssl/interstitial_v2.css:210: .yellow small-link {
:-webkit-any() again?

https://codereview.chromium.org/341653006/diff/40001/chrome/browser/resources...
File chrome/browser/resources/ssl/interstitial_v2.js (right):

https://codereview.chromium.org/341653006/diff/40001/chrome/browser/resources...
chrome/browser/resources/ssl/interstitial_v2.js:16: setupSSLFinchTrial();
setupSSLFinchTrial();  /* From ssl_errors_common.js. */

https://codereview.chromium.org/341653006/diff/40001/chrome/browser/resources...
File chrome/browser/resources/ssl/ssl_errors_common.js (right):

https://codereview.chromium.org/341653006/diff/40001/chrome/browser/resources...
chrome/browser/resources/ssl/ssl_errors_common.js:46: // For the M37 Finch
trial.
is there a bug you could link to here so authors can remove this code when it's
done (or otherwise track progress)?

https://codereview.chromium.org/341653006/diff/40001/chrome/browser/ssl/ssl_b...
File chrome/browser/ssl/ssl_blocking_page.cc (right):

https://codereview.chromium.org/341653006/diff/40001/chrome/browser/ssl/ssl_b...
chrome/browser/ssl/ssl_blocking_page.cc:68: const char* GetTrialCondition() {
nit: makes the code smaller and easier to read

  CommandLine* cli = CommandLine::ForCurrentProcess();
  std::string name(base::FieldTrialList::FindFullName(kInterstitialTrialName));

  if (cli->HasSwitch(switches::kSSLInterstitialV1) || name == kConvV1)
    return kCondV1;

https://codereview.chromium.org/341653006/diff/40001/chrome/browser/ssl/ssl_b...
chrome/browser/ssl/ssl_blocking_page.cc:331: strings.SetString("moreInfo5",
std::string());
^ don't these need to be base::string16()?

https://codereview.chromium.org/341653006/diff/40001/chrome/browser/ssl/ssl_b...
chrome/browser/ssl/ssl_blocking_page.cc:358:
IDS_SSL_OVERRIDABLE_PAGE_SHOULD_NOT_PROCEED));
opt nit:

  strings.SetString("reasonForNotProceeding",
      l10n_util::GetStringUTF16(IDS_SSL_OVERRIDABLE_PAGE_SHOULD_NOT_PROCEED));

[Dan Beam, 2014-06-20 21:28:27]

most advanced*

[felt, 2014-06-21 00:23:16]

> this is officially the most advance interstitial known to humankind

sometimes when people ignore ssl warnings they end up being tortured, so

https://codereview.chromium.org/341653006/diff/40001/chrome/browser/resources...
File chrome/browser/resources/ssl/interstitial_v2.css (right):

https://codereview.chromium.org/341653006/diff/40001/chrome/browser/resources...
chrome/browser/resources/ssl/interstitial_v2.css:121: .safe-browsing small-link
{
On 2014/06/20 21:28:19, Dan Beam wrote:
> nit: .small-link, probably? (unless you're actually using a <small-link>)
> 
> also
> 
>   .safe-browsing :-webkit-any(
>       a, #details, #details-button, h1, small-link) {
>     ...
>   }

What's the benefit of the :-webkit-any here?

https://codereview.chromium.org/341653006/diff/40001/chrome/browser/resources...
chrome/browser/resources/ssl/interstitial_v2.css:159:
url('images/2x/brokenssl_guard.png') 2x) !important;
On 2014/06/20 21:28:19, Dan Beam wrote:
> !important :(

this might be because I am clumsy at CSS. is there a better way to tell the
browser that this combination of two classes should take priority over the other
two?

https://codereview.chromium.org/341653006/diff/40001/chrome/browser/resources...
chrome/browser/resources/ssl/interstitial_v2.css:159:
url('images/2x/brokenssl_guard.png') 2x) !important;
On 2014/06/20 21:28:19, Dan Beam wrote:
> url('...') -> url(...) everywhere

Done.

https://codereview.chromium.org/341653006/diff/40001/chrome/browser/resources...
File chrome/browser/resources/ssl/interstitial_v2.js (right):

https://codereview.chromium.org/341653006/diff/40001/chrome/browser/resources...
chrome/browser/resources/ssl/interstitial_v2.js:16: setupSSLFinchTrial();
On 2014/06/20 21:28:20, Dan Beam wrote:
> setupSSLFinchTrial();  /* From ssl_errors_common.js. */

Done.

https://codereview.chromium.org/341653006/diff/40001/chrome/browser/resources...
File chrome/browser/resources/ssl/ssl_errors_common.js (right):

https://codereview.chromium.org/341653006/diff/40001/chrome/browser/resources...
chrome/browser/resources/ssl/ssl_errors_common.js:46: // For the M37 Finch
trial.
On 2014/06/20 21:28:20, Dan Beam wrote:
> is there a bug you could link to here so authors can remove this code when
it's
> done (or otherwise track progress)?

At this point it looks like this perhaps shall be the M-38 finch trial
instead... but bug has been linkified.

https://codereview.chromium.org/341653006/diff/40001/chrome/browser/ssl/ssl_b...
File chrome/browser/ssl/ssl_blocking_page.cc (right):

https://codereview.chromium.org/341653006/diff/40001/chrome/browser/ssl/ssl_b...
chrome/browser/ssl/ssl_blocking_page.cc:68: const char* GetTrialCondition() {
On 2014/06/20 21:28:20, Dan Beam wrote:
> nit: makes the code smaller and easier to read
> 
>   CommandLine* cli = CommandLine::ForCurrentProcess();
>   std::string
name(base::FieldTrialList::FindFullName(kInterstitialTrialName));
> 
>   if (cli->HasSwitch(switches::kSSLInterstitialV1) || name == kConvV1)
>     return kCondV1;

Done.

https://codereview.chromium.org/341653006/diff/40001/chrome/browser/ssl/ssl_b...
chrome/browser/ssl/ssl_blocking_page.cc:331: strings.SetString("moreInfo5",
std::string());
On 2014/06/20 21:28:20, Dan Beam wrote:
> ^ don't these need to be base::string16()?

Done, although it doesn't really matter for the empty string I think?

https://codereview.chromium.org/341653006/diff/40001/chrome/browser/ssl/ssl_b...
chrome/browser/ssl/ssl_blocking_page.cc:358:
IDS_SSL_OVERRIDABLE_PAGE_SHOULD_NOT_PROCEED));
On 2014/06/20 21:28:20, Dan Beam wrote:
> opt nit:
> 
>   strings.SetString("reasonForNotProceeding",
>       l10n_util::GetStringUTF16(IDS_SSL_OVERRIDABLE_PAGE_SHOULD_NOT_PROCEED));

sure

[Dan Beam, 2014-06-21 00:42:52]

https://codereview.chromium.org/341653006/diff/40001/chrome/browser/resources...
File chrome/browser/resources/ssl/interstitial_v2.css (right):

https://codereview.chromium.org/341653006/diff/40001/chrome/browser/resources...
chrome/browser/resources/ssl/interstitial_v2.css:121: .safe-browsing small-link
{
On 2014/06/21 00:23:15, felt wrote:
> On 2014/06/20 21:28:19, Dan Beam wrote:
> > nit: .small-link, probably? (unless you're actually using a <small-link>)
> > 
> > also
> > 
> >   .safe-browsing :-webkit-any(
> >       a, #details, #details-button, h1, small-link) {
> >     ...
> >   }
> 
> What's the benefit of the :-webkit-any here?

less code

https://codereview.chromium.org/341653006/diff/40001/chrome/browser/resources...
chrome/browser/resources/ssl/interstitial_v2.css:159:
url('images/2x/brokenssl_guard.png') 2x) !important;
On 2014/06/21 00:23:15, felt wrote:
> On 2014/06/20 21:28:19, Dan Beam wrote:
> > !important :(
> 
> this might be because I am clumsy at CSS. is there a better way to tell the
> browser that this combination of two classes should take priority over the
other
> two?

https://developer.mozilla.org/en-US/docs/Web/CSS/Specificity

https://codereview.chromium.org/341653006/diff/60001/chrome/browser/resources...
File chrome/browser/resources/ssl/ssl_errors_common.js (right):

https://codereview.chromium.org/341653006/diff/60001/chrome/browser/resources...
chrome/browser/resources/ssl/ssl_errors_common.js:46: // For the M37 Finch
trial. crbug.com/386033
nit: http:// in front

https://codereview.chromium.org/341653006/diff/60001/chrome/browser/ssl/ssl_b...
File chrome/browser/ssl/ssl_blocking_page.cc (right):

https://codereview.chromium.org/341653006/diff/60001/chrome/browser/ssl/ssl_b...
chrome/browser/ssl/ssl_blocking_page.cc:70: std::string
name(base::FieldTrialList::FindFullName(kInterstitialTrialName));
nit: declare |name| right before it's needed

https://codereview.chromium.org/341653006/diff/60001/chrome/browser/ssl/ssl_b...
chrome/browser/ssl/ssl_blocking_page.cc:326: strings.SetString("moreInfo5",
std::string16());
dare you to compile this ;)

[felt, 2014-06-21 00:56:32]

https://codereview.chromium.org/341653006/diff/60001/chrome/browser/resources...
File chrome/browser/resources/ssl/ssl_errors_common.js (right):

https://codereview.chromium.org/341653006/diff/60001/chrome/browser/resources...
chrome/browser/resources/ssl/ssl_errors_common.js:46: // For the M37 Finch
trial. crbug.com/386033
On 2014/06/21 00:42:52, Dan Beam wrote:
> nit: http:// in front

Done.

https://codereview.chromium.org/341653006/diff/60001/chrome/browser/ssl/ssl_b...
File chrome/browser/ssl/ssl_blocking_page.cc (right):

https://codereview.chromium.org/341653006/diff/60001/chrome/browser/ssl/ssl_b...
chrome/browser/ssl/ssl_blocking_page.cc:70: std::string
name(base::FieldTrialList::FindFullName(kInterstitialTrialName));
On 2014/06/21 00:42:52, Dan Beam wrote:
> nit: declare |name| right before it's needed

Done.

https://codereview.chromium.org/341653006/diff/60001/chrome/browser/ssl/ssl_b...
chrome/browser/ssl/ssl_blocking_page.cc:326: strings.SetString("moreInfo5",
std::string16());
On 2014/06/21 00:42:52, Dan Beam wrote:
> dare you to compile this ;)

http://goo.gl/xR1Dky

[Dan Beam, 2014-06-21 02:13:47]

https://codereview.chromium.org/341653006/diff/120001/chrome/browser/resource...
File chrome/browser/resources/ssl/interstitial_v2.css (right):

https://codereview.chromium.org/341653006/diff/120001/chrome/browser/resource...
chrome/browser/resources/ssl/interstitial_v2.css:161:
url(images/2x/brokenssl_guard.png) 2x) !important;
^ ping on removing !important

[felt, 2014-06-21 02:24:44]

https://codereview.chromium.org/341653006/diff/120001/chrome/browser/resource...
File chrome/browser/resources/ssl/interstitial_v2.css (right):

https://codereview.chromium.org/341653006/diff/120001/chrome/browser/resource...
chrome/browser/resources/ssl/interstitial_v2.css:161:
url(images/2x/brokenssl_guard.png) 2x) !important;
On 2014/06/21 02:13:47, Dan Beam wrote:
> ^ ping on removing !important

it has been banished

[Dan Beam, 2014-06-21 02:44:50]

lgtm

[felt, 2014-06-21 03:06:34]

The CQ bit was checked by felt@chromium.org

[commit-bot: I haz the power, 2014-06-21 03:07:56]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/felt@chromium.org/341653006/140001

[commit-bot: I haz the power, 2014-06-21 06:55:28]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-06-21 13:44:24]

Change committed as 278969