Bug 97558 - [chromium] platform/chromium/fast/forms/date/date-suggestion-picker-appearance-rtl.html is flaky on mac debug
: [chromium] platform/chromium/fast/forms/date/date-suggestion-picker-appearanc...
Status: RESOLVED WONTFIX
Product: WebKit
Classification: Unclassified
Component: Forms
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To: Keishi Hattori
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-09-25 05:21 PDT by Keishi Hattori
Modified: 2013-04-09 17:07 PDT (History)
7 users (show)

See Also:


Attachments
Patch (8.12 KB, patch)
2012-10-01 16:37 PDT, Keishi Hattori
ojan: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Keishi Hattori 2012-09-25 05:21:38 PDT
Rebaseline test fast/forms/date/date-suggestion-picker-appearance.html
Comment 1 Ojan Vafai 2012-09-27 14:30:10 PDT
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=platform%2Fchromium%2Ffast%2Fforms%2Fdate%2Fdate-suggestion-picker-appearance-rtl.html

It appears that the text cursor is sometimes shown and sometimes hidden. Seems like a race condition. I thought we had code in DRT to explicitly make it so the cursor didn't blink and was instead always visible.
Comment 2 Ojan Vafai 2012-10-01 12:09:52 PDT
Ryosuke pointed me at FrameSelection::setCaretBlinkingSuspended. But that can be called by non-DRT code to set the caret blinking again. I think we need a method in Settings that disables caret blinking and makes the caret always be visible.
Comment 3 Dirk Pranke 2012-10-01 12:15:00 PDT
that's a fun source of flakiness, alright.
Comment 4 Keishi Hattori 2012-10-01 16:37:11 PDT
Created attachment 166569 [details]
Patch
Comment 5 Ojan Vafai 2012-10-01 16:45:00 PDT
Comment on attachment 166569 [details]
Patch

Thanks for working on this! I was picturing that we'd change this setting from DumpRenderTree for all tests. I can't think of any test where we'd want caret blinking to be enabled. Any test that doesn't set this and has a caret will be flaky, no?
Comment 6 Kent Tamura 2012-10-01 18:44:01 PDT
If the number of setCaretBlinkingSuspended(true) calls and the number of setCaretBlinkingSuspended(false) calls are matched, we had better make FrameSelection::m_isCaretBlinkingSuspended an integer, and change the code so that setCaretBlinkingSuspended(true) increases the integer, setCaretBlinkingSuspended(false) decreases the integer, isCaretBlinkingSuspended() returns true if the integer is <= 0.
Comment 7 Ryosuke Niwa 2012-10-01 18:44:57 PDT
I think we should first make sure that our conjuncture about the bug is correct.
Comment 8 Ojan Vafai 2012-10-01 18:45:58 PDT
(In reply to comment #7)
> I think we should first make sure that our conjuncture about the bug is correct.

What is there to verify other than the clear results in the pixel output (i.e. sometimes it passes and other times it fails with a missing caret)?
Comment 9 Ryosuke Niwa 2012-10-01 18:50:25 PDT
(In reply to comment #8)
> (In reply to comment #7)
> > I think we should first make sure that our conjuncture about the bug is correct.
> 
> What is there to verify other than the clear results in the pixel output (i.e. sometimes it passes and other times it fails with a missing caret)?

I think we should at least verify that FrameSelection::setCaretBlinkingSuspended is what DRT uses to stop the blinks, and that there is some call to setCaretBlinkingSuspended during this test (with a race) that causes the test to fail sometimes. In particular, it'll be really nice if we learn what the races are.
Comment 10 Ojan Vafai 2012-10-01 18:54:15 PDT
As best I can tell, we were wrong when we thought DRT did something to control caret blinking. I couldn't find any code that did. It does not call setCaretBlinkingSuspended. So the race seems straightforward if I'm not mistaken. It's just that the caret is blinking.
Comment 11 Keishi Hattori 2012-10-01 19:02:02 PDT
(In reply to comment #10)
> As best I can tell, we were wrong when we thought DRT did something to control caret blinking. I couldn't find any code that did. It does not call setCaretBlinkingSuspended. So the race seems straightforward if I'm not mistaken. It's just that the caret is blinking.

That was my conclusion too.
Comment 12 Stephen Chenney 2013-04-09 17:07:13 PDT
Marked LayoutTest bugs, bugs with Chromium IDs, and some others as WontFix. Test failure bugs still are trackable via TestExpectations or disabled unit tests.