1090950 - Negative selection hides the overview graph's mask

Status: VERIFIED FIXED

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

Description

[Andrei Vaida [:avaida]]

Reproducible on:
 * Aurora 35.0a2 (2014-10-28 / 20141028155607),
 * Nightly 36.0a1 (2014-10-28),
using Windows 7 64-bit, Mac OS X 10.9.5 and Ubuntu 14.04 LTS 32-bit.

STR:
1. Launch Firefox with a clean profile.
2. Open menu → Developer → Toggle tools.
3. Click Toolbox Options and enable "Timeline" from the "Default Firefox Developer Tools" section.
4. Click Timeline and start recording.
5. Access a content-heavy website (e.g. http://theverge.com/) and scroll through the website for a few seconds.
6. Stop recording.
7. Select a section from the beginning of the overview graph.
8. Drag the selection beyond the 0 ms boundary of the graph.

Expected result:
Selectable intervals from the recording are only available from 0 ms to <end of recording>.

Actual result:
Negative intervals are selectable in the overview graph and displayed in the Records waterfall.

Additional notes:
- This issue is *not* a regression.
- This bug is reproducible across platforms (Windows, Linux, Mac).
- Screenshot: http://i.imgur.com/TkoGtKf.png.

Comment 1

[Victor Porof [:vporof][:vp]]

Moving into the Profiler component. Filter on GUTHRIE'S WAVY CAKES.

Comment 2

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: overview.gif

I'm not able to recreate a negative interval selection -- I'm guessing all the changes done in migrating the timeline/waterfall views over to Performance fixed this.

However, I do see when sliding the selection out of bounds to the left (maybe to the right too? not sure), the selection disappears (but still seems to bound the waterfall view from 0 to the right hand side of the selection).

Comment 3

[Brian Grinstead [:bgrins]]

Attachment: drag-selection-beforestart.patch

This seems to fix it.  I think it was just due to calls of fillRect not liking out of bounds start/end values.  Also added a simple test case.  Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ea0d3ad4b4ae

Comment 4

[Victor Porof [:vporof][:vp]]

Comment on attachment 8607745 [details] [diff] [review]
drag-selection-beforestart.patch

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

wat

Comment 5

[Brian Grinstead [:bgrins]]

(In reply to Brian Grinstead [:bgrins] from comment #3)
> Created attachment 8607745 [details] [diff] [review]
> drag-selection-beforestart.patch
> 
> This seems to fix it.  I think it was just due to calls of fillRect not
> liking out of bounds start/end values.  Also added a simple test case.  Try
> push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ea0d3ad4b4ae

xpcshell failures on that push look unrelated, probably just got a bad pull

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/ad0ab9f214d0

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/ad0ab9f214d0

Comment 8

[Brian Grinstead [:bgrins]]

Comment on attachment 8607745 [details] [diff] [review]
drag-selection-beforestart.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

Comment 9

[Brian Grinstead [:bgrins]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/3a82f6bef389

Comment 10

[Brian Grinstead [:bgrins]]

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 11

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8607745 [details] [diff] [review]
drag-selection-beforestart.patch

Change approved to skip one train as part of the spring campaign.

Comment 12

[Andrei Vaida [:avaida]]

Verified fixed on Aurora 40.0a2 (2015-06-04), using Windows 7 (x64), Ubuntu 14.04 (x64) and Mac OS X 10.8.5.

There are no negative intervals displayed if the selection is placed beyond the 0 mark.