Bug 1356097 - Improve correctness of HTTP_PAGE_* telemetry r=mcmanus
authorValentin Gosu <valentin.gosu@gmail.com>
Mon, 29 May 2017 22:15:37 +0200
changeset 412096 952cbd41608c8626884b832fd4f8aee8cbad8ad9
parent 412095 37fb38a7ef4b36718d8f5653fff0a1c903f1bfc1
child 412097 4c960793ed252f422636b4777b115ea681c5789c
push id1490
push usermtabara@mozilla.com
push dateMon, 31 Jul 2017 14:08:16 +0000
treeherdermozilla-release@70e32e6bf15e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmcmanus
bugs1356097
milestone55.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 1356097 - Improve correctness of HTTP_PAGE_* telemetry r=mcmanus When the last request is removed from the load group, we report telemetry for the default load request. This was done without checking if the request was successful, which may cause us to report telemetry for failed requests as well. Also, the NullHttpChannel had its timingEnabled attribute set to true, which could lead us to report invalid telemetry MozReview-Commit-ID: 5w7rd2V17Xd
netwerk/base/nsLoadGroup.cpp
netwerk/protocol/http/NullHttpChannel.cpp
--- a/netwerk/base/nsLoadGroup.cpp
+++ b/netwerk/base/nsLoadGroup.cpp
@@ -820,17 +820,23 @@ nsLoadGroup::SetUserAgentOverrideCache(c
 }
 
 
 ////////////////////////////////////////////////////////////////////////////////
 
 void
 nsLoadGroup::TelemetryReport()
 {
-    if (mDefaultLoadIsTimed) {
+    nsresult defaultStatus = NS_ERROR_INVALID_ARG;
+    // We should only report HTTP_PAGE_* telemetry if the defaultRequest was
+    // actually successful.
+    if (mDefaultLoadRequest) {
+        mDefaultLoadRequest->GetStatus(&defaultStatus);
+    }
+    if (mDefaultLoadIsTimed && NS_SUCCEEDED(defaultStatus)) {
         Telemetry::Accumulate(Telemetry::HTTP_REQUEST_PER_PAGE, mTimedRequests);
         if (mTimedRequests) {
             Telemetry::Accumulate(Telemetry::HTTP_REQUEST_PER_PAGE_FROM_CACHE,
                                   mCachedRequests * 100 / mTimedRequests);
         }
 
         nsCOMPtr<nsITimedChannel> timedChannel =
             do_QueryInterface(mDefaultLoadRequest);
--- a/netwerk/protocol/http/NullHttpChannel.cpp
+++ b/netwerk/protocol/http/NullHttpChannel.cpp
@@ -547,17 +547,18 @@ NullHttpChannel::GetIsDocument(bool *aIs
 
 //-----------------------------------------------------------------------------
 // NullHttpChannel::nsITimedChannel
 //-----------------------------------------------------------------------------
 
 NS_IMETHODIMP
 NullHttpChannel::GetTimingEnabled(bool *aTimingEnabled)
 {
-  *aTimingEnabled = true;
+  // We don't want to report timing for null channels.
+  *aTimingEnabled = false;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 NullHttpChannel::SetTimingEnabled(bool aTimingEnabled)
 {
   return NS_ERROR_NOT_IMPLEMENTED;
 }