Closed Bug 702184 Opened 13 years ago Closed 12 years ago

sporadic REFTEST TEST-UNEXPECTED-FAIL | reftest/tests/layout/reftests/bugs/598726-1.html | image comparison (==)

Categories

(Core :: CSS Parsing and Computation, defect)

Product:
Core
Component:
CSS Parsing and Computation
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla12

People

(Reporter: dholbert, Assigned: fryn)

References

(
URL
)

Details

(Keywords: intermittent-failure)

Attachments

(4 files, 1 obsolete file)

reftest log from comment 0 (57px diff, before frequent failures started)
15.88 KB, text/plain
Details
reftest log from comment 180 (888px diff, fails frequently)
15.05 KB, text/plain
Details
wallpaper, v2
1.60 KB, patch
ehsan.akhgari
: review+
mounir
: checkin+
Details | Diff | Splinter Review
patch
2.62 KB, patch
ehsan.akhgari
: review+
Details | Diff | Splinter Review 
https://tbpl.mozilla.org/php/getParsedLog.php?id=7379176&tree=Mozilla-Inbound
Rev3 MacOSX Leopard 10.5.8 mozilla-inbound debug test reftest on 2011-11-13 17:44:50 PST for push 9f2d2db76c57
{
REFTEST TEST-UNEXPECTED-FAIL | file:///Users/cltbld/talos-slave/test/build/reftest/tests/layout/reftests/bugs/598726-1.html | image comparison (==)
}

The difference isn't visually noticeable (to me at least) -- it's a 53px difference, and it seems to be in antialiasing along the edges of the characters in the text field.
OS: Windows 7 → All
Hardware: x86_64 → All
Timeline of this bug:

* 2011-01-19: This test was checked in
    (...10 months elapse...)
* 2011-11-13: random orange reported (comment 0)
    (...1 month elapses...)
* 2011-12-20: second instance of this random orange (comment 2), and now it starts failing ~13 times per day, continuing up to the present.

So apparently something changed to make this really bad on (or just before) 12/20.

(note: I'm explicitly ignoring comment 1, because it's not this bug -- it's a timeout)

Also: glancing at the last few logs' reftest snapshots, the difference is much more substantial (and "real bug" looking) than comment 0.

Comment 0 just had 53px different, and they each are e.g. rgb(n,n,n) vs rgb(n+1,n+1,n+1) antialiased edges of text. (not visually noticeable)  In contrast, the most recent few comments have hundreds to thousands of pixels different, and it's very visually noticeable -- the text box in the testcase has a different size / position / styling (depending on the platform).
So seems like the problem here is that the input box is not correctly painted by the time that the transitionend event is fired.  Can you guys imagine why?
The "guilty" pushlog (on m-i) doesn't suggest anything obvious...

The earliest report in the spammy-range was at 2011-12-20 7:25 AM, and it failed 4 times in the ~2 hours thereafter.  Just to be super-generous, here's the pushlog from 8am the previous day up to 8am that day:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?startdate=2011-12-19+08%3A00%3A00&enddate=2011-12-20+08%3A00%3A00

Nothing jumps out at me there.
Hmm, http://hg.mozilla.org/integration/mozilla-inbound/rev/21b0f339bf52 was a non-trivial change to the reftest harness itself at 4:46 that morning. I wonder if that might have been responsible?

http://hg.mozilla.org/integration/mozilla-inbound/rev/7f8c69ad0895 might also be relevant... (only if nsGfxScrollFrame gets used at all for simple <input> text boxes... I forget if it does)

I think we can also ignore the m-c to m-i merges in that pushlog.  If this had come from a m-c merge, then we would've gotten spammy-report from m-c first -- but we didn't.
Attachment #574229 - Attachment description: reftest log → reftest log from comment 0 (before this got spammy)
Attachment #574229 - Attachment description: reftest log from comment 0 (before this got spammy) → reftest log from comment 0 (57px diff, before frequent failures started)
(In reply to Daniel Holbert [:dholbert] from comment #181)
> Hmm, http://hg.mozilla.org/integration/mozilla-inbound/rev/21b0f339bf52 was
> a non-trivial change to the reftest harness itself at 4:46 that morning. I
> wonder if that might have been responsible?
(In reply to Ehsan Akhgari [:ehsan] from comment #182)
> CCing jmaher, author of
> http://hg.mozilla.org/integration/mozilla-inbound/rev/21b0f339bf52.

I think that one's innocent, actually.  I just did 2 TryServer pushes, for that cset and its parent (b52b4576f9f1), and gave both pushes tons of reftest runs.  The parent cset has already hit this failure on 3/13 WinXP Debug runs, which I think is enough to classify it in the "spammy" range -- hence, its child 21b0f339bf52 from jmaher is innocent.

Hence, un-CC'ing jmaher to spare him the tbplbot mail.

For reference, the TryServer run for that parent cset is:
  https://tbpl.mozilla.org/?tree=Try&rev=3ca52aea0573
Attached patch wallpaper (obsolete) — — Splinter Review 
Here's a wallpaper that I think would cure the orange problem.
(In reply to Daniel Holbert [:dholbert] from comment #179)
> The earliest report in the spammy-range was at 2011-12-20 7:25 AM, and it
> failed 4 times in the ~2 hours thereafter.  Just to be super-generous,
> here's the pushlog from 8am the previous day up to 8am that day:
> http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?startdate=2011-
> 12-19+08%3A00%3A00&enddate=2011-12-20+08%3A00%3A00

I did a few more try pushes from that range, and I got a lucky winner - Mats' push for bug 619273:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=0aa9c3a5b7e1

Try run from that push (0aa9c3a5b7e1):
  https://tbpl.mozilla.org/?tree=Try&rev=7d2060424376

Try run from its parent (feaccb6a4c35):
  https://tbpl.mozilla.org/?tree=Try&rev=a6cecf97e645

I gave both of the above Try runs 40-50 reftest jobs on WinXP Debug.
- On the blamed 0aa9c3a5b7e1 push, 11 of those jobs suffered from this bug.
- On the parent feaccb6a4c35 push, 0  of those jobs suffered from this bug.
That seems pretty definitive to me.

Hence, setting this as blocking as blocking bug 619273.  (While comment 0 was from before that bug landed, the remainder of the reports here are from the "spammy range" after it landed, so I think it's reasonable to consider this largely a regression from bug 619273.)
Blocks: 619273
(In reply to Daniel Holbert [:dholbert] from comment #211)
> so I think it's reasonable to consider
> this largely a regression from bug 619273.)

Yeah, I can see how bug 619273 could have affected this test through the
auto-selection of the <input> value on focus.
I doubt there's a bug in the code though, the test works fine with the
attached wallpaper.  I did ~50 reftest runs on it yesterday without
any orange.
Attachment #585938 - Flags: review?(dholbert)
Comment on attachment 585938 [details] [diff] [review]
wallpaper

ehsan's probably a better reviewer for this, as he wrote the test being tweaked.
Attachment #585938 - Flags: review?(dholbert) → review?(ehsan)
Comment on attachment 585938 [details] [diff] [review]
wallpaper

Does the orange happen without the 100ms timeouts?  If you expect the flushing to help, we should really keep the timeouts 0.  If not, then this will only make the orange happen less frequently, I think.
Attachment #585938 - Flags: review?(ehsan) → review-
Attached patch wallpaper, v2 — — Splinter Review 
Yeah, that works - none of the oranges are for this bug:
https://tbpl.mozilla.org/?tree=Try&rev=b70564cb4ef5
Attachment #585938 - Attachment is obsolete: true
Attachment #586540 - Flags: review?(ehsan)
Comment on attachment 586540 [details] [diff] [review]
wallpaper, v2

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

r=me, thanks!
Attachment #586540 - Flags: review?(ehsan) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/0f5d650bf2eb
Whiteboard: [orange] → [orange][inbound]
Target Milestone: --- → mozilla12
FTR, comment 303 is for a build before the fix landed.
https://hg.mozilla.org/mozilla-central/rev/0f5d650bf2eb

\o/
Assignee: nobody → matspal
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
This just reappeared

https://tbpl.mozilla.org/php/getParsedLog.php?id=9381057&tree=Mozilla-Inbound
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
The landing of the patches in bug 673873 is causing this test to fail a lot more often.
Blocks: 673873
Comment on attachment 586540 [details] [diff] [review]
wallpaper, v2

Marking as checked-in so it's easier to understand a fix has been tried, it's not coming.
Attachment #586540 - Flags: checkin+
This is now blocking the landing of the patches in bug 673873.
AFAICT, it's the test that is unreliable.
The code changes I'm making in bug 673873 don't seem actually to regress bug 598726.
Could we fix the test or disable it for now?
Attached patch patch — — Splinter Review 
Removed setTimeouts and reduced transition-duration to 200ms. I don't think we need to spend a full second (.5s * 2) in a general attempt to prevent "future random breakage of unrelated things".
Assignee: matspal → fryn
Status: REOPENED → ASSIGNED
Attachment #626904 - Flags: review?(ehsan)
Attachment #626904 - Flags: review?(ehsan) → review+
https://hg.mozilla.org/mozilla-central/rev/56715f4d5e4e
Status: ASSIGNED → RESOLVED
Closed: 12 years ago → 12 years ago
Resolution: --- → FIXED
Comment 628 was actually a different failure than comment 0; it was only 13 pixels and they were in a vertical line to the left of the text field.
Depends on: 782196
Whiteboard: [orange]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: