648882 - TabPriorityQueue updates tabItems when UI is busy

Status: RESOLVED FIXED

(Firefox Graveyard :: Panorama, defect)

Description

[Tim Taubert [:ttaubert] (inactive)]

The condition is now:

if (this.isPaintingPaused() || !UI.isIdle)

and should be:

if (this.isPaintingPaused() || !UI.isIdle())

We should add a test to verify that no tabItem updates are done when resizing or moving items.

Comment 1

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch v1

Comment 2

[Raymond Lee [:raymondlee]]

Comment on attachment 524979 [details] [diff] [review]
patch v1

+      EventUtils.synthesizeMouse(container, 5, 5, {type: "mousedown"}, cw);
+      EventUtils.synthesizeMouse(container, 40, 20, {type: "mousemove"}, cw);
+      EventUtils.synthesizeMouse(container, 20, 20, {type: "mouseup"}, cw);

The above repeats twice, might be good to write a helper function.

f+

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 524979 [details] [diff] [review]
patch v1

Passed try:

http://tbpl.mozilla.org/?tree=MozillaTry&pusher=[e-mail]&rev=20df3dc1f2fd

Comment 4

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch v2

Comment 5

[Ian Gilman [:iangilman]]

Comment on attachment 525095 [details] [diff] [review]
patch v2

That's a pretty slick test :)

Comment 6

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch for checkin

Comment 7

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch for checkin (added waitForFocus)

Comment 8

[(no longer active)]

http://hg.mozilla.org/projects/cedar/rev/b1fc69fab8b2

Comment 9

[(no longer active)]

The test added by this patch went orange: <http://tinderbox.mozilla.org/showlog.cgi?log=Cedar/1302807869.1302809327.10302.gz> so I backed it out: <http://hg.mozilla.org/projects/cedar/rev/53339e6932c3>

Comment 10

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch v3

Added another test for another issue fixed with this patch.

Comment 11

[Raymond Lee [:raymondlee]]

Comment on attachment 526118 [details] [diff] [review]
patch v3

Looks good.  Can we merge two tests file into one?

Comment 12

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch v4

(In reply to comment #11)
> Looks good.  Can we merge two tests file into one?

Of course, don't know why I split them :)

Comment 13

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 526179 [details] [diff] [review]
patch v4

Failed on OSX opt:

http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1302848610.1302854494.9835.gz

Investigating.

Comment 14

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch v5

Comment 15

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 526333 [details] [diff] [review]
patch v5

Passed try:

http://tbpl.mozilla.org/?tree=MozillaTry&pusher=[e-mail]&rev=c04433b1ca25

Comment 16

[Ian Gilman [:iangilman]]

Comment on attachment 526333 [details] [diff] [review]
patch v5

Looks good.

Comment 17

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch for checkin

Comment 18

[Mounir Lamouri (:mounir)]

Pushed:
http://hg.mozilla.org/mozilla-central/rev/028fc2dcb854

Comment 19

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

The test if failing: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1303305581.1303307355.28425.gz
I've made a backout: http://hg.mozilla.org/mozilla-central/rev/347ea6f52589

Comment 20

[Mounir Lamouri (:mounir)]

You should not use setTimeout(func, value) with value>0 if you do not want your test to be flaky.

Comment 21

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch v6 (without setTimeout)

(In reply to comment #20)
> You should not use setTimeout(func, value) with value>0 if you do not want your
> test to be flaky.

Good to know, removed.

Comment 22

[Raymond Lee [:raymondlee]]

Comment on attachment 527343 [details] [diff] [review]
patch v6 (without setTimeout)

Looks good

Comment 23

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 527343 [details] [diff] [review]
patch v6 (without setTimeout)

Passed try:

http://tbpl.mozilla.org/?tree=MozillaTry&pusher=[e-mail]&rev=43fe3397f7a0

Comment 24

[Ian Gilman [:iangilman]]

Comment on attachment 527343 [details] [diff] [review]
patch v6 (without setTimeout)

Review of attachment 527343 [details] [diff] [review]:

Very clever :)

Comment 25

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch for checkin

Comment 26

[Mounir Lamouri (:mounir)]

Pushed:
http://hg.mozilla.org/mozilla-central/rev/4e7e81ad7525

Thank you for fixing the random orange Tim :)