Deflake repaint layout tests which assumed scroll would happen before first paint.

Deflake repaint layout tests which assumed scroll would happen before first paint.

Introduces runAfterDisplay to text-based-repaint.js which can be used to
run an operation after the first display.

BUG=432129
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=187048

[rmcilroy, 2014-12-12 15:37:14]

rmcilroy@chromium.org changed reviewers:
+ wangxianzhu@chromium.org

[rmcilroy, 2014-12-12 15:37:49]

Xianzhu: could you please take a look, thanks.

[Xianzhu, 2014-12-12 16:42:00]

The change to the tests and script look good.

I scheduled some tryjobs and would like to see the result.

https://codereview.chromium.org/791453007/diff/1/LayoutTests/fast/repaint/hor...
File LayoutTests/fast/repaint/horizontal-bt-overflow-child-expected.txt (right):

https://codereview.chromium.org/791453007/diff/1/LayoutTests/fast/repaint/hor...
LayoutTests/fast/repaint/horizontal-bt-overflow-child-expected.txt:13: [29, 20,
100, 86]
Could you explain this change?

https://codereview.chromium.org/791453007/diff/1/LayoutTests/fast/repaint/res...
File LayoutTests/fast/repaint/resources/text-based-repaint.js (right):

https://codereview.chromium.org/791453007/diff/1/LayoutTests/fast/repaint/res...
LayoutTests/fast/repaint/resources/text-based-repaint.js:62: 
Nit: remove this line.

[rmcilroy, 2014-12-12 17:20:17]

https://codereview.chromium.org/791453007/diff/1/LayoutTests/fast/repaint/hor...
File LayoutTests/fast/repaint/horizontal-bt-overflow-child-expected.txt (right):

https://codereview.chromium.org/791453007/diff/1/LayoutTests/fast/repaint/hor...
LayoutTests/fast/repaint/horizontal-bt-overflow-child-expected.txt:13: [29, 20,
100, 86]
On 2014/12/12 16:42:00, Xianzhu wrote:
> Could you explain this change?

I'm assuming these are the invalidations which are due to the scroll.  Are you
meaning the last invalidation in particular?

https://codereview.chromium.org/791453007/diff/1/LayoutTests/fast/repaint/res...
File LayoutTests/fast/repaint/resources/text-based-repaint.js (right):

https://codereview.chromium.org/791453007/diff/1/LayoutTests/fast/repaint/res...
LayoutTests/fast/repaint/resources/text-based-repaint.js:62: 
On 2014/12/12 16:42:00, Xianzhu wrote:
> Nit: remove this line.

Done.

[Xianzhu, 2014-12-12 17:37:36]

https://codereview.chromium.org/791453007/diff/1/LayoutTests/fast/repaint/hor...
File LayoutTests/fast/repaint/horizontal-bt-overflow-child-expected.txt (right):

https://codereview.chromium.org/791453007/diff/1/LayoutTests/fast/repaint/hor...
LayoutTests/fast/repaint/horizontal-bt-overflow-child-expected.txt:13: [29, 20,
100, 86]
On 2014/12/12 17:20:17, rmcilroy wrote:
> On 2014/12/12 16:42:00, Xianzhu wrote:
> > Could you explain this change?
> 
> I'm assuming these are the invalidations which are due to the scroll.  Are you
> meaning the last invalidation in particular?

I mean all the new invalidations. The scroll happens before the repaint test
(and before a displayAsyncThen), so invalidations caused by the scroll should
happen before we start tracking the invalidations and should not be in the
result.

Was this result generated when the scheduler was on or off? If it generates this
result when scheduler is off, I'd like to debug and see what happened.

[rmcilroy, 2014-12-12 18:34:44]

On 2014/12/12 17:37:36, Xianzhu wrote:
>
https://codereview.chromium.org/791453007/diff/1/LayoutTests/fast/repaint/hor...
> File LayoutTests/fast/repaint/horizontal-bt-overflow-child-expected.txt
(right):
> 
>
https://codereview.chromium.org/791453007/diff/1/LayoutTests/fast/repaint/hor...
> LayoutTests/fast/repaint/horizontal-bt-overflow-child-expected.txt:13: [29,
20,
> 100, 86]
> On 2014/12/12 17:20:17, rmcilroy wrote:
> > On 2014/12/12 16:42:00, Xianzhu wrote:
> > > Could you explain this change?
> > 
> > I'm assuming these are the invalidations which are due to the scroll.  Are
you
> > meaning the last invalidation in particular?
> 
> I mean all the new invalidations. The scroll happens before the repaint test
> (and before a displayAsyncThen), so invalidations caused by the scroll should
> happen before we start tracking the invalidations and should not be in the
> result.
>
> Was this result generated when the scheduler was on or off? If it generates
this
> result when scheduler is off, I'd like to debug and see what happened.

This is the result for scheduler being off (we also get the same results with
the scheduler on).  I'm not quite sure why this is happening in this case, but I
don't have much knowledge in this area. If you are able to have a look that
would be great, otherwise I will try to figure out what's going on next week
(end of day in LON here).

[Xianzhu, 2014-12-12 18:44:38]

Debugged with the patch. There seems an existing bug causing the new
invalidations. I'll look into this and file a bug.

The patch lgtm.

[Xianzhu, 2014-12-12 18:45:01]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2014-12-12 18:46:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/791453007/20001

[rmcilroy, 2014-12-12 18:49:48]

On 2014/12/12 18:44:38, Xianzhu wrote:
> Debugged with the patch. There seems an existing bug causing the new
> invalidations. I'll look into this and file a bug.
> 
> The patch lgtm.

Thanks for debugging, and thanks for the review!

[rmcilroy, 2014-12-12 18:49:50]

On 2014/12/12 18:44:38, Xianzhu wrote:
> Debugged with the patch. There seems an existing bug causing the new
> invalidations. I'll look into this and file a bug.
> 
> The patch lgtm.

Thanks for debugging, and thanks for the review!

[commit-bot: I haz the power, 2014-12-12 19:32:52]

Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=187048