Closed Bug 614643 Opened 15 years ago Closed 15 years ago

Intermittent failure in browser/toolkit/content/tests/browser/browser_bug295977_autoscroll_overflow.js | g should have scrolled vertically (or any other letter) (or horizontally)

Categories

(Toolkit :: UI Widgets, defect)

Product:
Toolkit
Component:
UI Widgets
Platform:
x86
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla5
Tracking Flags:
Tracking Status
status2.0 --- ?

People

(Reporter: jdm, Assigned: ehsan.akhgari)

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file, 1 obsolete file)

Patch (v2)
3.43 KB, patch
Swatinem
: review+
Details | Diff | Splinter Review
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1290628411.1290629834.8192.gz Rev3 MacOSX Leopard 10.5.8 mozilla-central opt test mochitest-other on 2010/11/24 11:53:31 s: talos-r3-leopard-051 TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/content/tests/browser/browser_bug295977_autoscroll_overflow.js | g should have scrolled vertically TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/content/tests/browser/browser_bug295977_autoscroll_overflow.js | g should have scrolled horizontally
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1290619777.1290621332.1944.gz Rev3 MacOSX Leopard 10.5.8 mozilla-central opt test mochitest-other on 2010/11/24 09:29:37 s: talos-r3-leopard-022 TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/content/tests/browser/browser_bug295977_autoscroll_overflow.js | d should have scrolled vertically
Blocks: 295977
This is almost certainly a regression from bug 588435, since it started to happen a few days after that bug was landed. Looking at http://hg.mozilla.org/mozilla-central/rev/3b157fcde708, there's something which bothers me there: 1.76 + doc.getElementById("forceredraw").style.left = ev.timeStamp % 100; There is no guarantee that |ev.timeStamp % 100| is not equal to the current value of |doc.getElementById("forceredraw").style.left|. I did some local tests, and I confirmed that if that happens on the first iteration, we get a timeout, and if that happens on the second iteration, we get a failure similar to this bug. I have a patch which makes sure that the repaint happens in a more solid way.
Blocks: 588435
Attached patch Patch (v1) (obsolete) — — Splinter Review
Assignee: nobody → ehsan
Status: NEW → ASSIGNED
Attachment #519316 - Flags: review?(neil)
Comment on attachment 519316 [details] [diff] [review] Patch (v1) Sorry, I didn't look at the test closely enough, otherwise I might have noticed the curly quotes in the comments for instance :s
Attachment #519316 - Flags: review?(neil) → review+
http://hg.mozilla.org/mozilla-central/rev/bd942ed342bb
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0
Happened again, reopening.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Note that mozilla-2.0 has been branched off.
Target Milestone: mozilla2.0 → ---
When writing these tests, I had to skip a few frames (see http://hg.mozilla.org/mozilla-central/file/bd942ed342bb/toolkit/content/tests/browser/browser_bug295977_autoscroll_overflow.js#l32 ) before checking if the actions had any effect. So AfterPaint fires before the logic in BeforePaint actually took place. Or that AfterPaint does not guarantee that BeforePaint was even fired. Increasing the number of skipped frames may help as an intermediate solution. But the problem itself lies somewhere else, maybe someone can enlighten me: These are the steps that should (but do not apparently) occur in order: - We synthesize the mouse click/move (this is async I believe? maybe the problem source is right there?) -- Thus the autoscroll handler becomes active and does his own requestAnimationFrame/BeforePaint magic - We add our AfterPaint listener in the test, hoping that the above BeforePaint magic already happened - The test rolls its own requestAnimationFrame magic to also test the cases where there should be no magic done by the autoscroll handler. (- The test skips 3 frames as mentioned above) So how can we guarantee that the autoscroll handlers BeforePaint actually does his job before the tests AfterPaint checks for the result, especially without waiting 3 or more frames for each test (which adds up to longer test runtime)?!?
Attached patch Patch (v2) — — Splinter Review
OK, I investigated this test on IRC with Arpad more closely. It seems that generally, the assumption that each beforepaint event is followed by an afterpaint event is incorrect. These two events can fire in varying degrees, seemingly randomly. A more robust approach would be to just use the MozBeforePaint event. The first time that this event fires we should ignore it, since that's when the browser starts autoscroll. We should handle it at the second time, and perform the test then.
Attachment #519316 - Attachment is obsolete: true
Attachment #520375 - Flags: review?(arpad.borsos)
Comment on attachment 520375 [details] [diff] [review] Patch (v2) I guess you have tested it to make sure it works? (I haven’t)
Attachment #520375 - Flags: review?(arpad.borsos) → review+
(In reply to comment #260) > Comment on attachment 520375 [details] [diff] [review] > --> https://bugzilla.mozilla.org/attachment.cgi?id=520375 > Patch (v2) > > I guess you have tested it to make sure it works? (I haven’t) Yes, I have!
Depends on: post2.0
Whiteboard: [orange] → [orange][post-2.0]
http://hg.mozilla.org/projects/cedar/rev/b141be707734
Whiteboard: [orange][post-2.0] → [orange][post-2.0][fixed-in-cedar]
http://hg.mozilla.org/mozilla-central/rev/8f5bcdfe4e0e
Status: REOPENED → RESOLVED
Closed: 15 years ago → 15 years ago
No longer depends on: post2.0
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [orange][post-2.0][fixed-in-cedar] → [orange]
Target Milestone: --- → mozilla2.2
status2.0: --- → ?
Version: unspecified → Trunk
Whiteboard: [orange]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: