1280266 - Netmonitor JS stack traces shouldn't include chrome frames

Status: RESOLVED FIXED

(DevTools :: Netmonitor, defect, P2)

Description

[Jarda Snajdr [:jsnajdr]]

Requests JS stack traces in Netmonitor sometimes show chrome:// stack frames. This is confusing for users - they should only see the JS code from the page content.

The chrome:// frames should be shown only in browser toolbox.

Two examples of this behavior:

1) When the page is reloaded, the main document request shows stack trace from the browser calling into webNav.reload at chrome://browser/content/tab-content.js:73

2) When issuing a request from the console (i.e., type "fetch('x')"), the request stack trace contains frames like:

WCA_evalWithDebugger resource://devtools/server/actors/webconsole.js:1299

Comment 1

[Jarda Snajdr [:jsnajdr]]

One way how to implement this: when calling "components.stack", it eventually calls GetFirstSubsumedFrame at [1], which filters the frames by the current JSContext's principal. When called from network-monitor.js (at [2]), it's the system principal. It would help if we could somehow pass the content principal there instead.

Nick, do you have any advice?

[1] https://dxr.mozilla.org/mozilla-central/source/js/src/vm/SavedStacks.cpp#556
[2] https://dxr.mozilla.org/mozilla-central/source/devtools/shared/webconsole/network-monitor.js#220

Comment 2

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

https://github.com/fitzgen/gecko-dev/blob/master/js/src/doc/SavedFrame/SavedFrame.md has the documentation you're looking for.

Basically, Cu.waiveXrays(stack) will give you the content's view of the stack.

Comment 3

[Emma Humphries ☕️🎸🧞‍♀️✨ (she/they) [:emceeaich] (Pacific Time) use needinfo]

This is an assigned P1 bug without activity in two weeks. 

If you intend to continue working on this bug for the current release/iteration/sprint, remove the 'stale-bug' keyword.

Otherwise we'll reset the priority of the bug back to '--' on Monday, August 28th.

Comment 4

[:Harald Kirschner :digitarald]

This seems to be fixed, probably by bug 1392411. Re-open if chrome stacks still appear.

Comment 5

[Sebastian Zartner [:sebo]]

Sorry, not fixed yet!

STR:

1. Open the Network panel on this page
2. Reload the page
3. Click the request for the document
4. Switch to the Stack Trace tab

You'll see something like this:

receiveMessage/< resource:///actors/BrowserTabChild.jsm:98:26
wrapHandlingUserInput resource://gre/modules/E10SUtils.jsm:837:7
receiveMessage resource:///actors/BrowserTabChild.jsm:95:21

I've tested this in Nightly 72.0a1 (2019-11-04).

In the case of a user initiated request or one only containing stack frames of chrome scripts, I would expect the Stack Trace tab not to be displayed at all.
Having said that, in the Browser Toolbox this info might still be useful.

Sebastian

Comment 6

[Jan Honza Odvarko [:Honza] (always need-info? me)]

I can reproduce the problem on my machine too (Win10, Nightly).

@Brian, could you please look at this.

Thanks,
Honza

Comment 7

[:Harald Kirschner :digitarald]

Honza, Initiator column will put this front and center. For the non-browser-toolbox case, could we test if a stack looks internal and discard it if it does?

Comment 8

[Jan Honza Odvarko [:Honza] (always need-info? me)]

I am no longer able to reproduce the problem using STR from comment #5 (there is no Stack Trace tab).
Sebastian, can you?

Honza

Comment 9

[:Harald Kirschner :digitarald]

Still occurs for me.

Load https://platform-status.mozilla.org/. For https://platform-status.mozilla.org/images/favicon.ico I get:

load
resource:///modules/FaviconLoader.jsm:165:20
load
resource:///modules/FaviconLoader.jsm:557:70
loadIcons
resource:///modules/FaviconLoader.jsm:633:26
onPageShow
resource:///modules/FaviconLoader.jsm:670:12
onHeadParsed
resource:///actors/LinkHandlerChild.jsm:60:24
handleEvent
resource:///actors/LinkHandlerChild.jsm:175:21

Comment 10

[Sebastian Zartner [:sebo]]

For reference, the Stack Trace tab is not shown for the main document request anymore since bug 1595637 got fixed (according to moz-regression).

But, as Harald already pointed out, the issue can still be reproduced on other requests.

In the case of a user initiated request or one only containing stack frames of chrome scripts, I would expect the Stack Trace tab not to be displayed at all.
Having said that, in the Browser Toolbox this info might still be useful.

Actually, I believe displaying chrome frames should depend on the option Show Gecko platform data, which obviously got introduced for the JS Profiler, though also fits for this case, in my opinion.

Sebastian

Comment 11

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Thanks!

Some pointers to the code base:

1. The Stack Trace panel is rendered here:
   https://searchfox.org/mozilla-central/rev/96f1457323cc598a36f5701f8e67aedaf97acfcf/devtools/client/netmonitor/src/components/StackTracePanel.js#67

2. It's using StackTrace component that is implemented here
   https://searchfox.org/mozilla-central/rev/96f1457323cc598a36f5701f8e67aedaf97acfcf/devtools/client/shared/components/StackTrace.js#37

3. The stack-trace is collected on the backend here
   https://searchfox.org/mozilla-central/rev/96f1457323cc598a36f5701f8e67aedaf97acfcf/devtools/server/actors/network-monitor/stack-trace-collector.js#114

4. We could peel off the chrome frame on the backend to safe a bit on RDP traffic but, also depends whether we want to support that Show Gecko platform data pref.

Here is an existing helper implemented for the Console panel. It isn't handling all of the chrome frames but, could be adapted:
https://searchfox.org/mozilla-central/rev/96f1457323cc598a36f5701f8e67aedaf97acfcf/devtools/server/actors/webconsole/utils.js#173-214

@Nicolas: can you provide some info about how the helper could be improved?

Honza

Comment 12

[Nicolas Chevobbe [:nchevobbe]]

I think we could have another helper, which would do a similar job, but that would filter out all the frames that starts with resource:// or chrome:// ? (maybe we should be more thoughtful on this for webextension? not sure

Comment 13

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Thanks Nicolas!
Honza

Comment 14

[manas]

Should the helper function be implemented in the StackTrace class or in StackTraceCollector itself?

Comment 15

[Jan Honza Odvarko [:Honza] (always need-info? me)]

We should peel off the chrome frames directly in the StackTraceCollector (and send to the client only what's needed) but, want tot keep these frames within the Browser Toolbox Network panel. Nicolas, do we have a way to check out whether the Network panel (and it's backend) is instantiated in the regular or Browser Toolbox?

Honza

Comment 16

[Maria Ortiz [ :marylicious ]]

Is someone working on this bug? I want to try it

Comment 17

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Assigned to you, thanks for the help!

Honza

Comment 18

[Nicolas Chevobbe [:nchevobbe]]

(In reply to Jan Honza Odvarko [:Honza] (always need-info? me) from comment #15)

We should peel off the chrome frames directly in the StackTraceCollector (and send to the client only what's needed) but, want tot keep these frames within the Browser Toolbox Network panel. Nicolas, do we have a way to check out whether the Network panel (and it's backend) is instantiated in the regular or Browser Toolbox?

We have this function on the toolbox devtools/client/framework/toolbox.js#606-608

Comment 19

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Hi Maria, are you still interested in fixing this bug?
Is there anything we can help with?

Honza

Comment 20

[Maria Ortiz [ :marylicious ]]

Attachment: Bug 1280266 - adding a filter for chrome frames on stacktrace panel

Comment 21

[Jan Honza Odvarko [:Honza] (always need-info? me)]

See also bug 1640371, should we fix both by one patch?

Honza

Comment 22

[Maria Ortiz [ :marylicious ]]

ok, I will check it out

Comment 24

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Another nice test case that shows the problem if HTTP request for fav-icon. It's done by the browser (not by the page) and the stack trace points to resource:// modules.

Honza

Comment 25

[BugBot [:suhaib / :marco/ :calixte]]

This good-first-bug hasn't had any activity for 6 months, it is automatically unassigned.
For more information, please visit auto_nag documentation.

Comment 26

[Riz]

Hi, I can reproduce this problem when looking at the stack trace of an HTTP request to https://www.google.com/favicon.ico .

My User-Agent when reproducing this bug is: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:138.0) Gecko/20100101 Firefox/138.0

Can you assign this bug to me please?

Comment 27

[Riz]

Hi, I've started looking in to this. I can't find an identifier called StackTraceCollector in the codebase (as per comment #15).

I can see how this helper could be repurposed to filter out the frames that start with resource://, chrome:// and webextension:// (should we filter out webextension:// too?) , and am trying to figure out how to ensure that it does so for the Network panel in the regular browser, but not in the Browser Toolbox.

Do I need to find the StackTraceCollector identifier and then peel the chrome frames directly in StackTraceCollector (as per comment #15)?

I think the codebase has changed significantly since the above comments were posted, so this insight might now be out of date.

Thanks

Comment 28

[Hubert Boma Manilla (:bomsy)]

Thanks Riz for the interest in working on this.
I'll assign to you.

The stacktraces are created and sent for network panel in https://searchfox.org/mozilla-central/rev/578d9c83f046d8c361ac6b98b297c27990d468fd/devtools/server/actors/resources/network-events-stacktraces.js#192-199. I think you might be able to filter out the chrome requests before this.onStackTraceAvailable is called.

For distinguishing the browser toolbox you could probably use this.watcherActor.sessionContext.type == "all" as in https://searchfox.org/mozilla-central/rev/578d9c83f046d8c361ac6b98b297c27990d468fd/devtools/server/actors/resources/network-events.js#255

Hope this helps!

Comment 29

[Riz]

Thank you Hubert

Comment 30

[Riz]

Hi Hubert, I added a console.log() statement to the first line of this method for my testing and local development, and it is barely ever invoked when I run my test case in the Nightly browser. My test case for this patch is to navigate to google.com and then look at the stacktrace of the favicon request in the Network tab of the browser inspector.

But despite the chrome frames always appearing in the stacktrace of the request, in the Network tab (and so the stacktrace always being generated), the console.log('stacktrace: ', stacktrace); call that I added, on the first line of _setStackTrace(), isn't always invoked when I navigate to google.com. In fact it is hardly ever invoked.

I could be missing something here about when/how it is invoked, and if so then this is likely because I am new to the codebase.

For me to write a helper function that successfully filters out the chrome frames from the stacktrace, I need to verify that it works by logging output. And that doesn't seem to be possible because this method doesn't seem to be called.

Comment 31

[Hubert Boma Manilla (:bomsy)]

(In reply to Riz from comment #30)

Hi Hubert, I added a console.log() statement to the first line of this method for my testing and local development, and it is barely ever invoked when I run my test case in the Nightly browser. My test case for this patch is to navigate to google.com and then look at the stacktrace of the favicon request in the Network tab of the browser inspector.

But despite the chrome frames always appearing in the stacktrace of the request, in the Network tab (and so the stacktrace always being generated), the console.log('stacktrace: ', stacktrace); call that I added, on the first line of _setStackTrace(), isn't always invoked when I navigate to google.com. In fact it is hardly ever invoked.

I could be missing something here about when/how it is invoked, and if so then this is likely because I am new to the codebase.

For me to write a helper function that successfully filters out the chrome frames from the stacktrace, I need to verify that it works by logging output. And that doesn't seem to be possible because this method doesn't seem to be called.

console.log won't work as this is devtools server code. Try using dump instead like this dump("the msg to log \n"). Also see https://searchfox.org/mozilla-central/search?q=dump%28&path=&case=false&regexp=false

Comment 32

[Riz]

Hi, that doesn't work either. I can't see any output as a result of invoking dump() on the first line of _setStackTrace.

I have recorded a demo and uploaded it to YouTube so you can take a look at the output in the console as I navigate to google.com in Nightly: https://www.youtube.com/watch?v=s7fd0_oaGgs

Comment 33

[Hubert Boma Manilla (:bomsy)]

(In reply to Riz from comment #32)

Hi, that doesn't work either. I can't see any output as a result of invoking dump() on the first line of _setStackTrace.

I have recorded a demo and uploaded it to YouTube so you can take a look at the output in the console as I navigate to google.com in Nightly: https://www.youtube.com/watch?v=s7fd0_oaGgs

Thanks for the video.

That's odd! Did you build i.e ./mach build after adding the dump statement?

Comment 34

[Riz]

Hi, I did build (i.e ./mach build) after adding the dump statement, but didn't record myself doing so because I didn't want the video to be longer than it needed to be.

I can record the video again in doing so record the ./mach build process in Powershell if you want.

Comment 35

[Riz]

I can record the video again** and, in doing so,** record the ./mach build process in Powershell if you want. *

Comment 36

[Riz]

Hi Hubert,

I forgot to tag my messages above with the 'needinfo' tag, so I am doing so now.

Comment 37

[Hubert Boma Manilla (:bomsy)]

(In reply to Riz from comment #34)

Hi, I did build (i.e ./mach build) after adding the dump statement, but didn't record myself doing so because I didn't want the video to be longer than it needed to be.

I can record the video again in doing so record the ./mach build process in Powershell if you want.

I tried this and it works for me. i'm on a Mac,.

We can also try using the Browser Toolbox, which should enable you debug the web toolbox

• You can enable the browser toolbox https://firefox-source-docs.mozilla.org/devtools-user/browser_toolbox/index.html#browser-toolbox
• Then switch the Browser Toolbox Mode to Multiprocess (Slower)
• In the browser toolbox switch to the debugger panel,
• Then do a Ctrl + P to open the quick open search panel
• Search for "network-events-stacktraces.js" file
• Open the file and you should be able to add breakpoints etc which should work.

I'll try to do a screencast. but let me know if you have questions.

Comment 38

[Riz]

Attachment: Bug 1280266 -Netmonitor JS stack traces no longer includes chrome frames. r=bomsy

Comment 39

[Riz]

I ran the following mochitests and XPCShell tests:

./mach xpcshell-test --tag devtools
./mach mochitest --headless devtools/client/netmonitor/test

I ran them with both the old devtools/server/actors/resources/network-events-stacktraces.js file in my local build and the new devtools/server/actors/resources/network-events-stacktraces.js file (i.e. the file containing my edits) file in my local build.

And the results were the same:

XPCShell test results

Ran 469 checks (15 subtests, 454 tests)
Expected results: 455
Skipped: 11 tests
Unexpected results: 3
  test: 2 (1 fail, 1 timeout)
  subtest: 1 (1 fail)

Unexpected Results
------------------
devtools/shared/tests/xpcshell/test_require.js
  FAIL devtools/shared/tests/xpcshell/test_require.js - xpcshell return code: -11
devtools/server/tests/xpcshell/test_breakpoint-20.js
  TIMEOUT devtools/server/tests/xpcshell/test_breakpoint-20.js - Test timed out
  FAIL  - Should pause because of hitting our breakpoint (not debugger statement). - "debuggerStatement" == "breakpoint"
/home/riz/firefox/obj-x86_64-pc-linux-gnu/_tests/xpcshell/devtools/server/tests/xpcshell/test_breakpoint-20.js:testBreakpoint:53
ERROR Unexpected exception NS_ERROR_ABORT:
_abort_failed_test@/home/riz/firefox/testing/xpcshell/head.js:869:20
do_report_result@/home/riz/firefox/testing/xpcshell/head.js:981:5
Assert<@/home/riz/firefox/testing/xpcshell/head.js:70:21
Assert.prototype.report@resource://testing-common/Assert.sys.mjs:251:10
equal@resource://testing-common/Assert.sys.mjs:293:8
testBreakpoint@/home/riz/firefox/obj-x86_64-pc-linux-gnu/_tests/xpcshell/devtools/server/tests/xpcshell/test_breakpoint-20.js:53:8
async*@/home/riz/firefox/obj-x86_64-pc-linux-gnu/_tests/xpcshell/devtools/server/tests/xpcshell/test_breakpoint-20.js:16:11
runThreadFrontTestWithServer@/home/riz/firefox/obj-x86_64-pc-linux-gnu/_tests/xpcshell/devtools/server/tests/xpcshell/head_dbg.js:963:13
async*threadFrontTest/<@/home/riz/firefox/obj-x86_64-pc-linux-gnu/_tests/xpcshell/devtools/server/tests/xpcshell/head_dbg.js:982:13
async*_run_next_test/<@/home/riz/firefox/testing/xpcshell/head.js:1759:22
_run_next_test@/home/riz/firefox/testing/xpcshell/head.js:1759:38
run@/home/riz/firefox/testing/xpcshell/head.js:808:9
_do_main@/home/riz/firefox/testing/xpcshell/head.js:245:6
_execute_test@/home/riz/firefox/testing/xpcshell/head.js:596:5
@-e:1:1

ERROR devtools/server/tests/xpcshell/test_breakpoint-20.js | Process still running after test!
 5:10.09 INFO Node moz-http2 server shutting down ...
 5:10.09 INFO http3Server server shutting down ...
Tests were run in parallel. Try running with --sequential to make sure the failures were not caused by this.

mochitests

Ran 14055 checks (13971 subtests, 84 tests)
Expected results: 14050
Skipped: 3 tests
Unexpected results: 2
  test: 1 (1 fail)
  subtest: 1 (1 fail)

Unexpected Results
------------------
devtools/client/netmonitor/test/browser_net_frame.js
  FAIL Test timed out -
  FAIL devtools/client/netmonitor/test/browser_net_frame.js - Application shut down (without crashing) in the middle of a test!

However during both runs my edits to browser.toml and my new test files browser_net_stacktraces-filtering.js and html_single_get_page_favicon.html were in the build. I can't imagine that would be a problem, because
the automated tests don't test test files anyway.

I didn't write an automated test to assert that the stack trace tab is visible in the Browser Toolbox because it seems that most of the automated tests are designed to execute on the browser process itself. However I did test this manually before pushing my changes to Phabricator.

Comment 40

[Riz]

Attachment: Bug 1280266 - Rename test file and clean up network events watcher. r=bomsy

Comment 41

[Riz]

Hi, I have left a comment under your comment in phabricator but am not sure whether it has sent you a notification. I clicked 'submit' but phabricator says my comment is 'not submitted' . I clicked submit again just now but am posting here in case I am doing something wrong. Cheers

Comment 42

[Hubert Boma Manilla (:bomsy)]

(In reply to Riz from comment #41)

Hi, I have left a comment under your comment in phabricator but am not sure whether it has sent you a notification. I clicked 'submit' but phabricator says my comment is 'not submitted' . I clicked submit again just now but am posting here in case I am doing something wrong. Cheers

Hey Riz,
I've just responded to your comment in Phabricator. Phab might have just been acting up at that point. I got a notification.

Cheers

Comment 43

[Riz]

Hi Hubert, I replied to you in phabricator RE the toolbox test. I don't know whether you've received a notification.

Comment 44

[Hubert Boma Manilla (:bomsy)]

Hi Riz, I've responded to the comment. Let me know if that helps.

Comment 45

[Riz]

Hi Hubert, I've responded in Phabricator. Did you receive a notification for my most recent comment in Phabricator? If so then I will stop letting you know in the comments section of this ticket.

Comment 46

[Hubert Boma Manilla (:bomsy)]

(In reply to Riz from comment #45)

Hi Hubert, I've responded in Phabricator. Did you receive a notification for my most recent comment in Phabricator? If so then I will stop letting you know in the comments section of this ticket.

I've responded, i was able to fix the browser toolbox test.

Thanks.

Comment 47

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/92ac932841a2
https://hg.mozilla.org/integration/autoland/rev/25ba57e653f2
Netmonitor JS stack traces no longer includes chrome frames. r=bomsy,devtools-reviewers

Comment 48

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/25ba57e653f2