Closed Bug 1458793 Opened Last year Closed Last year

Intermittent browser/base/content/test/performance/browser_tabclose.js | unexpected changed rect: ({x1:0, x2:223, y1:0, y2:32, w:224, h:33}), window width: 1280 -

Categories

(Firefox :: General, defect, P5)

Product:
Firefox
Component:
General
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 62
Tracking Flags:
Tracking Status
firefox61 --- fixed
firefox62 --- fixed

People

(Reporter: intermittent-bug-filer, Assigned: dao)

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file)

Bug 1458793 - Update flicker conditions to account for bug 1183135.
59 bytes, text/x-review-board-request
mconley
: review+
Details
See Also: → 1448497
Assignee: nobody → dao+bmo
Blocks: 1183135
I had to use <= instead of == for the width because this is Linux-only and intermittent -- I still don't know why.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3deab83b7c02814532c5b419ebded4db0a3706c4
Comment on attachment 8973296 [details]
Bug 1458793 - Update flicker conditions to account for bug 1183135.

https://reviewboard.mozilla.org/r/241770/#review247742

Thanks for digging into this, dao!

::: browser/base/content/test/performance/browser_tabclose.js:68
(Diff revision 1)
>             condition: r =>
>               // In tab strip
>               r.y1 >= tabStripRect.top && r.y2 <= tabStripRect.bottom &&
>               r.x1 >= tabStripRect.left && r.x2 <= tabStripRect.right &&
> -             // Width of one tab.
> -             r.w == gBrowser.selectedTab.clientWidth
> +             // Width of one tab plus tab separator.
> +             r.w <= gBrowser.selectedTab.clientWidth + 1

(In reply to Dão Gottwald [::dao] from comment #3)
> I had to use <= instead of == for the width because this is Linux-only and
> intermittent -- I still don't know why.
> 
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=3deab83b7c02814532c5b419ebded4db0a3706c4

I think because this is sometimes wavering between (at least) the following types of captured frames:

224px width change:
Before: https://imgur.com/XoaTwOt
After: https://imgur.com/IHbYmAY

224px because what's happening is that the left-most tab is being selected, which creates a change rect that is 1px smaller than the tab because of the already-visible tab separator

225px width change: 
Before: https://imgur.com/uoG1fWN
After: https://imgur.com/YTQR49t

225px because what's happening is that the right-most tab is being de-selected before the first tab is selected, so the right-most tab and the right-most separator disappears.


Instead of checking for clientWidth + 1 or less, I think it'd make more sense to choose a smaller range of tolerance. Can we ensure that instead that r.w is within 1 or 2 pixels of gBrowser.selectedTab.clientWidth? You could perhaps use something like [this](https://searchfox.org/mozilla-central/rev/53afcfdbabed96883126d0ebbcac499b358e32f2/browser/base/content/test/performance/browser_tabopen.js#36), via:

```js
inRange(gBrowser.selectedTab.clientWidth - r.w, 0, 2)
```
Attachment #8973296 - Flags: review?(mconley) → review+
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8a4ebd5b130d
Update flicker conditions to account for bug 1183135. r=mconley
Thanks, dao!
Backout by toros@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/15e1801775a4
Backed out changeset 8a4ebd5b130d for frequently failing browser/base/content/test/performance/browser_tabclose.js on Linux on a CLOSED TREE
Log snippet with the failure: 

[task 2018-05-07T18:07:06.125Z] 18:07:06     INFO - TEST-START | browser/base/content/test/performance/browser_tabclose.js
[task 2018-05-07T18:07:07.076Z] 18:07:07     INFO - TEST-INFO | started process screentopng
[task 2018-05-07T18:07:07.629Z] 18:07:07     INFO - TEST-INFO | screentopng: exit 0
[task 2018-05-07T18:07:07.630Z] 18:07:07     INFO - Buffered messages logged at 18:07:06
[task 2018-05-07T18:07:07.630Z] 18:07:07     INFO - Entering test bound 
[task 2018-05-07T18:07:07.631Z] 18:07:07     INFO - TEST-PASS | browser/base/content/test/performance/browser_tabclose.js | 0 unexpected reflows - true == true - 
[task 2018-05-07T18:07:07.631Z] 18:07:07     INFO - comparing 4 frames
[task 2018-05-07T18:07:07.632Z] 18:07:07     INFO - Buffered messages logged at 18:07:07
[task 2018-05-07T18:07:07.635Z] 18:07:07     INFO - TEST-FAIL | browser/base/content/test/performance/browser_tabclose.js | bug 1444886 - the next tab should be selected at the same time as the closed one disappears, ({x1:225, x2:449, y1:0, y2:32, w:225, h:33}), window width: 1280 - 
[task 2018-05-07T18:07:07.636Z] 18:07:07     INFO - Buffered messages finished
[task 2018-05-07T18:07:07.638Z] 18:07:07     INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/performance/browser_tabclose.js | unexpected changed rect: ({x1:0, x2:223, y1:0, y2:32, w:224, h:33}), window width: 1280 - 
[task 2018-05-07T18:07:07.639Z] 18:07:07     INFO - Stack trace:
[task 2018-05-07T18:07:07.640Z] 18:07:07     INFO - chrome://mochitests/content/browser/browser/base/content/test/performance/head.js:reportUnexpectedFlicker/rects<:574
[task 2018-05-07T18:07:07.644Z] 18:07:07     INFO - chrome://mochitests/content/browser/browser/base/content/test/performance/head.js:reportUnexpectedFlicker:565
[task 2018-05-07T18:07:07.647Z] 18:07:07     INFO - chrome://mochitests/content/browser/browser/base/content/test/performance/head.js:withPerfObserver:633
Flags: needinfo?(dao+bmo)
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/05ed1e14418c
Update flicker conditions to account for bug 1183135. r=mconley
https://hg.mozilla.org/mozilla-central/rev/05ed1e14418c
Status: NEW → RESOLVED
Closed: Last year
status-firefox62: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 62
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7958bfdf67172760389ca8bd2bafa78d364c5c53

I tried to land this on beta with a=test-only but it looks like I'm not allowed to do this anymore.
I'm not sure what difficulties you hit, but that's still the policy. Anyway, I tagged it as affected specifically so it'd show up my "test-only fixes ready to ride-along with the next round of uplifts" query anyway :)

https://hg.mozilla.org/releases/mozilla-beta/rev/5d7d7ee1bbd1
status-firefox61: affected → fixed
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.