[Affected Versions]
Firefox 72.0a1(20191108095936)
Firefox 71.0b7(20191104135555)

[Preconditions]
Set 'devtools.netmonitor.features.webSockets' to "true" in about:config and restart firefox
Set 'devtools.netmonitor.ws.displayed-frames.limit' to "2"

[Steps to reproduce]

1. Launch Firefox
2. Navigate to: http://janodvarko.cz/test/websockets/
3. Press F12 to open Devtools
4. Go to the Network tab
5. Press "Connect" button from the page opened in step 2
6. Click on the "http://echo.websocket.org" websocket request
7. Click on the "Messages" panel in the right
8. Press the "Send" button on the website four times
9. Check the 'Keep all future messages' checkbox
10. Change filter from 'All' to 'Sent' or 'Received'

[Actual results]
None of the messages appear.
The 'x messages have been truncated to conserve memory' is not updated after filtering.

[Expected results]
First two 'sent/received' messages should appear and the number of truncated messages should update to '2' in this case.

Thanks for the report, good catch!

I can reproduce the issue on my machine (Win10, Nightly)

Honza

Some instructions for anyone interested in fixing this bug:

1. The list of frames is rendered here:
   https://searchfox.org/mozilla-central/rev/6566d92dd46417a2f57e75c515135ebe84c9cef5/devtools/client/netmonitor/src/components/websockets/FrameListContent.js#231

2. The frames property is calculated using getDisplayedFrames method here:
   https://searchfox.org/mozilla-central/rev/6566d92dd46417a2f57e75c515135ebe84c9cef5/devtools/client/netmonitor/src/components/websockets/FrameListContent.js#345

3. getDisplayedFrames is implemented here
   https://searchfox.org/mozilla-central/rev/6566d92dd46417a2f57e75c515135ebe84c9cef5/devtools/client/netmonitor/src/selectors/web-sockets.js#13

Note that this method returns list that is already filtered (based on the UI mentioned in the STRs, comment #0)

4. Finally, there is a code that calculates the final number of rendered frames here:
   https://searchfox.org/mozilla-central/rev/6566d92dd46417a2f57e75c515135ebe84c9cef5/devtools/client/netmonitor/src/components/websockets/FrameListContent.js#245-262

This logic involves:

• Frames returned by getDisplayedFrames
• The frame limit given by devtools.netmonitor.ws.displayed-frames.limit pref
• State of the Keep all future messages checkbox

Something is wrong around how this calculation is done.

Honza

@Harald, the current devtools.netmonitor.ws.displayed-frames.limit is only limiting number of displayed frames. Perhaps we should have a pref for limiting the back-end too (i.e. limit for number of stored frames)?

Honza

limit for number of stored frames

Can you confirm that we are not limiting the number of stored frames at all, atm?

Some more comments:

1. The backend is creating LongStringActor for every sent/received frame payload here
   https://searchfox.org/mozilla-central/rev/131338e5017bc0283d86fb73844407b9a2155c98/devtools/server/actors/network-monitor/websocket-actor.js#106,131

These actors are not explicitly removed (e.g. when they would reach a limit)

2. The client side is handling WS sent/received notifications here:
   https://searchfox.org/mozilla-central/rev/131338e5017bc0283d86fb73844407b9a2155c98/devtools/client/netmonitor/src/connector/firefox-data-provider.js#468,478

This fires Redux action WS_ADD_FRAME

3. The action is handled by WS reducer here:
   https://searchfox.org/mozilla-central/rev/131338e5017bc0283d86fb73844407b9a2155c98/devtools/client/netmonitor/src/reducers/web-sockets.js#82

It adds WS frame into a frames map.

There is no logic that would explicitly remove frames from the map when reaching a limit.

I am raising the priority since this could be perf/memory issue.

Honza

@Hengyeow: can you please look at this, it's important to avoid huge memory consumption in case of high WS traffic.
See my instructions, it should be simple to fix.

Honza

@Honza I can try to look at this tonight or tomorrow

(In reply to Finn Williams from comment #7)

@Honza I can try to look at this tonight or tomorrow

Excellent, thanks!
Honza

Hello @Honza,

Can I try to look at this ?

Best regards,
Seif

Assigned to you, thanks for the help!
Honza

Hello @Honza,

I have fixed the reported bug but i have a question about the perf/memory consumption, what should be the limit ?

Thank you,
Seif

Depends on D60822

Some comments related to proper fix

1. The number of truncated messages should always reflect the amount of removed messages
2. The number of truncated messages should not change when the filter changes (we don't know what message types have been removed)
3. The number of truncated messages resets when the user clicks the Clear button

@Harald, does that make sense?

Honza

Yes, those all go back to a basic requirement that filtering must not affect truncation. Filtering should just hide messages but not truncate them.

If we want to remove that requirement, it's also an option. This would allow users to view more Sent messages but would make it impossible to go back to prior received messages that have been filtered/truncated.

Seifeddinebesbes, are you still interested in fixing this bug? I left some review comments in the patch/Phab.

Honza

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

Seifeddinebesbes, are you still interested in fixing this bug? I left some review comments in the patch/Phab.

Honza

Honza,

Thank you for your feedback,
I apologize for the delay, I have been a bit busy for the past few weeks.

I will deal with this bug these few days.

Seif

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

What kind of changes happened on the code base to give more attention on the future fix?

devtools when 'Keep all future messages' is enabled r=Honza

Hi folks,

I made a fix for this issue and added a unit test for verification. Please, review.

STR in the first message are quite outdated, so here is my update on that.

[Affected Versions]
105.0a1

[Preconditions]
Set 'devtools.netmonitor.msg.displayed-messages.limit' to "10"

[Steps to reproduce]

1. Open https://www.piesocket.com/websocket-tester and DevTools
2. Go to Network tab
3. Filter requests by WS type (will be easier to find the one)
4. Click on "Connect" button at the website
5. Select the WebSocket request and go to Response tab
6. Wait while the displayed messages limit is reached (should not take a long time)
7. Wait while there will be some number (~50, 100) of truncated messages. More truncated messages - more clear the bug.
8. Check 'Keep all future messages' checkbox
9. Clear all messages
10. Wait until the 'Keep all future messages' checkbox appear again (displayed messages limit)
11. Actual results

[Actual results]
None of the messages appear.
The 'x messages have been truncated to conserve memory' is not updated after filtering.
The messages will appear again when the counter at the bottom will exceed the truncated messages number.

[Expected results]
Messages should appear.
The 'x messages have been truncated to conserve memory' should be updated to '0 messages have been truncated to conserve memory' since 'Keep all future messages' is enabled (not sure here, please, verify).

Thank you for the patch!

Please take a look at my comments in Phab
https://phabricator.services.mozilla.com/D155763

Done. Please, review.

The following patch is waiting for review from an inactive reviewer:

:simakin.eugene, could you please find another reviewer?

For more information, please visit auto_nag documentation.

Done. Please, check.
Should I change the commit message?

Hi Eugene, the bot notification was a bug (investigated at Bug 1789646), we can keep Honza as your reviewer here.

Comments posted to Phab (I think I know why the test is failing intermittently).

https://hg.mozilla.org/mozilla-central/rev/a72369dae9cc

I managed to reproduce this issue using the STR from Comment 21 on Firefox 105.0(build ID: 20220915150737) on Ubuntu 22. Verified as fixed on Firefox 106.0(build ID: 20221010110315) and Nightly 107.0a1(build ID: 20221012094509) on Ubuntu 22, Windows 10, macOS 12.