Closed
Bug 1278778
Opened 6 years ago
Closed 6 years ago
Show JS stack traces of Fetch requests in Netmonitor
Categories
(DevTools :: Netmonitor, defect, P2)
DevTools
Netmonitor
Tracking
(firefox50 fixed)
RESOLVED
FIXED
Firefox 50
Tracking | Status | |
---|---|---|
firefox50 | --- | fixed |
People
(Reporter: jsnajdr, Assigned: jsnajdr)
References
Details
Attachments
(1 file, 2 obsolete files)
6.71 KB,
patch
|
jsnajdr
:
review+
|
Details | Diff | Splinter Review |
This is a followup to bug 1134073. Netmonitor doesn't display stack traces of Fetch requests, because the nsIHttpChannel::asyncOpen2 is not called synchronously when there is still a JS stack, but rather scheduled to a next event loop tick at [1]. [1] https://dxr.mozilla.org/mozilla-central/source/dom/fetch/FetchDriver.cpp#86
Assignee | ||
Comment 1•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5130d07ad6da
Assignee | ||
Comment 2•6 years ago
|
||
Fixed by merging FetchDriver::ContinueFetch into FetchDriver::Fetch and removing the async call. Also added a devtools mochitest for the JS stacktrace of a fetch request. JS stacktraces in devtools start working after this, but a lot of other tests is broken - see the try run. Fetch requests from workers, error handling... My approach seems to be too naive. Ben, please, do you have any advice?
Attachment #8761159 -
Flags: feedback?(bkelly)
Comment 3•6 years ago
|
||
Comment on attachment 8761159 [details] [diff] [review] Show JS stack traces of Fetch requests in Netmonitor Review of attachment 8761159 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/fetch/FetchDriver.cpp @@ -82,5 @@ > > MOZ_RELEASE_ASSERT(!mRequest->IsSynchronous(), > "Synchronous fetch not supported"); > > - return NS_DispatchToCurrentThread(NewRunnableMethod(this, &FetchDriver::ContinueFetch)); Sorry, its harder than this. See this description of the problem: https://bugzilla.mozilla.org/show_bug.cgi?id=1206079#c8
Attachment #8761159 -
Flags: feedback?(bkelly) → feedback-
Priority: -- → P2
Assignee | ||
Comment 4•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7c6dd291730a
Assignee | ||
Comment 5•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=63a78f71565c
Assignee | ||
Comment 6•6 years ago
|
||
After reading the instructions in bug 1206079 and getting familiar with the code, I figured out there should be no problem with calling FetchDriverObserver's callbacks synchronously - all they do, after all, is resolving the promise (either directly or through the worker proxy). The only thing that needed fixing is correctly acquiring and releasing the PromiseWorkerProxy lock. To prevent deadlock, it needs to be already released when calling into FetchDriver::Fetch, because this method can call synchronously into WorkerFetchResolver, which wants to acquire the mutex, too.
Attachment #8762616 -
Flags: review?(bkelly)
Assignee | ||
Updated•6 years ago
|
Attachment #8761159 -
Attachment is obsolete: true
Comment 7•6 years ago
|
||
Comment on attachment 8762616 [details] [diff] [review] Show JS stack traces of Fetch requests in Netmonitor Review of attachment 8762616 [details] [diff] [review]: ----------------------------------------------------------------- Looks reasonable. r=me with comment addressed and assuming all tests continue to pass. ::: dom/fetch/Fetch.cpp @@ +143,4 @@ > } > + > + // ...but release it before calling Fetch, because mResolver's callback can > + // be called synchronously and they want the mutex, too. Please document that the FetchDriverObserver call backs can be called synchronously or asynchronously in FetchDriver.h.
Attachment #8762616 -
Flags: review?(bkelly) → review+
Assignee | ||
Comment 8•6 years ago
|
||
Assignee | ||
Updated•6 years ago
|
Attachment #8762616 -
Attachment is obsolete: true
Assignee | ||
Comment 9•6 years ago
|
||
Comment on attachment 8762652 [details] [diff] [review] Show JS stack traces of Fetch requests in Netmonitor Added a comment about FetchDriverObserver callback sync/async I ran two try builds with all tests and they seem to be OK. Lots of intermittents, but nothing that looks related to this. I also checked other recent test results on try and they tend to display the same kind of failures - trouble with clipboard etc.
Attachment #8762652 -
Flags: review+
Assignee | ||
Updated•6 years ago
|
Keywords: checkin-needed
Comment 10•6 years ago
|
||
Pushed by cbook@mozilla.com: https://hg.mozilla.org/integration/fx-team/rev/ae9a292741ef Show JS stack traces of Fetch requests in Netmonitor r=bkelly
Keywords: checkin-needed
Comment 11•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/ae9a292741ef
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
Updated•4 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•