Bug 196985 - REGRESSION: (r244182) Layout Test editing/execCommand/insert-nested-lists.html is flaky
Summary: REGRESSION: (r244182) Layout Test editing/execCommand/insert-nested-lists.htm...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-04-16 14:25 PDT by Truitt Savell
Modified: 2019-04-22 19:39 PDT (History)
9 users (show)

See Also:


Attachments
Fixes the test (1.82 KB, patch)
2019-04-19 12:31 PDT, Ryosuke Niwa
wenson_hsieh: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Truitt Savell 2019-04-16 14:25:51 PDT
The following layout test is flaky on Mac Release WK2

editing/execCommand/insert-nested-lists.html

Probable cause:

This test became flakey with https://trac.webkit.org/changeset/244182/webkit

I reproduced this issue with command:
run-webkit-tests editing/execCommand/insert-nested-lists.html --iterations 500 -f

The test fails randomly on 244182 and passes fully on 244181. 

Flakiness Dashboard:

https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=editing%2FexecCommand%2Finsert-nested-lists.html

Diff:
--- /Volumes/Data/slave/mojave-release-tests-wk2/build/layout-test-results/editing/execCommand/insert-nested-lists-expected.txt
+++ /Volumes/Data/slave/mojave-release-tests-wk2/build/layout-test-results/editing/execCommand/insert-nested-lists-actual.txt
@@ -43,52 +43,39 @@
 |     <br>
 |   <ol>
 |     <li>
-|       "bar"
-|     <ul>
-|       <li>
-|         <#selection-caret>
+|       <#selection-caret>
 
 After selecting a range and inserting another unordered list:
 | <ul>
 |   <li>
 |     "foo"
 |     <br>
-|   <ol>
+|   <ul>
 |     <li>
-|       "bar"
-|     <ul>
+|       <#selection-caret>
+|     <ol>
 |       <li>
-|         <#selection-caret>
-|       <ul>
-|         <li>
 
 After outdenting:
 | <ul>
 |   <li>
 |     "foo"
 |     <br>
-|   <ol>
-|     <li>
-|       "bar"
-|     <li>
-|       <#selection-caret>
-|       <br>
-|     <ul>
-|       <ul>
-|         <li>
+|   <li>
+|     <#selection-caret>
+|     <br>
+|   <ul>
+|     <ol>
+|       <li>
 
 After outdenting again:
 | <ul>
 |   <li>
 |     "foo"
 |     <br>
-|   <ol>
-|     <li>
-|       "bar"
-|   <li>
-|     "baz<#selection-caret>"
-|     <br>
-|   <ol>
-|     <ul>
-|       <ul>
-|         <li>
+| "baz<#selection-caret>"
+| <br>
+| <ul>
+|   <ul>
+|     <ol>
+|       <li>
Comment 1 Radar WebKit Bug Importer 2019-04-16 14:27:22 PDT
<rdar://problem/49954111>
Comment 2 Alexey Proskuryakov 2019-04-18 10:02:55 PDT
This is a bit surprising. This test doesn't use rAF, and generally looks like it should be deterministic.

Said, Wenson, any ideas on how the regression could happen here?
Comment 3 Wenson Hsieh 2019-04-18 10:04:47 PDT
(In reply to Alexey Proskuryakov from comment #2)
> This is a bit surprising. This test doesn't use rAF, and generally looks
> like it should be deterministic.
> 
> Said, Wenson, any ideas on how the regression could happen here?

It actually does use requestAnimationFrame:

await new Promise(resolve => requestAnimationFrame(resolve));
Comment 4 Shawn Roberts 2019-04-18 11:27:45 PDT
Found another editing test regressed by r244182.

Linking to 197065
Comment 5 Ryosuke Niwa 2019-04-19 12:25:33 PDT
There is no reason to believe that waiting for rAF would cause UI process' runloop to run. So we need to be making an explicit round trip to UI process instead.
Comment 6 Ryosuke Niwa 2019-04-19 12:31:12 PDT
Created attachment 367811 [details]
Fixes the test
Comment 7 Ryosuke Niwa 2019-04-19 12:33:25 PDT
Committed r244461: <https://trac.webkit.org/changeset/244461>
Comment 8 Ryosuke Niwa 2019-04-22 19:39:26 PDT
Looks like the flakiness is gone!