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 id19684
push usercbook@mozilla.com
push dateWed, 15 Jun 2016 00:05:56 +0000
treeherderfx-team@ae9a292741ef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbkelly
bugs1278778
milestone50.0a1
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();