Bug 1423495 - Part6: Use threadsafe refcounting for nsServerTiming draft
authorValentin Gosu <valentin.gosu@gmail.com>
Tue, 24 Apr 2018 13:04:12 +0200
changeset 787149 abaec5fd4a8cefa5f6981c064f40a85ec49119c9
parent 786593 6861261b3b05e8ba378412e141c3a8cc029afa9d
push id107659
push uservalentin.gosu@gmail.com
push dateTue, 24 Apr 2018 11:12:27 +0000
bugs1423495
milestone61.0a1
Bug 1423495 - Part6: Use threadsafe refcounting for nsServerTiming * 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;
   }