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

VERIFIED FIXED in Firefox 40

Status

DevTools
Performance Tools (Profiler/Timeline)
P2
normal
VERIFIED FIXED
4 years ago
5 days ago

People

(Reporter: vporof, Assigned: bgrins)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 41
Dependency tree / graph

Firefox Tracking Flags

(firefox40 verified, firefox41 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

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

Overview graphs should redraw.
(Reporter)

Updated

4 years ago
Blocks: 1075567
(Reporter)

Comment 1

4 years ago
Jordan practically begged me to work on this
Assignee: nobody → jsantell
Status: NEW → ASSIGNED
introduced via markers/memory timelines in bug 1077457, bafb52753752d4c49ead17cf045d239675811af4

Updated

3 years ago
Blocks: 1123815
(Reporter)

Updated

3 years ago
No longer blocks: 1075567
(Reporter)

Updated

3 years ago
Blocks: 1075567
No longer blocks: 1123815
(Reporter)

Updated

3 years ago
Duplicate of this bug: 1129641
(Reporter)

Updated

3 years ago
Blocks: 1145697
No longer blocks: 1075567
Assignee: jsantell → nobody
(Reporter)

Updated

3 years ago
Assignee: nobody → vporof
Priority: -- → P2
(Assignee)

Updated

3 years ago
See Also: → bug 1164327
(Assignee)

Comment 4

3 years ago
Created attachment 8608417 [details] [diff] [review]
graph-resizer.patch

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+
(Reporter)

Comment 6

3 years ago
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+
(Assignee)

Updated

3 years ago
Blocks: 1167252
(Reporter)

Updated

3 years ago
Duplicate of this bug: 1167337
(Assignee)

Comment 8

3 years ago
Created attachment 8609091 [details] [diff] [review]
graph-resizer.patch

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+
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/1e64ee0dc934
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox41: --- → fixed
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 41
Flags: qe-verify+
(Assignee)

Comment 11

3 years ago
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?
(Assignee)

Comment 13

3 years ago
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
status-firefox40: fixed → verified
Flags: qe-verify+

Updated

5 days ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.