From 0b4287e91cfefdee4dbed6bfb34e6e89a159d6c2 Mon Sep 17 00:00:00 2001 From: Sebastian Hengst Date: Thu, 25 Aug 2016 14:22:47 +0200 Subject: [PATCH] Backed out changeset eb0c3d8a6d53 (bug 1281220) for crashing in testBug1217581 in rc3. r=backout on a CLOSED TREE --- .../android/chrome/content/aboutAccounts.js | 48 +++++++++++++++++-- .../mozilla/gecko/tests/testBug1217581.java | 2 +- toolkit/components/telemetry/Histograms.json | 32 +++++++++++++ .../telemetry/histogram-whitelists.json | 4 ++ 4 files changed, 82 insertions(+), 4 deletions(-) diff --git a/mobile/android/chrome/content/aboutAccounts.js b/mobile/android/chrome/content/aboutAccounts.js index 4801a76a1e74..873fcf41451d 100644 --- a/mobile/android/chrome/content/aboutAccounts.js +++ b/mobile/android/chrome/content/aboutAccounts.js @@ -64,18 +64,60 @@ var loadedDeferred = null; // We have a new load starting. Replace the existing promise with a new one, // and queue up the transition to remote content. -function deferTransitionToRemoteAfterLoaded() { +function deferTransitionToRemoteAfterLoaded(url) { log.d('Waiting for LOADED message.'); + // We are collecting data to understand the experience when using + // about:accounts to connect to the specific fxa-content-server hosted by + // Mozilla at accounts.firefox.com. However, we don't want to observe what + // alternate servers users might be using, so we can't collect the whole URL. + // Here, we filter the data based on whether the user is /not/ using + // accounts.firefox.com, and then record just the endpoint path. Other + // collected data could expose that the user is using Firefox Accounts, and + // together, that leaks the number of users not using accounts.firefox.com. + // We accept this leak: Mozilla already collects data about whether Sync (both + // legacy and FxA) is using a custom server in various situations: see the + // WEAVE_CUSTOM_* Telemetry histograms. + let recordResultTelemetry; // Defined only when not customized. + let isCustomized = Services.prefs.prefHasUserValue("identity.fxaccounts.remote.webchannel.uri"); + if (!isCustomized) { + // Turn "https://accounts.firefox.com/settings?context=fx_fennec_v1&email=EMAIL" into "/settings". + let key = Services.io.newURI(url, null, null).path.split("?")[0]; + + let startTime = Cu.now(); + + let start = Services.telemetry.getKeyedHistogramById('ABOUT_ACCOUNTS_CONTENT_SERVER_LOAD_STARTED_COUNT'); + start.add(key); + + recordResultTelemetry = function(success) { + let rate = Services.telemetry.getKeyedHistogramById('ABOUT_ACCOUNTS_CONTENT_SERVER_LOADED_RATE'); + rate.add(key, success); + + // We would prefer to use TelemetryStopwatch, but it doesn't yet support + // keyed histograms (see Bug 1205898). So we measure and store ourselves. + let delta = Cu.now() - startTime; // Floating point milliseconds, microsecond precision. + let time = success ? + Services.telemetry.getKeyedHistogramById('ABOUT_ACCOUNTS_CONTENT_SERVER_LOADED_TIME_MS') : + Services.telemetry.getKeyedHistogramById('ABOUT_ACCOUNTS_CONTENT_SERVER_FAILURE_TIME_MS'); + time.add(key, Math.round(delta)); + }; + } + loadedDeferred = PromiseUtils.defer(); loadedDeferred.promise.then(() => { log.d('Got LOADED message!'); document.getElementById("remote").style.opacity = 0; show("remote"); document.getElementById("remote").style.opacity = 1; + if (!isCustomized) { + recordResultTelemetry(true); + } }) .catch((e) => { log.w('Did not get LOADED message: ' + e.toString()); + if (!isCustomized) { + recordResultTelemetry(false); + } }); } @@ -90,7 +132,7 @@ var wrapper = { init: function (url) { this.url = url; - deferTransitionToRemoteAfterLoaded(); + deferTransitionToRemoteAfterLoaded(this.url); let iframe = document.getElementById("remote"); this.iframe = iframe; @@ -106,7 +148,7 @@ var wrapper = { }, retry: function () { - deferTransitionToRemoteAfterLoaded(); + deferTransitionToRemoteAfterLoaded(this.url); let webNav = this.iframe.frameLoader.docShell.QueryInterface(Ci.nsIWebNavigation); webNav.loadURI(this.url, Ci.nsIWebNavigation.LOAD_FLAGS_BYPASS_HISTORY, null, null, null); diff --git a/mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/testBug1217581.java b/mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/testBug1217581.java index 315041248d12..ce2bdfab0016 100644 --- a/mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/testBug1217581.java +++ b/mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/testBug1217581.java @@ -11,7 +11,7 @@ import org.mozilla.gecko.Telemetry; public class testBug1217581 extends BaseTest { // Take arbitrary histogram names used by Fennec. private static final String TEST_HISTOGRAM_NAME = "FENNEC_SYNC_NUMBER_OF_SYNCS_COMPLETED"; - private static final String TEST_KEYED_HISTOGRAM_NAME = "FENNEC_SYNC_NUMBER_OF_SYNCS_COMPLETED"; + private static final String TEST_KEYED_HISTOGRAM_NAME = "ABOUT_ACCOUNTS_CONTENT_SERVER_LOAD_STARTED_COUNT"; private static final String TEST_KEY_NAME = "testBug1217581"; diff --git a/toolkit/components/telemetry/Histograms.json b/toolkit/components/telemetry/Histograms.json index 44330db22698..e3a3ae968662 100644 --- a/toolkit/components/telemetry/Histograms.json +++ b/toolkit/components/telemetry/Histograms.json @@ -8741,6 +8741,38 @@ "kind": "count", "description": "Counts the number of times that a sync has failed because of trying to sync before server backoff interval has passed." }, + "ABOUT_ACCOUNTS_CONTENT_SERVER_LOAD_STARTED_COUNT": { + "alert_emails": ["mobile-frontend@mozilla.com"], + "expires_in_version": "50", + "kind": "count", + "keyed": true, + "description": "The number of remote content loads started. Keyed on fxa-content-server endpoint, like '/signin' or '/settings'." + }, + "ABOUT_ACCOUNTS_CONTENT_SERVER_LOADED_RATE": { + "alert_emails": ["mobile-frontend@mozilla.com"], + "expires_in_version": "50", + "kind": "boolean", + "keyed": true, + "description": "The number of remote content loads that fail (0) vs. receive the remote 'LOADED' message (1). Keyed on fxa-content-server endpoint path, like '/signin' or '/settings'." + }, + "ABOUT_ACCOUNTS_CONTENT_SERVER_LOADED_TIME_MS": { + "alert_emails": ["mobile-frontend@mozilla.com"], + "expires_in_version": "50", + "keyed": true, + "kind": "exponential", + "high": 60000, + "n_buckets": 50, + "description": "The length of time (in milliseconds) between starting remote content load and receiving the remote 'LOADED' message. Keyed on fxa-content-server endpoint path, like '/signin' or '/settings'." + }, + "ABOUT_ACCOUNTS_CONTENT_SERVER_FAILURE_TIME_MS": { + "alert_emails": ["mobile-frontend@mozilla.com"], + "expires_in_version": "50", + "keyed": true, + "kind": "exponential", + "high": 60000, + "n_buckets": 50, + "description": "The length of time (in milliseconds) between starting remote content load and failing with a connection error. Keyed on fxa-content-server endpoint path, like '/signin' or '/settings'." + }, "SLOW_SCRIPT_NOTICE_COUNT": { "alert_emails": ["perf-telemetry-alerts@mozilla.com"], "expires_in_version": "never", diff --git a/toolkit/components/telemetry/histogram-whitelists.json b/toolkit/components/telemetry/histogram-whitelists.json index 291d099a1165..c82eb999169f 100644 --- a/toolkit/components/telemetry/histogram-whitelists.json +++ b/toolkit/components/telemetry/histogram-whitelists.json @@ -798,6 +798,10 @@ "A11Y_INSTANTIATED_FLAG", "A11Y_ISIMPLEDOM_USAGE_FLAG", "A11Y_UPDATE_TIME", + "ABOUT_ACCOUNTS_CONTENT_SERVER_FAILURE_TIME_MS", + "ABOUT_ACCOUNTS_CONTENT_SERVER_LOADED_RATE", + "ABOUT_ACCOUNTS_CONTENT_SERVER_LOADED_TIME_MS", + "ABOUT_ACCOUNTS_CONTENT_SERVER_LOAD_STARTED_COUNT", "ADDON_SHIM_USAGE", "APPLICATION_REPUTATION_COUNT", "APPLICATION_REPUTATION_LOCAL",