Page MenuHomePhabricator
Differential D169361

Bug 1815460 - [devtools] Netmonitor waterfall sort should fallback to resource ids
AbandonedPublic
Actions

Authored by jdescottes on Feb 9 2023, 6:09 PM.

Details

Reviewers
ochameau
bomsy
Group Reviewers
devtools-reviewers
Commits
rMOZILLACENTRAL7765cf9b63f5: Bug 1815460 - [devtools] Netmonitor waterfall sort should fallback to resource…
Bugzilla Bug ID
1815460
Summary

resource ids are channel ids and should be incrementally generated. The current id is actually an actor id, meaning it is a string which will lead to incorrect comparisons (eg "Actor10" will be considered "lower" than "Actor2") and also fully depends on the order in which we create the actors.

On some platform / configurations we get identical timestamps for batches of requests, so the fallback mechanism should be reliable

Diff Detail

Repository
rMOZILLACENTRAL mozilla-central
Lint
Lint Not Applicable
Unit
Tests Not Applicable
Build Status
Buildable 521005
Build 617667: arc lint + arc unit

Event Timeline

jdescottes created this revision.Feb 9 2023, 6:09 PM
Herald added a project: secure-revision. · View Herald TranscriptFeb 9 2023, 6:09 PM
Harbormaster completed remote builds in B505815: Diff 679353.Feb 9 2023, 6:09 PM
phab-bot published this revision for review.Feb 9 2023, 6:09 PM
phab-bot changed the visibility from "Custom Policy" to "Public (No Login Required)".
phab-bot changed the edit policy from "Custom Policy" to "Restricted Project (Project)".
phab-bot removed a project: secure-revision.
jdescottes added a child revision: D168538: Bug 1815460 - [devtools] Merge addRequestPostData, addResponseHeaders, addResponseCookies into addResponseStart.Feb 9 2023, 6:12 PM
bomsy accepted this revision.EditedFeb 13 2023, 10:59 AM

Thanks @jdescottes! This makes sense. Good catch.

ochameau accepted this revision.Feb 15 2023, 12:51 PM
ochameau added a project: testing-exception-unchanged (Doesn't change behavior for users).

Have we ever had id attribute...?

But I'm wondering if we ever fallback to this code.
startedMs seems to be always defined:
https://searchfox.org/mozilla-central/source/devtools/shared/commands/resource/transformers/network-events.js#14
Or is it to explicitely sort request having the exact same startedDateTime?

This revision is now accepted and ready to land.Feb 15 2023, 12:51 PM
jdescottes added a comment.Feb 15 2023, 1:45 PM
In D169361#5589841, @ochameau wrote:

Have we ever had id attribute...?

I haven't tracked why, but id here is set to the actorID for the network event.

But I'm wondering if we ever fallback to this code.
startedMs seems to be always defined:
https://searchfox.org/mozilla-central/source/devtools/shared/commands/resource/transformers/network-events.js#14
Or is it to explicitely sort request having the exact same startedDateTime?

Yes that's exactly when this is useful.
And on most platforms this never happens, but on some windows platforms (no idea why) requests seem to be created in batches with the exact same creation time.

jdescottes updated this revision to Diff 687682.Mar 1 2023, 6:14 PM

rebase

Harbormaster completed remote builds in B512717: Diff 687682.Mar 1 2023, 6:14 PM
jdescottes updated this revision to Diff 687843.Mar 1 2023, 10:47 PM

rebase

Harbormaster completed remote builds in B512850: Diff 687843.Mar 1 2023, 10:47 PM
jdescottes updated this revision to Diff 689724.Mar 7 2023, 3:20 PM

rebase

Herald added a reviewer: devtools-reviewers. · View Herald TranscriptMar 7 2023, 3:20 PM
This revision now requires review to proceed.Mar 7 2023, 3:20 PM
Harbormaster completed remote builds in B514463: Diff 689724.Mar 7 2023, 3:20 PM
bomsy accepted this revision.Mar 7 2023, 3:44 PM
This revision is now accepted and ready to land.Mar 7 2023, 3:44 PM
jdescottes updated this revision to Diff 689910.Mar 7 2023, 9:00 PM

rebase

Harbormaster completed remote builds in B514591: Diff 689910.Mar 7 2023, 9:00 PM
jdescottes updated this revision to Diff 693468.Mar 15 2023, 9:04 PM

rebase

Harbormaster completed remote builds in B517631: Diff 693468.Mar 15 2023, 9:04 PM
jdescottes edited the summary of this revision. (Show Details)Mar 15 2023, 9:05 PM
jdescottes removed a parent revision: D168791: Bug 1815459 - [devtools] Remove NetworkUtils.createNetworkEvent.
jdescottes updated this revision to Diff 697092.Mar 24 2023, 2:42 PM

rebase

Harbormaster completed remote builds in B520657: Diff 697092.Mar 24 2023, 2:42 PM
Closed by commit rMOZILLACENTRAL7765cf9b63f5: Bug 1815460 - [devtools] Netmonitor waterfall sort should fallback to resource… (authored by jdescottes). · Explain WhyMar 24 2023, 4:20 PM
This revision was automatically updated to reflect the committed changes.
jdescottes added a commit: rMOZILLACENTRAL7765cf9b63f5: Bug 1815460 - [devtools] Netmonitor waterfall sort should fallback to resource….
• nataliaCs reopened this revision.Mar 24 2023, 8:24 PM
This revision is now accepted and ready to land.Mar 24 2023, 8:24 PM
• nataliaCs added a reverting change: rMOZILLACENTRAL97b5b8a2256a: Backed out 2 changesets (bug 1815460) for causing netmonitor devtools failures..Mar 24 2023, 8:24 PM
jdescottes abandoned this revision.Mar 25 2023, 12:26 PM

Looks like this now creates issues on windows 11. I think the tests for the netmonitor incorrectly assume some request ordering, and this patch is rather a step in the right direction but since it creates more intermittent issues, let's table it for now.

jdescottes removed a child revision: D168538: Bug 1815460 - [devtools] Merge addRequestPostData, addResponseHeaders, addResponseCookies into addResponseStart.Mar 25 2023, 12:26 PM

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
devtools/
client/
netmonitor/
src/
utils/
2 lines54%
Loading...

Diff 697144

View Options

devtools/client/netmonitor/src/utils/sort-predicates.js

Loading...
Privacy · Cookies · Legal