Bug 1278778 - Show JS stack traces of Fetch requests in Netmonitor r=bkelly
authorJarda Snajdr <jsnajdr@gmail.com>
Tue, 14 Jun 2016 04:08:00 +0100
changeset 301726 ae9a292741efb2cf14f474eca487fcdff738cf6e
parent 301725 6763cbc32e3e31d65678327632f1e7748a0070f1
child 301727 22add247f72f2c361cc16d7c62895e529cfbf61f
push id30340
push usercbook@mozilla.com
push dateWed, 15 Jun 2016 05:21:46 +0000
treeherdermozilla-central@53f5b5c289fb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbkelly
bugs1278778
milestone50.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 1278778 - Show JS stack traces of Fetch requests in Netmonitor r=bkelly
devtools/client/netmonitor/test/browser_net_cause.js
devtools/client/netmonitor/test/html_cause-test-page.html
dom/fetch/Fetch.cpp
dom/fetch/FetchDriver.cpp
dom/fetch/FetchDriver.h
--- a/devtools/client/netmonitor/test/browser_net_cause.js
+++ b/devtools/client/netmonitor/test/browser_net_cause.js
@@ -36,21 +36,28 @@ const EXPECTED_REQUESTS = [
   {
     method: "GET",
     url: EXAMPLE_URL + "xhr_request",
     causeType: "xhr",
     causeUri: CAUSE_URL,
     hasStack: { fn: "performXhrRequest", file: CAUSE_FILE_NAME, line: 22 }
   },
   {
+    method: "GET",
+    url: EXAMPLE_URL + "fetch_request",
+    causeType: "fetch",
+    causeUri: CAUSE_URL,
+    hasStack: { fn: "performFetchRequest", file: CAUSE_FILE_NAME, line: 26 }
+  },
+  {
     method: "POST",
     url: EXAMPLE_URL + "beacon_request",
     causeType: "beacon",
     causeUri: CAUSE_URL,
-    hasStack: { fn: "performBeaconRequest", file: CAUSE_FILE_NAME, line: 26 }
+    hasStack: { fn: "performBeaconRequest", file: CAUSE_FILE_NAME, line: 30 }
   },
 ];
 
 var test = Task.async(function* () {
   // the initNetMonitor function clears the network request list after the
   // page is loaded. That's why we first load a bogus page from SIMPLE_URL,
   // and only then load the real thing from CAUSE_URL - we want to catch
   // all the requests the page is making, not only the XHRs.
--- a/devtools/client/netmonitor/test/html_cause-test-page.html
+++ b/devtools/client/netmonitor/test/html_cause-test-page.html
@@ -17,17 +17,22 @@
     <img src="img_request" />
     <script type="text/javascript">
       function performXhrRequest() {
         var xhr = new XMLHttpRequest();
         xhr.open("GET", "xhr_request", true);
         xhr.send();
       }
 
+      function performFetchRequest() {
+        fetch("fetch_request");
+      }
+
       function performBeaconRequest() {
         navigator.sendBeacon("beacon_request");
       }
 
       performXhrRequest();
+      performFetchRequest();
       performBeaconRequest();
     </script>
   </body>
 </html>
--- a/dom/fetch/Fetch.cpp
+++ b/dom/fetch/Fetch.cpp
@@ -119,33 +119,39 @@ public:
   {
     MOZ_ASSERT(mResolver);
   }
 
   NS_IMETHODIMP
   Run()
   {
     AssertIsOnMainThread();
+    RefPtr<FetchDriver> fetch;
     RefPtr<PromiseWorkerProxy> proxy = mResolver->mPromiseProxy;
-    MutexAutoLock lock(proxy->Lock());
-    if (proxy->CleanedUp()) {
-      NS_WARNING("Aborting Fetch because worker already shut down");
-      return NS_OK;
+
+    {
+      // Acquire the proxy mutex while getting data from the WorkerPrivate...
+      MutexAutoLock lock(proxy->Lock());
+      if (proxy->CleanedUp()) {
+        NS_WARNING("Aborting Fetch because worker already shut down");
+        return NS_OK;
+      }
+
+      nsCOMPtr<nsIPrincipal> principal = proxy->GetWorkerPrivate()->GetPrincipal();
+      MOZ_ASSERT(principal);
+      nsCOMPtr<nsILoadGroup> loadGroup = proxy->GetWorkerPrivate()->GetLoadGroup();
+      MOZ_ASSERT(loadGroup);
+      fetch = new FetchDriver(mRequest, principal, loadGroup);
     }
 
-    nsCOMPtr<nsIPrincipal> principal = proxy->GetWorkerPrivate()->GetPrincipal();
-    MOZ_ASSERT(principal);
-    nsCOMPtr<nsILoadGroup> loadGroup = proxy->GetWorkerPrivate()->GetLoadGroup();
-    MOZ_ASSERT(loadGroup);
-    RefPtr<FetchDriver> fetch = new FetchDriver(mRequest, principal, loadGroup);
-    nsresult rv = fetch->Fetch(mResolver);
-    // Right now we only support async fetch, which should never directly fail.
-    if (NS_WARN_IF(NS_FAILED(rv))) {
-      return rv;
-    }
+    // ...but release it before calling Fetch, because mResolver's callback can
+    // be called synchronously and they want the mutex, too.
+    fetch->Fetch(mResolver);
+
+    // FetchDriver::Fetch never directly fails
     return NS_OK;
   }
 };
 
 already_AddRefed<Promise>
 FetchRequest(nsIGlobalObject* aGlobal, const RequestOrUSVString& aInput,
              const RequestInit& aInit, ErrorResult& aRv)
 {
--- a/dom/fetch/FetchDriver.cpp
+++ b/dom/fetch/FetchDriver.cpp
@@ -78,30 +78,22 @@ FetchDriver::Fetch(FetchDriverObserver* 
   Telemetry::Accumulate(Telemetry::SERVICE_WORKER_REQUEST_PASSTHROUGH,
                         mRequest->WasCreatedByFetchEvent());
 
   // FIXME(nsm): Deal with HSTS.
 
   MOZ_RELEASE_ASSERT(!mRequest->IsSynchronous(),
                      "Synchronous fetch not supported");
 
-  return NS_DispatchToCurrentThread(NewRunnableMethod(this, &FetchDriver::ContinueFetch));
-}
-
-nsresult
-FetchDriver::ContinueFetch()
-{
-  workers::AssertIsOnMainThread();
-
-  nsresult rv = HttpFetch();
-  if (NS_FAILED(rv)) {
+  if (NS_FAILED(HttpFetch())) {
     FailWithNetworkError();
   }
 
-  return rv;
+  // Any failure is handled by FailWithNetworkError notifying the aObserver.
+  return NS_OK;
 }
 
 // This function implements the "HTTP Fetch" algorithm from the Fetch spec.
 // Functionality is often split between here, the CORS listener proxy and the
 // Necko HTTP implementation.
 nsresult
 FetchDriver::HttpFetch()
 {
--- a/dom/fetch/FetchDriver.h
+++ b/dom/fetch/FetchDriver.h
@@ -22,16 +22,21 @@ class nsILoadGroup;
 class nsIPrincipal;
 
 namespace mozilla {
 namespace dom {
 
 class InternalRequest;
 class InternalResponse;
 
+/**
+ * Provides callbacks to be called when response is available or on error.
+ * Implemenations usually resolve or reject the promise returned from fetch().
+ * The callbacks can be called synchronously or asynchronously from FetchDriver::Fetch.
+ */
 class FetchDriverObserver
 {
 public:
   FetchDriverObserver() : mGotResponseAvailable(false)
   { }
 
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(FetchDriverObserver);
   void OnResponseAvailable(InternalResponse* aResponse)
@@ -87,17 +92,16 @@ private:
   bool mFetchCalled;
 #endif
 
   FetchDriver() = delete;
   FetchDriver(const FetchDriver&) = delete;
   FetchDriver& operator=(const FetchDriver&) = delete;
   ~FetchDriver();
 
-  nsresult ContinueFetch();
   nsresult HttpFetch();
   // Returns the filtered response sent to the observer.
   already_AddRefed<InternalResponse>
   BeginAndGetFilteredResponse(InternalResponse* aResponse,
                               bool aFoundOpaqueRedirect);
   // Utility since not all cases need to do any post processing of the filtered
   // response.
   void FailWithNetworkError();