Adding 3 new SSL experimental conditions

This adds 3 more experimental conditions for controlled experiments on security interstitials. These are the 3 with images that were missing previously.

The images themselves are in Issue 17279009 so that I could dcommit the binary files but send the code changes through the CQ.

BUG=224070
TBR=abarth@chromium.org

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

[felt, 2013-06-20 06:16:13]

Adam: Can you please review a tiny change to ssl_blocking_page.cc? The previous
CL for this experiment was missing these images, which I now have.

[felt, 2013-06-20 06:16:36]

Dan: Can you please review the changes to roadblock.html? The previous CL for
this experiment was missing these images, which I now have.

[agl, 2013-06-20 14:57:34]

The .cc change LGTM, but I think it's a noop: it's adding unit-local values that
aren't referenced. The compiler will just throw them away. Did you mean to use
those values in some way?

[felt, 2013-06-20 15:00:59]

On 2013/06/20 14:57:34, agl wrote:
> The .cc change LGTM, but I think it's a noop: it's adding unit-local values
that
> aren't referenced. The compiler will just throw them away. Did you mean to use
> those values in some way?

We don't actually need to reference them (since the value is populated into
trialCondition_) but I wanted to add them so it's clear what all of the possible
values of trialCondition_ are.

[agl, 2013-06-20 15:03:15]

lgtm

[Dan Beam, 2013-06-20 17:52:12]

https://codereview.chromium.org/17416003/diff/1/chrome/browser/resources/ssl/...
File chrome/browser/resources/ssl/roadblock.html (right):

https://codereview.chromium.org/17416003/diff/1/chrome/browser/resources/ssl/...
chrome/browser/resources/ssl/roadblock.html:71: 
html[dir='rtl'] .test-image {
  float: left;
}

https://codereview.chromium.org/17416003/diff/1/chrome/browser/resources/ssl/...
chrome/browser/resources/ssl/roadblock.html:171: function setupIconExperiments()
{
nit: the JS haxor way that also works in RTL -

var type = templateData.trialType.split('SSL').slice(-1).toLowerCase();
if (/policeman|stoplight|badguy/.test(type)) {
  $('trial-' + type).hidden = false;
  $('more-info-short').style.webkitMarginEnd = '30px';
  $('more-info-long').style.webkitMarginEnd = '30px';
}

https://codereview.chromium.org/17416003/diff/1/chrome/browser/resources/ssl/...
chrome/browser/resources/ssl/roadblock.html:171: function setupIconExperiments()
{
can you ensure this isn't called multiple times or handles this, i.e.:

  assert(!document.querySelector('.test-image:not([hidden])'));

[felt, 2013-06-20 19:57:58]

Hi Dan, this will never run on RTL languages (it's restricted to en) but it
won't hurt so I made the changes anyway.

Adrienne

https://codereview.chromium.org/17416003/diff/1/chrome/browser/resources/ssl/...
File chrome/browser/resources/ssl/roadblock.html (right):

https://codereview.chromium.org/17416003/diff/1/chrome/browser/resources/ssl/...
chrome/browser/resources/ssl/roadblock.html:71: 
On 2013/06/20 17:52:12, Dan Beam wrote:
> html[dir='rtl'] .test-image {
>   float: left;
> }

Done.

https://codereview.chromium.org/17416003/diff/1/chrome/browser/resources/ssl/...
chrome/browser/resources/ssl/roadblock.html:171: function setupIconExperiments()
{
I added a boolean.

On 2013/06/20 17:52:12, Dan Beam wrote:
> can you ensure this isn't called multiple times or handles this, i.e.:
> 
>   assert(!document.querySelector('.test-image:not([hidden])'));

https://codereview.chromium.org/17416003/diff/1/chrome/browser/resources/ssl/...
chrome/browser/resources/ssl/roadblock.html:171: function setupIconExperiments()
{
On 2013/06/20 17:52:12, Dan Beam wrote:
> nit: the JS haxor way that also works in RTL -
> 
> var type = templateData.trialType.split('SSL').slice(-1).toLowerCase();
> if (/policeman|stoplight|badguy/.test(type)) {
>   $('trial-' + type).hidden = false;
>   $('more-info-short').style.webkitMarginEnd = '30px';
>   $('more-info-long').style.webkitMarginEnd = '30px';
> }

Done.

[felt, 2013-06-20 19:59:27]

abarth: I know you don't do SSL stuff these days, but FYI I've TBR'ed you as an
OWNER.

[Dan Beam, 2013-06-20 20:58:08]

lgtm

https://codereview.chromium.org/17416003/diff/18001/chrome/browser/resources/...
File chrome/browser/resources/ssl/roadblock.html (right):

https://codereview.chromium.org/17416003/diff/18001/chrome/browser/resources/...
chrome/browser/resources/ssl/roadblock.html:178: else setupExperiment = true;
nit: no else after if () { return; }

https://codereview.chromium.org/17416003/diff/18001/chrome/browser/resources/...
chrome/browser/resources/ssl/roadblock.html:178: else setupExperiment = true;
nit: assert(!setupExperiment); (if you plan on never hitting this code)

[felt, 2013-06-20 21:57:04]

dan, quick question about assert() below

https://codereview.chromium.org/17416003/diff/18001/chrome/browser/resources/...
File chrome/browser/resources/ssl/roadblock.html (right):

https://codereview.chromium.org/17416003/diff/18001/chrome/browser/resources/...
chrome/browser/resources/ssl/roadblock.html:178: else setupExperiment = true;
On 2013/06/20 20:58:08, Dan Beam wrote:
> nit: no else after if () { return; }

Done.

https://codereview.chromium.org/17416003/diff/18001/chrome/browser/resources/...
chrome/browser/resources/ssl/roadblock.html:178: else setupExperiment = true;
On 2013/06/20 20:58:08, Dan Beam wrote:
> nit: assert(!setupExperiment); (if you plan on never hitting this code)

Hi Dan, where does the assert function come from? It doesn't exist in the scope
of this file, is there a script you want me to src?

[Dan Beam, 2013-06-20 22:26:21]

https://codereview.chromium.org/17416003/diff/18001/chrome/browser/resources/...
File chrome/browser/resources/ssl/roadblock.html (right):

https://codereview.chromium.org/17416003/diff/18001/chrome/browser/resources/...
chrome/browser/resources/ssl/roadblock.html:178: else setupExperiment = true;
On 2013/06/20 21:57:04, felt wrote:
> On 2013/06/20 20:58:08, Dan Beam wrote:
> > nit: assert(!setupExperiment); (if you plan on never hitting this code)
> 
> Hi Dan, where does the assert function come from? It doesn't exist in the
scope
> of this file, is there a script you want me to src?

src/ui/webui/resources/js/util.js

[commit-bot: I haz the power, 2013-06-20 23:54:16]

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

[commit-bot: I haz the power, 2013-06-21 10:03:14]

Change committed as 207781