This is a breakdown task for bug 1308450 in order to integrate HTTP inspector into the Net panel. - Implement Timings Panel

Current WIP patch is focusing on providing react migration without integrating redux store since all details sub-panels depend on bug 1309866, but it hasn't landed yet. I will continue finishing WIP patch once bug 1309866 is landed.

Here is my patch which has already integrated with Jarda's requests view. I'll rebase Jarda's patch once it is landed in m-c and the current patch is for gathering early feedback to speed up review progress.

Comment on attachment 8814038 [details] Bug 1309187 - Implement Timings Panel https://reviewboard.mozilla.org/r/95356/#review95996 ::: devtools/client/netmonitor/components/http-inspector/timings-panel.js:25 (Diff revision 4) > + timings, > + totalTime, > +}) { > + // FIXME: A workaround to get the pane width since the width of html div > + // is unable to fit into the parent xul element automatically > + const tabboxWidth = $("#details-pane").getAttribute("width"); Is the component going to update when I resize the sidebar using the splitter? ::: devtools/client/netmonitor/components/http-inspector/timings-panel.js:29 (Diff revision 4) > + const offset = types > + .slice(0, idx) > + .reduce((acc, cur) => (acc + timings[cur]), 0); Functional style is sometimes hard to read, isn't it? Worth explaining in a comment what this computes. Also, will this code have any sensible result when the 'timings' object is empty? I'm afraid that the transform style will end up being "translateX(NaNpx)". ::: devtools/client/netmonitor/components/http-inspector/timings-panel.js:32 (Diff revision 4) > + const scale = totalTime > 0 ? Math.max(tabboxWidth / 2 / totalTime, 0) : 0; > + const timelines = types.map((type, idx) => { > + const offset = types > + .slice(0, idx) > + .reduce((acc, cur) => (acc + timings[cur]), 0); > + const transform = `translateX(${scale * offset}px)`; Using translateX won't work well in RTL environment. At best, it will need a special case for RTL. In the request list waterfall, I solved this by abandoning translateX, and replacing it by one of the following: - a "filler" span as a first element of every row, with "width" set to the offset value. - or setting padding-inline-start on the 'timings-summary-*' div. ::: devtools/client/netmonitor/components/http-inspector/timings-panel.js:57 (Diff revision 4) > + L10N.getFormatStr("networkMenu.totalMS", timings[type]) > + ) > + ); > + }); > + > + return div({}, timelines); If 'timelines' is an array of elements, each of them should have a 'key' attribute. Otherwise, React will issue warnings. And rerender will be slower.

Comment on attachment 8814038 [details] Bug 1309187 - Implement Timings Panel https://reviewboard.mozilla.org/r/95356/#review95996 > Is the component going to update when I resize the sidebar using the splitter? No, resizing sidebar doesn't update any redux state, so it won't make component re-render. (I've confirmed that problem doesn't happen)

(In reply to Jarda Snajdr [:jsnajdr] from comment #8) > Comment on attachment 8814038 [details] > Bug 1309187 - Implement Timings Panel > > https://reviewboard.mozilla.org/r/95356/#review95996 > > ::: devtools/client/netmonitor/components/http-inspector/timings-panel.js:25 > (Diff revision 4) > > + timings, > > + totalTime, > > +}) { > > + // FIXME: A workaround to get the pane width since the width of html div > > + // is unable to fit into the parent xul element automatically > > + const tabboxWidth = $("#details-pane").getAttribute("width"); > > Is the component going to update when I resize the sidebar using the > splitter? Oops I misunderstand you here. I found that it is an existing issue in current implementation. Resizing the splitter doesn't resize the timing elements as well, so I think I can fix that problem in this bug.

Summary the sidebar width resizing and the behavior of timeline resizing (requests view and timings panel) * I noticed that new timeline doesn't update itself immediately after toggling sidebar in Jarda's RequestsView patch * Timelines width change don't as responsive as cursor movement. * Toggle sidebar and resize browser window will change the timelines after a few microseconds (You can see it's still slow) but sometime they don't resize accordingly. * Close sidebar doesn't resize the timelines obviously. The current XUL implementation of timeline in timing panel doesn't support resizing. I also prefer to use a fixed width value SIDEBAR_WIDTH that would be more efficient and it looks great even you shrink sidebar into the minimum size which doesn't look weird. How do you think?

Jarda, I found the resize implementation in your patch and modified my patch to register "mouseup" event for splitter. Resizing performance now is much better than "mousemove" event, so I think you could try it in your patch to improve the resizing laggy as well.

I put "mousemove" back since the performance is not so bad as I thought.

Comment on attachment 8814038 [details] Bug 1309187 - Implement Timings Panel https://reviewboard.mozilla.org/r/95356/#review96452 ::: devtools/client/netmonitor/details-view.js:146 (Diff revision 11) > this._responseCookies = L10N.getStr("responseCookies"); > > $("tabpanels", this.widget).addEventListener("select", this._onTabSelect); > + > + this._splitter = $("#network-inspector-view-splitter"); > + this._splitter.addEventListener("mousemove", this._onResize, false); I am seeing significant perf improvement when "mouseup" is used (tested on Win10). Dragging the splitter is visibly smoother. The same when testing the Timings patch (bug 1309187)

Comment on attachment 8814038 [details] Bug 1309187 - Implement Timings Panel https://reviewboard.mozilla.org/r/95356/#review96472 Like I suggested for the Security panel, we should try to implement the panel layout in pure CSS, without having to know the exact width. First, set a -moz-box style on the top-level HTML:DIV. Second, the offsets and widths of the timing bars could be specified as percentages of the totalTime. This is difficult for the waterfall column in the request list, because it requires re-rendering all waterfall rows on every update (the percentages change on every update). But here, it should be easy. ::: devtools/client/netmonitor/components/http-inspector/timings-panel.js:23 (Diff revision 12) > +function TimingsPanel({ > + sidebarWidth, > + timings = {}, > + totalTime = 0, > +}) { > + const types = ["blocked", "dns", "connect", "send", "wait", "receive"]; nit: Extract this contant array out of the TimingsPanel function, so that it's not allocated again on every call. The render() functions should be fast. ::: devtools/client/netmonitor/components/http-inspector/timings-panel.js:24 (Diff revision 12) > + sidebarWidth, > + timings = {}, > + totalTime = 0, > +}) { > + const types = ["blocked", "dns", "connect", "send", "wait", "receive"]; > + const scale = totalTime > 0 ? Math.max(sidebarWidth / 2 / totalTime, 0) : 0; The pure CSS layout will let us get rid of this magic constant with value "2". ::: devtools/client/netmonitor/components/http-inspector/timings-panel.js:33 (Diff revision 12) > + const offset = types > + .slice(0, idx) > + .reduce((acc, cur) => (acc + timings[cur] || 0), 0); > + > + return div({ > + key: `timings-summary-${type}`, nit: The 'key' can be just `${type}`. It need to be unique only locally, not globally. ::: devtools/client/netmonitor/components/http-inspector/timings-panel.js:37 (Diff revision 12) > + return div({ > + key: `timings-summary-${type}`, > + id: `timings-summary-${type}`, > + className: "tabpanel-summary-container", > + }, > + span({ className: "plain tabpanel-summary-label" }, The "plain" CSS class is not needed here. It's useful for styling XUL label elements, but not here. ::: devtools/client/netmonitor/components/http-inspector/timings-panel.js:50 (Diff revision 12) > + }), > + span({ > + className: `requests-menu-timings-box ${type}`, > + style: { width: timings[type] * scale }, > + }), > + span({ className: "plain requests-menu-timings-total" }, No "plain".

Comment on attachment 8814038 [details] Bug 1309187 - Implement Timings Panel https://reviewboard.mozilla.org/r/95356/#review96472 Could you explain more about "offsets and widths of the timing bars could be specified as percentages of the totalTime"? thanks!

First, compute the offsets as percentages of the totalTime: let offset = (sumOfPreviousBoxWidths / totalTime) * 100; let boxWidth = (widthOfBoxInMs / totalTime) * 100; Second, use these percentages when styling the timing boxes: div({ className: "timing-row" }, [ span({ className: "offset", style: { width: `${offset}%` } }), span({ className: "box", style: { width: `${boxWidth}%` } }), ]) This way, your component doesn't need to care what exactly is the width of the sidebar. CSS will do 100% of the job when the sidebar is resized.

What a perfect CSS solution! I really like that. These fixed percentage values work well and code is cleaner and better now. I still need to use value * 50 instead of * 100 since there are summary-label and timings-total have to render. So I think it makes sense to / 2 in previous implementation.

Updated my patch and addressed all issues as well as linter errors. Right now the TimingsPanel is more robust and performant. * Support resizing timeline width while moving splitter and resizing browser window. (Old implementation doesn't support that) * "mouseup" event listener has been removed and use pure CSS solution instead. * We can reach the same responsiveness when resizing timeline width as using "mousemove" event. * So it's apparently a performance improvement.

(In reply to Ricky Chien [:rickychien] from comment #27) > I still need to use value * 50 instead of * 100 since there are > summary-label and timings-total have to render. So I think it makes sense to > / 2 in previous implementation. You could add some fixed padding to the left (for summary-label) and to the right (for timings-total) and let the timing boxes fill the entire available area in between. That's more correct than the "width / 2" approach.

Improve CSS as comment 32 mentioned and fixed the missing translateX animation.

Comment on attachment 8814038 [details] Bug 1309187 - Implement Timings Panel https://reviewboard.mozilla.org/r/95356/#review96810 Wow, this new version looks and works great! One nit about the animation: when you change the selection from one request to another, then the change of offsets is animated, and the change of the box width is not animated. Under the right conditions, the total width of both boxes can be more than 100% for a brief moment. When I add the same animation also to the requests-menu-timings-box width, the animation becomes smoother and looks better. We should also check if the React tabs don't cause any further performance regression - each of them has its own store subscription, and does a linear search on the request list on every update.

Nice tip! Animation now looks smoother after applying the same transition to the requests-menu-timings-box width. Patch has been updated again.

Since bug 1309866 has been landed, I've rebased to latest thunk.

Comment on attachment 8814038 [details] Bug 1309187 - Implement Timings Panel https://reviewboard.mozilla.org/r/95356/#review97152 R+ assuming try is green. But, please see my inline comment. Honza ::: devtools/client/netmonitor/components/moz.build:6 (Diff revision 19) > # This Source Code Form is subject to the terms of the Mozilla Public > # License, v. 2.0. If a copy of the MPL was not distributed with this > # file, You can obtain one at http://mozilla.org/MPL/2.0/. > > +DIRS += [ > + 'http-inspector', I think we should rename this to 'shared' Honza

Pushed by rchien@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/76bbb68f8441 Implement Timings Panel r=Honza,jsnajdr

I had to back this out because bug 1309865 got backed out on inbound, and this depended on it, so when the backout merges around it would cause merge failures. https://hg.mozilla.org/mozilla-central/rev/bfa85d23df57c8a1db17c99b267667becc1c4afd

Pushed by rchien@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/7ffd1b73e3f5 Implement Timings Panel r=Honza,jsnajdr

sorry had to back this out because after merging to m-c we had mac os static debug build bustage like described in bug 1322685 I'm not sure if this patches here need to change or a releng fix is needed but to have a working tree i had to back this out to fix the static bustage

Backout by cbook@mozilla.com: https://hg.mozilla.org/mozilla-central/rev/a0b88dac1929 Backed out changeset 7ffd1b73e3f5

It will be fixed in bug 1309866.

Pushed by rchien@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2682c37617a4 Implement Timings Panel r=Honza,jsnajdr

This issue is verified fixed on latest Nightly 53.0a1 (2016-12-18) on the following OSes: - Windows 10 x64 - Ubuntu 16.04 x64 LTS - Mac OS 10.11.6 I can confirm that, Timings Panel is working as expected on the platforms mentioned above.