Fix captive portal histogram in SSL interstitials.

Fix captive portal histogram in SSL interstitials.

The SSL interstitial currently records captive portal histograms in its
constructor, but captive portal detection results arrive after the
interstitial is displayed so the histograms are incomplete.
This CL moves the histogram recording to the interstitial's destructor.

BUG=335933
Committed: https://crrev.com/59114e6e4736489f3a30eb30acccc1b59829c5f1
Cr-Commit-Position: refs/heads/master@{#316018}

[meacer, 2015-02-12 01:21:01]

meacer@chromium.org changed reviewers:
+ felt@chromium.org, jwd@chromium.org

[meacer, 2015-02-12 01:21:02]

felt, jwd, can you please take a look?

felt: chrome/browser/ssl/*
jwd: histograms.xml

Thanks.

[felt, 2015-02-12 01:35:37]

https://codereview.chromium.org/914253004/diff/1/chrome/browser/ssl/ssl_error...
File chrome/browser/ssl/ssl_error_classification.cc (right):

https://codereview.chromium.org/914253004/diff/1/chrome/browser/ssl/ssl_error...
chrome/browser/ssl/ssl_error_classification.cc:63: CAPTIVE_PORTAL_ALL,
All older data will get bucketed incorrectly as a result. For example if you
were to look at all channels next week you'd see a mix of the two different
histograms and it would lead to really messy data. I recommend either adding ALL
at the end, or defining a new histogram for this.

[meacer, 2015-02-12 01:40:30]

On 2015/02/12 01:35:37, felt wrote:
>
https://codereview.chromium.org/914253004/diff/1/chrome/browser/ssl/ssl_error...
> File chrome/browser/ssl/ssl_error_classification.cc (right):
> 
>
https://codereview.chromium.org/914253004/diff/1/chrome/browser/ssl/ssl_error...
> chrome/browser/ssl/ssl_error_classification.cc:63: CAPTIVE_PORTAL_ALL,
> All older data will get bucketed incorrectly as a result. For example if you
> were to look at all channels next week you'd see a mix of the two different
> histograms and it would lead to really messy data. I recommend either adding
ALL
> at the end, or defining a new histogram for this.

Well, it's all incomplete anyways (we only have CAPTIVE_PORTAL_DETECTION_ENABLED
and CAPTIVE_PORTAL_DETECTION_ENABLED_OVERRIDABLE), so I don't have a problem
with this :)

[felt, 2015-02-12 05:09:40]

On 2015/02/12 01:40:30, Mustafa Emre Acer wrote:
> On 2015/02/12 01:35:37, felt wrote:
> >
>
https://codereview.chromium.org/914253004/diff/1/chrome/browser/ssl/ssl_error...
> > File chrome/browser/ssl/ssl_error_classification.cc (right):
> > 
> >
>
https://codereview.chromium.org/914253004/diff/1/chrome/browser/ssl/ssl_error...
> > chrome/browser/ssl/ssl_error_classification.cc:63: CAPTIVE_PORTAL_ALL,
> > All older data will get bucketed incorrectly as a result. For example if you
> > were to look at all channels next week you'd see a mix of the two different
> > histograms and it would lead to really messy data. I recommend either adding
> ALL
> > at the end, or defining a new histogram for this.
> 
> Well, it's all incomplete anyways (we only have
CAPTIVE_PORTAL_DETECTION_ENABLED
> and CAPTIVE_PORTAL_DETECTION_ENABLED_OVERRIDABLE), so I don't have a problem
> with this :)

if you really don't care about squashing the existing data, lgtm

[jwd, 2015-02-12 18:01:56]

lgtm

Same sentiment as felt@

[meacer, 2015-02-12 18:04:08]

The CQ bit was checked by meacer@chromium.org

[commit-bot: I haz the power, 2015-02-12 18:05:18]

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

[meacer, 2015-02-12 18:58:41]

Thanks!

[commit-bot: I haz the power, 2015-02-12 19:20:25]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-02-12 19:21:26]

Patchset 1 (id:??) landed as
https://crrev.com/59114e6e4736489f3a30eb30acccc1b59829c5f1
Cr-Commit-Position: refs/heads/master@{#316018}