Pull out everything into something else. This was an attempt to break apart the netmonitor into several different components, with options to swap out table rendering for react, and to more easily reason about the different view components. Here are my notes when I was working on it: * Have a RequestCollections containing request models and emitting events on actions (add, update, reset, etc). * All filtering and sorting logic are also in this collection model, and just emits ‘sort’ and ‘filtered’ events. * Models now do most of the heavy lifting — like `model.toCurlString()`, and renderers (fancy templates, mostly) do all the DOM stuff, with our views becoming just event bindings to the DOM mapped to actions. * Pulled out many individual views to solely handling actions within their jurisdiction. Almost all views just respond to RequestCollection events, and minimizing the amount they have to communicate with each other. This isn’t perfect yet, but should be moved to event-based actions between views. * TableRenderer is a component used by the RequestsMenu view to just handle the rendering of the tables. This’ll make it easier to swap in a different kind of rendering system (TableWidget? React?), as it just has to hook into the RequestCollection events, and provide some exposed API (like react to refreshing the waterfall view, etc), and these are all documented. TableRenderer is using Widgets under the hood to interact with the collection, where it assumes the model is the `attachment` property on an item. If we swap this out, each item should just be the model, so there are some wrappers implemented for this, that can easily be removed if we have a new renderer. * There’s also a SummaryRenderer to handle rendering of the summary, that’s pretty small. * Many things pulled out of RequestsView into a general NetMonitorView as they were global-ish, or the controller, if appropriate, or pulled into different views. * Custom Requests: Rather than adding this into the view immediately, and removing it if the user cancels, this just toggles a new custom request window, and if and only if the user sends the custom request, does everything else fall into place. The monitor handles it and the custom request closes, and selects the new request when available.

Giant patch. Still needs to re-enable tooltips and the tests mostly need renaming the global view names

Try build? How's the perf?

I can push up a try build, but there shouldn't be any visible changes. Perf seems the same. This will just make it easier to fix many of the common bugs and reducing side effects, and make it easy to swap out the rendering for a fast one -- like switching the table rendering to use react, as it's now all in one place

(In reply to Jordan Santell [:jsantell] [@jsantell] from comment #3) > I can push up a try build, but there shouldn't be any visible changes. Perf > seems the same. This will just make it easier to fix many of the common bugs > and reducing side effects, and make it easy to swap out the rendering for a > fast one -- like switching the table rendering to use react, as it's now all > in one place Okay, got it - I hadn't read the patch and assumed there was more done here. *Very* interested in try builds of react or other experiments that impact front-end perf.

Putting this as P1, which seems strange at first glance. If we jusst land this patch, no end users will actually notice, however I suspect if we do land this patch as soon as possible it will make hacking on the net monitor and in particular reviewing patches much much easier, so my reasoning is - "It's important because there will be a flow-through effect".

This is my new everything. Looking to get this in Victor's for review before the 13th this week.

Still very much a WIP, but everything works for the most part. Tooltips, and IP address in details view not working yet -- and some of the cloning requests (the actor ID from the webconsoleclient sendHTTPRequest is different than the real request that comes in -- we just assume they're the same, so maybe an improvement?). Lots of tests are still acting up, but slowly whittling those down. Obvs would like to land this ASAP, for other patches landing (otherwise I have to manually translate lines one-by-one), and the sooner we can shift out any bugs due to the refactoring. Might not be a terrible idea to display some sort of "warning" when using a tool in nightly during certain stages, like a "keep a look out for weirdness, and report bugs".

I can still look at this Jordan, just postponed it for a while since there's other things that have more priority right now. If it's urgent, ping and I'll finish this asap. Sorry for the delay!

I have an updated version of this too from a bit back, but it's still gonna take me a bit to have time to finish it, so got rid of the r? for now until me (or someone else) has time to see it through

realistically, i won't get around to this for awhile.

Should we get someone to finish this off? Net monitor being open can really slow page load (bug 1176050), reducing the value of the tool (since it being active significantly alters the data it wants to present). Profiling shows it's largely blocking on drawing all the updates, so a better arch that makes it easy to improve this sounds worth landing.

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #11) > Should we get someone to finish this off? > > Net monitor being open can really slow page load (bug 1176050), reducing the > value of the tool (since it being active significantly alters the data it > wants to present). Profiling shows it's largely blocking on drawing all the > updates, so a better arch that makes it easy to improve this sounds worth > landing. This is definitely a worthy bug to work on. As for the perf impact of netmonitor, I think over the last few cycles Brian has greatly improved performance. Is it still blocked on drawing updates?

(In reply to Jeff Griffiths (:canuckistani) from comment #12) > As for the perf impact of netmonitor, I think over the last few cycles Brian > has greatly improved performance. Is it still blocked on drawing updates? I did a bit of further profiling for Bug 1176050 and it looks like there is still a bit of frontend jank, but AFAICT it may be more backend: https://bugzilla.mozilla.org/show_bug.cgi?id=1176050#c12. A good test for that, which I just thought of and haven't done, is to run the same test with the netmonitor not having been opened yet and see if it gets similar results as when it is opened. Regardless, refactoring this makes sense for technical debt, contributor friction, etc.

I'm in the swing of refactoring the debugger, and very soon I want to get feedback from others and start helping cleanup other tools. It feels really good to get rid of technical debt, so I'm enjoying it. The network tool is probably a good next tool for me to help with, so maybe I can take this in Q4.

(In reply to Brian Grinstead [:bgrins] from comment #13) > Regardless, refactoring this makes sense for technical debt, contributor > friction, etc. Agreed.

Probably; I won't be able to do this until December earliest, and even that is unlikely, but depends on priority of other things -- Jeff's call

(In reply to Jordan Santell [:jsantell] [@jsantell] from comment #16) > Probably; I won't be able to do this until December earliest, and even that > is unlikely, but depends on priority of other things -- Jeff's call My call is still that someone should do it and you're busy. We're short-staffed atm but maybe hiring will eventually un-jam things like this. I imagine this would be a great project for a newer employee to get used to the codebase?

(In reply to (Unavailable until Apr 6) Brian Grinstead [:bgrins] from comment #13) > (In reply to Jeff Griffiths (:canuckistani) from comment #12) > > As for the perf impact of netmonitor, I think over the last few cycles Brian > > has greatly improved performance. Is it still blocked on drawing updates? > > I did a bit of further profiling for Bug 1176050 and it looks like there is > still a bit of frontend jank, but AFAICT it may be more backend: > https://bugzilla.mozilla.org/show_bug.cgi?id=1176050#c12. A good test for > that, which I just thought of and haven't done, is to run the same test with > the netmonitor not having been opened yet and see if it gets similar results > as when it is opened. I did that see my comment: https://bugzilla.mozilla.org/show_bug.cgi?id=1176050#c17 My results point to Net panel UI being the culprit and slowing down the page load time. As soon as I am done with Firebug migration, which is taking my full time I can start work on this one and move the Net panel towards React adoption. I am keeping the priority set to #1 tho. Honza