Bug 1567462 - Resurrect telemetry for http redirects, r=valentin
authorJunior Hsu <juhsu@mozilla.com>
Tue, 10 Dec 2019 19:00:37 +0000
changeset 506357 f1be062c4e2a7462258eea0ee2dfad6b592ef1ed
parent 506356 c78f989b17befffd3009ce9fea4103a759bbbd71
child 506358 b67be4771a4b46bc00e316d67add73528428ec0f
push id102830
push userjuhsu@mozilla.com
push dateTue, 10 Dec 2019 23:51:47 +0000
treeherderautoland@f1be062c4e2a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersvalentin
bugs1567462
milestone73.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1567462 - Resurrect telemetry for http redirects, r=valentin Differential Revision: https://phabricator.services.mozilla.com/D53310
netwerk/base/nsIOService.cpp
netwerk/protocol/http/nsHttpHandler.cpp
netwerk/test/unit/test_redirect_protocol_telemetry.js
netwerk/test/unit/xpcshell.ini
toolkit/components/telemetry/Histograms.json
--- a/netwerk/base/nsIOService.cpp
+++ b/netwerk/base/nsIOService.cpp
@@ -630,16 +630,37 @@ nsresult nsIOService::AsyncOnChannelRedi
   nsCOMArray<nsIChannelEventSink> entries;
   mChannelEventSinks.GetEntries(entries);
   int32_t len = entries.Count();
   for (int32_t i = 0; i < len; ++i) {
     nsresult rv =
         helper->DelegateOnChannelRedirect(entries[i], oldChan, newChan, flags);
     if (NS_FAILED(rv)) return rv;
   }
+
+  nsCOMPtr<nsIHttpChannel> httpChan(do_QueryInterface(oldChan));
+
+  // Collect the redirection from HTTP(S) only.
+  if (httpChan) {
+    MOZ_ASSERT(NS_IsMainThread());
+    nsCOMPtr<nsIURI> newURI;
+    newChan->GetURI(getter_AddRefs(newURI));
+    MOZ_ASSERT(newURI);
+
+    nsAutoCString scheme;
+    newURI->GetScheme(scheme);
+    MOZ_ASSERT(!scheme.IsEmpty());
+
+    Telemetry::AccumulateCategoricalKeyed(
+        scheme,
+        oldChan->IsDocument()
+            ? Telemetry::LABELS_NETWORK_HTTP_REDIRECT_TO_SCHEME::topLevel
+            : Telemetry::LABELS_NETWORK_HTTP_REDIRECT_TO_SCHEME::subresource);
+  }
+
   return NS_OK;
 }
 
 nsresult nsIOService::CacheProtocolHandler(const char* scheme,
                                            nsIProtocolHandler* handler) {
   MOZ_ASSERT(NS_IsMainThread());
 
   for (unsigned int i = 0; i < NS_N(gScheme); i++) {
--- a/netwerk/protocol/http/nsHttpHandler.cpp
+++ b/netwerk/protocol/http/nsHttpHandler.cpp
@@ -126,17 +126,16 @@
 #define BROWSER_PREF(_pref) BROWSER_PREF_PREFIX _pref
 
 #define NS_HTTP_PROTOCOL_FLAGS \
   (URI_STD | ALLOWS_PROXY | ALLOWS_PROXY_HTTP | URI_LOADABLE_BY_ANYONE)
 
 //-----------------------------------------------------------------------------
 
 using mozilla::dom::Promise;
-using mozilla::Telemetry::LABELS_NETWORK_HTTP_REDIRECT_TO_SCHEME;
 
 namespace mozilla {
 namespace net {
 
 LazyLogModule gHttpLog("nsHttp");
 
 #ifdef ANDROID
 static nsCString GetDeviceModelId() {
@@ -819,31 +818,16 @@ void nsHttpHandler::NotifyObservers(nsIC
   LOG(("nsHttpHandler::NotifyObservers [chan=%p event=\"%s\"]\n", chan, event));
   nsCOMPtr<nsIObserverService> obsService = services::GetObserverService();
   if (obsService) obsService->NotifyObservers(chan, event, nullptr);
 }
 
 nsresult nsHttpHandler::AsyncOnChannelRedirect(
     nsIChannel* oldChan, nsIChannel* newChan, uint32_t flags,
     nsIEventTarget* mainThreadEventTarget) {
-  MOZ_ASSERT(NS_IsMainThread() && (oldChan && newChan));
-
-  nsCOMPtr<nsIURI> newURI;
-  newChan->GetURI(getter_AddRefs(newURI));
-  MOZ_ASSERT(newURI);
-
-  nsAutoCString scheme;
-  newURI->GetScheme(scheme);
-  MOZ_ASSERT(!scheme.IsEmpty());
-
-  Telemetry::AccumulateCategoricalKeyed(
-      scheme, oldChan->IsDocument()
-                  ? LABELS_NETWORK_HTTP_REDIRECT_TO_SCHEME::topLevel
-                  : LABELS_NETWORK_HTTP_REDIRECT_TO_SCHEME::subresource);
-
   // TODO E10S This helper has to be initialized on the other process
   RefPtr<nsAsyncRedirectVerifyHelper> redirectCallbackHelper =
       new nsAsyncRedirectVerifyHelper();
 
   return redirectCallbackHelper->Init(oldChan, newChan, flags,
                                       mainThreadEventTarget);
 }
 
new file mode 100644
--- /dev/null
+++ b/netwerk/test/unit/test_redirect_protocol_telemetry.js
@@ -0,0 +1,59 @@
+const { HttpServer } = ChromeUtils.import("resource://testing-common/httpd.js");
+const { TelemetryTestUtils } = ChromeUtils.import(
+  "resource://testing-common/TelemetryTestUtils.jsm"
+);
+
+function make_channel(url, callback, ctx) {
+  return NetUtil.newChannel({ uri: url, loadUsingSystemPrincipal: true });
+}
+
+add_task(async function check_protocols() {
+  let httpserv = new HttpServer();
+  httpserv.registerPathHandler("/redirect", redirectHandler);
+  httpserv.registerPathHandler("/content", contentHandler);
+  httpserv.start(-1);
+
+  var responseProtocol;
+
+  function redirectHandler(metadata, response) {
+    response.setStatusLine(metadata.httpVersion, 301, "Moved");
+    let location =
+      responseProtocol == "data"
+        ? "data:text/plain,test"
+        : `${responseProtocol}://localhost:${
+            httpserv.identity.primaryPort
+          }/content`;
+    response.setHeader("Location", location, false);
+    response.setHeader("Cache-Control", "no-cache", false);
+  }
+
+  function contentHandler(metadata, response) {
+    let responseBody = "Content body";
+    response.setHeader("Content-Type", "text/plain");
+    response.bodyOutputStream.write(responseBody, responseBody.length);
+  }
+
+  function make_test(protocol) {
+    do_get_profile();
+    let redirect_hist = TelemetryTestUtils.getAndClearKeyedHistogram(
+      "NETWORK_HTTP_REDIRECT_TO_SCHEME"
+    );
+    return new Promise(resolve => {
+      const URL = `http://localhost:${httpserv.identity.primaryPort}/redirect`;
+      responseProtocol = protocol;
+      let channel = make_channel(URL);
+      let p = new Promise(resolve1 =>
+        channel.asyncOpen(new ChannelListener(resolve1))
+      );
+      p.then((request, buffer) => {
+        TelemetryTestUtils.assertKeyedHistogramSum(redirect_hist, protocol, 1);
+        resolve();
+      });
+    });
+  }
+
+  await make_test("http");
+  await make_test("data");
+
+  await new Promise(resolve => httpserv.stop(resolve));
+});
--- a/netwerk/test/unit/xpcshell.ini
+++ b/netwerk/test/unit/xpcshell.ini
@@ -292,16 +292,17 @@ skip-if = os == "android"
 [test_redirect_canceled.js]
 [test_redirect_failure.js]
 [test_redirect_from_script.js]
 [test_redirect_from_script_after-open_passing.js]
 [test_redirect_passing.js]
 [test_redirect_loop.js]
 [test_redirect_baduri.js]
 [test_redirect_different-protocol.js]
+[test_redirect_protocol_telemetry.js]
 [test_reentrancy.js]
 [test_reopen.js]
 [test_resumable_channel.js]
 [test_resumable_truncate.js]
 [test_safeoutputstream.js]
 [test_schema_2_migration.js]
 [test_schema_3_migration.js]
 [test_simple.js]
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -2478,20 +2478,21 @@
     "kind": "exponential",
     "high": 60000,
     "n_buckets": 100,
     "description": "The delay caused by the e10s back pressure suspension(ms)"
   },
   "NETWORK_HTTP_REDIRECT_TO_SCHEME" :{
     "record_in_processes": ["main"],
     "products": ["firefox", "fennec", "geckoview"],
-    "alert_emails": ["necko@mozilla.com", "seceng-telemetry@mozilla.com", "jkt@mozilla.com"],
-    "bug_numbers": [1413512],
-    "expires_in_version": "64",
-    "kind": "categorical",
+    "alert_emails": ["necko@mozilla.com"],
+    "bug_numbers": [1413512, 1567462],
+    "expires_in_version": "never",
+    "kind": "categorical",
+    "releaseChannelCollection": "opt-out",
     "keyed": true,
     "description": "Count of the HTTP redirection that triggered by top-level document or by subresource, keyed by the URL scheme redirected to.",
     "labels": ["topLevel", "subresource"]
   },
   "NETWORK_CROSS_ORIGIN_STYLESHEET_CONTENT_TYPE": {
     "record_in_processes": ["main"],
     "products": ["firefox", "fennec", "geckoview"],
     "alert_emails": ["necko@mozilla.com", "vgosu@mozilla.com"],