Bug 1423495 - Part6: Use threadsafe refcounting for nsServerTiming r=baku,nwgh
authorValentin Gosu <valentin.gosu@gmail.com>
Tue, 24 Apr 2018 13:04:12 +0200
changeset 415404 fa8202661c163cf890873b2d6979db1a6917bebc
parent 415403 26e3cf3458025c17087b0219dea9064af67f1d80
child 415405 52805bf2d0f867b148e7f8e90ac1a3b08dd1ad3d
push id102567
push usercbrindusan@mozilla.com
push dateTue, 24 Apr 2018 21:59:46 +0000
treeherdermozilla-inbound@d3c93907c3ed [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbaku, nwgh
bugs1423495
milestone61.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 1423495 - Part6: Use threadsafe refcounting for nsServerTiming r=baku,nwgh * Also keeps the timing array as nsTArray<nsCOMPtr<nsIServerTiming>> instead of the scriptable nsIArray (which doesn't like being released on another thread) MozReview-Commit-ID: 37uPZJ38saQ
dom/performance/PerformanceResourceTiming.cpp
dom/performance/PerformanceTiming.cpp
dom/performance/PerformanceTiming.h
netwerk/base/nsITimedChannel.idl
netwerk/protocol/http/HttpBaseChannel.cpp
netwerk/protocol/http/NullHttpChannel.cpp
netwerk/protocol/http/nsServerTiming.h
--- a/dom/performance/PerformanceResourceTiming.cpp
+++ b/dom/performance/PerformanceResourceTiming.cpp
@@ -93,29 +93,20 @@ PerformanceResourceTiming::GetServerTimi
                             nsTArray<RefPtr<PerformanceServerTiming>>& aRetval,
                             Maybe<nsIPrincipal*>& aSubjectPrincipal)
 {
   aRetval.Clear();
   if (!TimingAllowedForCaller(aSubjectPrincipal)) {
     return;
   }
 
-  nsCOMPtr<nsIArray> serverTimingArray = mTimingData->GetServerTiming();
-  if (!serverTimingArray) {
-    return;
-  }
-
-  uint32_t length = 0;
-  if (NS_WARN_IF(NS_FAILED(serverTimingArray->GetLength(&length)))) {
-    return;
-  }
-
+  nsTArray<nsCOMPtr<nsIServerTiming>> serverTimingArray = mTimingData->GetServerTiming();
+  uint32_t length = serverTimingArray.Length();
   for (uint32_t index = 0; index < length; ++index) {
-    nsCOMPtr<nsIServerTiming> serverTiming =
-      do_QueryElementAt(serverTimingArray, index);
+    nsCOMPtr<nsIServerTiming> serverTiming = serverTimingArray.ElementAt(index);
     MOZ_ASSERT(serverTiming);
 
     aRetval.AppendElement(
       new PerformanceServerTiming(GetParentObject(), serverTiming));
   }
 }
 
 bool
--- a/dom/performance/PerformanceTiming.cpp
+++ b/dom/performance/PerformanceTiming.cpp
@@ -157,17 +157,17 @@ PerformanceTimingData::PerformanceTiming
     aChannel->GetCacheReadEnd(&mCacheReadEnd);
 
     aChannel->GetDispatchFetchEventStart(&mWorkerStart);
     aChannel->GetHandleFetchEventStart(&mWorkerRequestStart);
     // TODO: Track when FetchEvent.respondWith() promise resolves as
     //       ServiceWorker interception responseStart?
     aChannel->GetHandleFetchEventEnd(&mWorkerResponseEnd);
 
-    aChannel->GetServerTiming(getter_AddRefs(mServerTiming));
+    aChannel->GetNativeServerTiming(mServerTiming);
 
     // The performance timing api essentially requires that the event timestamps
     // have a strict relation with each other. The truth, however, is the
     // browser engages in a number of speculative activities that sometimes mean
     // connections and lookups begin at different times. Workaround that here by
     // clamping these values to what we expect FetchStart to be.  This means the
     // later of AsyncOpen or WorkerStart times.
     if (!mAsyncOpen.IsNull()) {
@@ -668,23 +668,22 @@ PerformanceTiming::IsTopLevelContentDocu
   nsCOMPtr<nsIDocShellTreeItem> rootItem;
   Unused << docShell->GetSameTypeRootTreeItem(getter_AddRefs(rootItem));
   if (rootItem.get() != static_cast<nsIDocShellTreeItem*>(docShell.get())) {
     return false;
   }
   return rootItem->ItemType() == nsIDocShellTreeItem::typeContent;
 }
 
-already_AddRefed<nsIArray>
-PerformanceTimingData::GetServerTiming() const
+nsTArray<nsCOMPtr<nsIServerTiming>>
+PerformanceTimingData::GetServerTiming()
 {
   if (!nsContentUtils::IsPerformanceTimingEnabled() || !IsInitialized() ||
       !TimingAllowed() ||
       nsContentUtils::ShouldResistFingerprinting()) {
-    return nullptr;
+    return nsTArray<nsCOMPtr<nsIServerTiming>>();
   }
 
-  nsCOMPtr<nsIArray> serverTiming = mServerTiming;
-  return serverTiming.forget();
+  return nsTArray<nsCOMPtr<nsIServerTiming>>(mServerTiming);
 }
 
 } // dom namespace
 } // mozilla namespace
--- a/dom/performance/PerformanceTiming.h
+++ b/dom/performance/PerformanceTiming.h
@@ -8,16 +8,17 @@
 #define mozilla_dom_PerformanceTiming_h
 
 #include "mozilla/Attributes.h"
 #include "nsContentUtils.h"
 #include "nsDOMNavigationTiming.h"
 #include "nsRFPService.h"
 #include "nsWrapperCache.h"
 #include "Performance.h"
+#include "nsITimedChannel.h"
 
 class nsIHttpChannel;
 class nsITimedChannel;
 
 namespace mozilla {
 namespace dom {
 
 class PerformanceTiming;
@@ -164,26 +165,26 @@ public:
 
   // Cached result of CheckAllowedOrigin. If false, security sensitive
   // attributes of the resourceTiming object will be set to 0
   bool TimingAllowed() const
   {
     return mTimingAllowed;
   }
 
-  already_AddRefed<nsIArray> GetServerTiming() const;
+  nsTArray<nsCOMPtr<nsIServerTiming>> GetServerTiming();
 
 private:
   // Checks if the resource is either same origin as the page that started
   // the load, or if the response contains the Timing-Allow-Origin header
   // with a value of * or matching the domain of the loading Principal
   bool CheckAllowedOrigin(nsIHttpChannel* aResourceChannel,
                           nsITimedChannel* aChannel);
 
-  nsCOMPtr<nsIArray> mServerTiming;
+  nsTArray<nsCOMPtr<nsIServerTiming>> mServerTiming;
   nsString mNextHopProtocol;
 
   TimeStamp mAsyncOpen;
   TimeStamp mRedirectStart;
   TimeStamp mRedirectEnd;
   TimeStamp mDomainLookupStart;
   TimeStamp mDomainLookupEnd;
   TimeStamp mConnectStart;
--- a/netwerk/base/nsITimedChannel.idl
+++ b/netwerk/base/nsITimedChannel.idl
@@ -4,27 +4,31 @@
 
 #include "nsISupports.idl"
 interface nsIArray;
 interface nsIPrincipal;
 %{C++
 namespace mozilla {
 class TimeStamp;
 }
+#include "nsTArrayForwardDeclare.h"
+#include "nsCOMPtr.h"
 %}
 
 native TimeStamp(mozilla::TimeStamp);
 
 [scriptable, uuid(c2d9e95b-9cc9-4f47-9ef6-1de0cf7ebc75)]
 interface nsIServerTiming : nsISupports {
   [must_use] readonly attribute ACString name;
   [must_use] readonly attribute double duration;
   [must_use] readonly attribute ACString description;
 };
 
+[ref] native nsServerTimingArrayRef(nsTArray<nsCOMPtr<nsIServerTiming>>);
+
 // All properties return zero if the value is not available
 [scriptable, uuid(ca63784d-959c-4c3a-9a59-234a2a520de0)]
 interface nsITimedChannel : nsISupports {
   // Set this attribute to true to enable collection of timing data.
   // channelCreationTime will be available even with this attribute set to
   // false.
   attribute boolean timingEnabled;
 
@@ -105,9 +109,10 @@ interface nsITimedChannel : nsISupports 
   readonly attribute PRTime cacheReadEndTime;
   readonly attribute PRTime redirectStartTime;
   readonly attribute PRTime redirectEndTime;
 
   // If this attribute is false, this resource MUST NOT be reported in resource timing.
   [noscript] attribute boolean reportResourceTiming;
 
   readonly attribute nsIArray serverTiming;
+  nsServerTimingArrayRef getNativeServerTiming();
 };
--- a/netwerk/protocol/http/HttpBaseChannel.cpp
+++ b/netwerk/protocol/http/HttpBaseChannel.cpp
@@ -4561,52 +4561,64 @@ HttpBaseChannel::CallTypeSniffers(void *
   if (!newType.IsEmpty()) {
     chan->SetContentType(newType);
   }
 }
 
 template <class T>
 static void
 ParseServerTimingHeader(const nsAutoPtr<T> &aHeader,
-                        nsIMutableArray* aOutput)
+                        nsTArray<nsCOMPtr<nsIServerTiming>>& aOutput)
 {
   if (!aHeader) {
     return;
   }
 
   nsAutoCString serverTimingHeader;
   Unused << aHeader->GetHeader(nsHttp::Server_Timing, serverTimingHeader);
   if (serverTimingHeader.IsEmpty()) {
     return;
   }
 
   ServerTimingParser parser(serverTimingHeader);
   parser.Parse();
 
   nsTArray<nsCOMPtr<nsIServerTiming>> array = parser.TakeServerTimingHeaders();
-  for (const auto &data : array) {
-    aOutput->AppendElement(data);
-  }
+  aOutput.AppendElements(array);
 }
 
 NS_IMETHODIMP
 HttpBaseChannel::GetServerTiming(nsIArray **aServerTiming)
 {
+  nsresult rv;
   NS_ENSURE_ARG_POINTER(aServerTiming);
 
+  nsCOMPtr<nsIMutableArray> array = do_CreateInstance(NS_ARRAY_CONTRACTID, &rv);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  nsTArray<nsCOMPtr<nsIServerTiming>> data;
+  rv = GetNativeServerTiming(data);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  for (const auto &entry : data) {
+    array->AppendElement(entry);
+  }
+
+  array.forget(aServerTiming);
+  return NS_OK;
+}
+
+NS_IMETHODIMP
+HttpBaseChannel::GetNativeServerTiming(nsTArray<nsCOMPtr<nsIServerTiming>>& aServerTiming)
+{
+  aServerTiming.Clear();
+
   bool isHTTPS = false;
   if (NS_SUCCEEDED(mURI->SchemeIs("https", &isHTTPS)) && isHTTPS) {
-    nsTArray<nsCOMPtr<nsIServerTiming>> data;
-    nsresult rv = NS_OK;
-    nsCOMPtr<nsIMutableArray> array = do_CreateInstance(NS_ARRAY_CONTRACTID, &rv);
-    NS_ENSURE_SUCCESS(rv, rv);
-
-    ParseServerTimingHeader(mResponseHead, array);
-    ParseServerTimingHeader(mResponseTrailers, array);
-
-    array.forget(aServerTiming);
+    ParseServerTimingHeader(mResponseHead, aServerTiming);
+    ParseServerTimingHeader(mResponseTrailers, aServerTiming);
   }
 
   return NS_OK;
 }
 
 } // namespace net
 } // namespace mozilla
--- a/netwerk/protocol/http/NullHttpChannel.cpp
+++ b/netwerk/protocol/http/NullHttpChannel.cpp
@@ -896,16 +896,22 @@ NullHttpChannel::GetReportResourceTiming
 }
 
 NS_IMETHODIMP
 NullHttpChannel::GetServerTiming(nsIArray **aServerTiming)
 {
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
+NS_IMETHODIMP
+NullHttpChannel::GetNativeServerTiming(nsTArray<nsCOMPtr<nsIServerTiming>>& aServerTiming)
+{
+  return NS_ERROR_NOT_IMPLEMENTED;
+}
+
 #define IMPL_TIMING_ATTR(name)                                 \
 NS_IMETHODIMP                                                  \
 NullHttpChannel::Get##name##Time(PRTime* _retval) {            \
     TimeStamp stamp;                                           \
     Get##name(&stamp);                                         \
     if (stamp.IsNull()) {                                      \
         *_retval = 0;                                          \
         return NS_OK;                                          \
--- a/netwerk/protocol/http/nsServerTiming.h
+++ b/netwerk/protocol/http/nsServerTiming.h
@@ -9,17 +9,17 @@
 
 #include "nsITimedChannel.h"
 #include "nsString.h"
 #include "nsTArray.h"
 
 class nsServerTiming final : public nsIServerTiming
 {
 public:
-  NS_DECL_ISUPPORTS
+  NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSISERVERTIMING
 
   nsServerTiming() = default;
 
   void SetName(const nsACString &aName)
   {
     mName = aName;
   }