Closed Bug 1122662 Opened 5 years ago Closed 4 years ago

Overview graphs don't refresh when the browser window is resized

Categories

(DevTools :: Performance Tools (Profiler/Timeline), defect, P2)

defect

Tracking

(firefox40 verified, firefox41 fixed)

VERIFIED FIXED
Firefox 41
Tracking Status
firefox40 --- verified
firefox41 --- fixed

People

(Reporter: vporof, Assigned: bgrins)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

STR:
1. Start recording
2. Finish recording
3. Make the browser window narrower

Overview graphs should redraw.
Blocks: perf-tool-v2
Jordan practically begged me to work on this
Assignee: nobody → jsantell
Status: NEW → ASSIGNED
introduced via markers/memory timelines in bug 1077457, bafb52753752d4c49ead17cf045d239675811af4
No longer blocks: perf-tool-v2
Blocks: perf-tool-v2
No longer blocks: enable-perf-tool
Duplicate of this bug: 1129641
Blocks: perf-tool-papercuts
No longer blocks: perf-tool-v2
Assignee: jsantell → nobody
Assignee: nobody → vporof
Priority: -- → P2
See Also: → 1164327
Attached patch graph-resizer.patch (obsolete) — Splinter Review
Took a lot of investigation, but here is what appears to be happening:

* When the window gets resized to be larger, the iframes get a new width (based on the boundingClientRect of their parent)
* When the window gets resized to be smaller, the iframes old width (the old window size) force the boundingClientRect of it's parent to stay at the previous size.

I tried a lot of things to track this down, but the simplest one was to convert the iframe parents to be display: flex and then set the iframe's width to 100% in CSS. *shrug*

A separate problem is that some of the long labels in the <deck> are taking up a lot of space (> 500px) and this causes any measurement of the parent element to be wrong since the performance-view-content container gets enlarged from this.  This was amazingly hard to track down, and I ended up setting by switching to textContent for the label and then using the vw unit in CSS for max-width.

Victor and/or Jordan, please apply the patch and double check that this fixes the issue you are seeing.
Assignee: vporof → bgrinstead
Attachment #8608417 - Flags: review?(vporof)
Attachment #8608417 - Flags: feedback?(jsantell)
Comment on attachment 8608417 [details] [diff] [review]
graph-resizer.patch

Review of attachment 8608417 [details] [diff] [review]:
-----------------------------------------------------------------

Oh my, this is great! Overview graphs, flame graph all resize correctly when resizing the window. They all kinda rerender and pop-out at different sizes a bit (probably a canvas thing), but that's fine (and/or can be handled in the future), but other than that, looks great! The waterfall doesn't rerender and resize, but again that's another bug somewhere.

Thanks Brian!
Attachment #8608417 - Flags: feedback?(jsantell) → feedback+
Comment on attachment 8608417 [details] [diff] [review]
graph-resizer.patch

Review of attachment 8608417 [details] [diff] [review]:
-----------------------------------------------------------------

Teach me your ways.

::: browser/themes/shared/devtools/performance.inc.css
@@ +157,5 @@
> +/* These rules prevent graph frames from not being able to shrink once expanded
> +   by not letting a width attribute set on them expand the boundingClientRect
> +   of the parent elements.  See Bug 1122662. */
> +#overview-pane > hbox,
> +#js-flamegraph-view {

Targeting the js-flamegraph-view is oddly specific, and I can imagine this can happen with all the other decks as well, in the future. Make this selector a bit more generic.

@@ +162,5 @@
> +  display: flex;
> +}
> +
> +#overview-pane iframe,
> +#js-flamegraph-view iframe {

Ditto.
Attachment #8608417 - Flags: review?(vporof) → review+
Duplicate of this bug: 1167337
Moved the graph styling out of specific CSS selectors and into the AbstractCanvasGraph.createIframe call.

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=52f83fafb43b
Attachment #8608417 - Attachment is obsolete: true
Attachment #8609091 - Flags: review+
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/1e64ee0dc934
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 41
Flags: qe-verify+
Comment on attachment 8609091 [details] [diff] [review]
graph-resizer.patch


Approval Request Comment
[Feature/regressing bug #]: 1167252, the new performance tool
[User impact if declined]: Won't ship the performance tool
[Describe test coverage new/current, TreeHerder]: There are try pushes in Bug 1167252 with all patches needing uplift
[Risks and why]: Requesting uplift for the accumulated changes in the performance tool since the 40 merge date, so these changes haven't had the full 6 weeks to bake.  Risks are generally contained within devtools, specifically within the performance panel.
[String/UUID change made/needed]: None
Attachment #8609091 - Flags: approval-mozilla-aurora?
Note: I had verbal confirmation for these uplifts from Sylvestre even before he's flagged them as a+.  See https://bugzilla.mozilla.org/show_bug.cgi?id=1167252#c26
Comment on attachment 8609091 [details] [diff] [review]
graph-resizer.patch

Change approved to skip one train as part of the spring campaign.
Attachment #8609091 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified fixed on Aurora 40.0a2 (2015-06-04), using Windows 7 (x64), Ubuntu 14.04 (x64) and Mac OS X 10.8.5.

The Waterfall, Call Tree and Flamechart are properly resized according to browser window size.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.