We should implement more telemetry for measuring how successful the upgrading of content is happening within Firefox implemented in Bug 1435733. I think to measure the following data points would be helpful: - Failure of non upgrade HTTPS - Failure of browser upgrade HTTPS - Success of non upgrade HTTPS - Success of browser upgrade HTTPS Questions: - Should timeouts be measured separately? - Should CSP and HSTS upgrades be measured separately? - Would implementing this on a channel be the right direction? - Do we have other existing telemetry that might be useful to monitor here? Initial telemetry suggests these loads are over 0.37% of requests: https://telemetry.mozilla.org/new-pipeline/dist.html#!cumulative=0&end_date=2018-02-22&keys=__none__!__none__!__none__&max_channel_version=nightly%252F60&measure=HTTP_SCHEME_UPGRADE_TYPE&min_channel_version=null&processType=*&product=Firefox&sanitize=0&sort_keys=submissions&start_date=2018-01-29&table=0&trim=1&use_submission_date=0

We should get that telemtry update landed ASAP, otherwise the whole study does not provide any results.

Comment on attachment 8955902 [details] Bug 1440701 - Adding in telemetry for upgrading display content. https://reviewboard.mozilla.org/r/224878/#review230858 ::: netwerk/protocol/http/nsHttpChannel.cpp:7298 (Diff revision 1) > + "security.mixed_content.upgrade_display_content", false); > + } > + > + if (mLoadInfo && sIsUpgradableDisplayContentPrefEnabled) { > + ChannelDisposition upgradeChanDisposition = chanDisposition; > + if (mLoadInfo->GetBrowserUpgradeInsecureRequests()) { Add a comment explaining what we are doing here and why. ::: toolkit/components/telemetry/Histograms.json:2884 (Diff revision 1) > "n_values": 16, > "releaseChannelCollection": "opt-out", > "description": "Channel Disposition: 0=Cancel, 1=Disk, 2=NetOK, 3=NetEarlyFail, 4=NetlateFail, +8 for HTTPS" > }, > + "HTTP_CHANNEL_DISPOSITION_UPGRADE" : { > + "record_in_processes": ["main", "content"], nsHttpChannel only lives in the main channel. You may want to not record in the content process.

Thanks Valentin! I'm thinking we might want to use a keyed histogram here similar to the HTTP redirects one. This would allow different scenarios to be separated into histograms. My thought for this was that I think it would be worth having failure telemetry on display content that would have been upgraded if the pref was off, it looks like http failure rate is actually much higher than https anyway. However we might want to move adding this into another bug as it will require more keys on the loadInfo etc.

Comment on attachment 8955902 [details] Bug 1440701 - Adding in telemetry for upgrading display content. https://reviewboard.mozilla.org/r/224878/#review230912 Hey, besides my comments about the patch I don't understand how that patch actually works. Maybe I just don't understand what we are trying to do. Please summarize what you would try to measure to make sure we are not missing anything. Once document and my suggestions incoroporated, please flag me again. ::: netwerk/protocol/http/nsHttpChannel.cpp:7293 (Diff revision 1) > + static bool sIsUpgradableDisplayContentPrefEnabled; > + static bool sIsInited = false; > + if (!sIsInited) { > + sIsInited = true; > + Preferences::AddBoolVarCache(&sIsUpgradableDisplayContentPrefEnabled, > + "security.mixed_content.upgrade_display_content", false); Ideally that would be encapsulated within a static function within nsMixedContentBlocker which would then allow to remove sIsUpgradableDisplayContentPrefEnabled from nsContentUtils.cpp so we have that info within on place and do not need to call AddBoolVarCache twice. ::: toolkit/components/telemetry/Histograms.json:2887 (Diff revision 1) > }, > + "HTTP_CHANNEL_DISPOSITION_UPGRADE" : { > + "record_in_processes": ["main", "content"], > + "alert_emails": ["necko@mozilla.com", "seceng-telemetry@mozilla.com", "jkt@mozilla.com"], > + "bug_numbers": [1440701], > + "expires_in_version": "never", I thought we don't allow 'never' anymore. I am personally fine with it, but you need to get a data-review from a peer anyway. I think you can flag francois.

Comment on attachment 8955902 [details] Bug 1440701 - Adding in telemetry for upgrading display content. https://reviewboard.mozilla.org/r/224878/#review231662 yep, that looks good to me. Please don't forget to get a data peer sign off on it!

Could I get a data review francois?

Comment on attachment 8958983 [details] request-1440701.md 1) Is there or will there be **documentation** that describes the schema for the ultimate data set available publicly, complete and accurate? Yes, in Histograms.json. 2) Is there a control mechanism that allows the user to turn the data collection on and off? Yes, telemetry setting. 3) If the request is for permanent data collection, is there someone who will monitor the data over time?** Not permanent. 4) Using the **[category system of data types](https://wiki.mozilla.org/Firefox/Data_Collection)** on the Mozilla wiki, what collection type of data do the requested measurements fall under? ** Category 1 5) Is the data collection request for default-on or default-off? Default-on everywhere. 6) Does the instrumentation include the addition of **any *new* identifiers** (whether anonymous or otherwise; e.g., username, random IDs, etc. See the appendix for more details)? No. 7) Is the data collection covered by the existing Firefox privacy notice? Yes. 8) Does there need to be a check-in in the future to determine whether to renew the data? No, telemetry alerts are fine.

Pushed by jkingston@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/f61cd15b68a0 Adding in telemetry for upgrading display content. r=ckerschb,valentin