Bug 203264 - [iOS] 3 editing/pasteboard/smart-paste-paragraph-* tests are flaky
Summary: [iOS] 3 editing/pasteboard/smart-paste-paragraph-* tests are flaky
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: Wenson Hsieh
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-10-22 13:14 PDT by Russell Epstein
Modified: 2019-10-28 12:58 PDT (History)
11 users (show)

See Also:


Attachments
Patch (5.63 KB, patch)
2019-10-25 20:14 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff
Rename a variable (5.76 KB, patch)
2019-10-25 20:18 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Russell Epstein 2019-10-22 13:14:08 PDT
The following three layout tests have been flaky on iOS since they landed in r243124:

editing/pasteboard/smart-paste-paragraph-001.html
editing/pasteboard/smart-paste-paragraph-002.html
editing/pasteboard/smart-paste-paragraph-004.html

Test History:

https://results.webkit.org/?suite=layout-tests&suite=layout-tests&suite=layout-tests&test=editing%2Fpasteboard%2Fsmart-paste-paragraph-001.html&test=editing%2Fpasteboard%2Fsmart-paste-paragraph-002.html&test=editing%2Fpasteboard%2Fsmart-paste-paragraph-004.html&platform=ios


Steps to Reproduce:

run-webkit-tests --ios-simulator --no-retry --no-sample-on-timeout -f editing/pasteboard/smart-paste-paragraph-001.html editing/pasteboard/smart-paste-paragraph-002.html editing/pasteboard/smart-paste-paragraph-004.html --iter 1000


Diffs:


editing/pasteboard/smart-paste-paragraph-001.html
Diff:

--- /Volumes/Data/slave/ios-simulator-13-release-tests-wk2/build/layout-test-results/editing/pasteboard/smart-paste-paragraph-001-expected.txt
+++ /Volumes/Data/slave/ios-simulator-13-release-tests-wk2/build/layout-test-results/editing/pasteboard/smart-paste-paragraph-001-actual.txt
@@ -7,10 +7,6 @@
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 0 of DIV > DIV > BODY > HTML > #document to 0 of DIV > DIV > BODY > HTML > #document givenAction:WebViewInsertActionPasted
-EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 15 of #text > DIV > DIV > BODY > HTML > #document to 15 of #text > DIV > DIV > BODY > HTML > #document toDOMRange:range from 0 of DIV > DIV > BODY > HTML > #document to 0 of DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
-EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 Tests: 
 Smart paste when pasting a paragraph between two paragraphs.
 Expected Results: 
@@ -22,8 +18,6 @@
 Last test paragraph.
 Test paragraph.
 
-Test paragraph.
-
 Last test paragraph.
 execCopyCommand: <div id="test"> Test paragraph. </div> <div><br></div> <div> Last test paragraph. </div>
-execPasteCommand: <div id="test"> Test paragraph. </div> <div><br></div><div>Test paragraph.<br></div><div><br></div> <div> Last test paragraph. </div>
+execPasteCommand: <div id="test"> Test paragraph. </div> <div><br></div> <div> Last test paragraph. </div>

editing/pasteboard/smart-paste-paragraph-002.html
Diff:

--- /Volumes/Data/slave/ios-simulator-13-release-tests-wk2/build/layout-test-results/editing/pasteboard/smart-paste-paragraph-002-expected.txt
+++ /Volumes/Data/slave/ios-simulator-13-release-tests-wk2/build/layout-test-results/editing/pasteboard/smart-paste-paragraph-002-actual.txt
@@ -7,10 +7,6 @@
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 12 of #text > PRE > BODY > HTML > #document to 12 of #text > PRE > BODY > HTML > #document givenAction:WebViewInsertActionPasted
-EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 3 of PRE > BODY > HTML > #document to 3 of PRE > BODY > HTML > #document toDOMRange:range from 0 of PRE > PRE > BODY > HTML > #document to 0 of PRE > PRE > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
-EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 Tests: 
 Smart paste when pasting a paragraph between two paragraphs in a pre element.
 Expected Results: 
@@ -22,8 +18,6 @@
 Last test paragraph.
 Test paragraph.
 
-Test paragraph.
-
 Last test paragraph.
 execCopyCommand: <span id="test">Test</span> paragraph. <br>Last test paragraph.
-execPasteCommand: <span id="test">Test</span> paragraph. <pre id="root" class="editing" style="word-wrap: break-word;"><span><br></span></pre><pre id="root" class="editing" style="word-wrap: break-word;"><span id="test">Test</span> paragraph.</pre><pre id="root" class="editing" style="word-wrap: break-word;"><br></pre>Last test paragraph.
+execPasteCommand: <span id="test">Test</span> paragraph. <br>Last test paragraph.

editing/pasteboard/smart-paste-paragraph-004.html
Diff:

--- /Volumes/Data/slave/ios-simulator-13-release-tests-wk2/build/layout-test-results/editing/pasteboard/smart-paste-paragraph-004-expected.txt
+++ /Volumes/Data/slave/ios-simulator-13-release-tests-wk2/build/layout-test-results/editing/pasteboard/smart-paste-paragraph-004-actual.txt
@@ -7,10 +7,6 @@
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 0 of DIV > DIV > BODY > HTML > #document to 0 of DIV > DIV > BODY > HTML > #document givenAction:WebViewInsertActionPasted
-EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 15 of #text > DIV > DIV > BODY > HTML > #document to 15 of #text > DIV > DIV > BODY > HTML > #document toDOMRange:range from 0 of DIV > DIV > BODY > HTML > #document to 0 of DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
-EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 0 of DIV > DIV > BODY > HTML > #document to 0 of DIV > DIV > BODY > HTML > #document givenAction:WebViewInsertActionPasted
 EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 15 of #text > DIV > DIV > BODY > HTML > #document to 15 of #text > DIV > DIV > BODY > HTML > #document toDOMRange:range from 0 of DIV > DIV > BODY > HTML > #document to 0 of DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
@@ -30,9 +26,7 @@
 
 Test paragraph.
 
-Test paragraph.
-
 Last test paragraph.
 execCopyCommand: <div id="test"> Test paragraph. </div> <div><br></div> <div> Last test paragraph. </div>
+execPasteCommand: <div id="test"> Test paragraph. </div> <div><br></div> <div> Last test paragraph. </div>
 execPasteCommand: <div id="test"> Test paragraph. </div> <div><br></div><div>Test paragraph.<br></div><div><br></div> <div> Last test paragraph. </div>
-execPasteCommand: <div id="test"> Test paragraph. </div> <div><br></div><div>Test paragraph.<br></div><div><br></div><div>Test paragraph.<br></div><div><br></div> <div> Last test paragraph. </div>
Comment 1 Radar WebKit Bug Importer 2019-10-22 13:14:44 PDT
<rdar://problem/56512107>
Comment 2 Russell Epstein 2019-10-22 13:29:22 PDT
Issue appears to be limited to Release builds.

Updated TestExpectations in r251449
Comment 3 Aakash Jain 2019-10-23 07:12:39 PDT
@Russell, Can you please check the fourth test as well: editing/pasteboard/smart-paste-paragraph-003.html

This is also seems extremely flaky and is slowing down EWS (ios-wk2 queue). e.g.: https://ews-build.webkit.org/#/builders/24/builds/2333
https://ews-build.webkit.org/#/builders/24/builds/2336
https://ews-build.webkit.org/#/builders/24/builds/2341
Comment 4 Truitt Savell 2019-10-23 11:26:04 PDT
Strange(In reply to Aakash Jain from comment #3)
> @Russell, Can you please check the fourth test as well:
> editing/pasteboard/smart-paste-paragraph-003.html
> 
> This is also seems extremely flaky and is slowing down EWS (ios-wk2 queue).
> e.g.: https://ews-build.webkit.org/#/builders/24/builds/2333
> https://ews-build.webkit.org/#/builders/24/builds/2336
> https://ews-build.webkit.org/#/builders/24/builds/2341

This is strange because we are not seeing that test at all on the testers. This may be a test list related to only the EWS
Comment 5 Aakash Jain 2019-10-25 14:36:43 PDT
(In reply to Truitt Savell from comment #4)
> This is strange because we are not seeing that test at all on the testers.
> This may be a test list related to only the EWS
This is clearly failing on EWS consistently, we need to do something about this test. editing/pasteboard/smart-paste-paragraph-003.html

e.g.: https://ews-build.webkit.org/#/builders/24/builds/2652
Comment 6 Jonathan Bedard 2019-10-25 15:20:15 PDT
(In reply to Aakash Jain from comment #5)
> (In reply to Truitt Savell from comment #4)
> > This is strange because we are not seeing that test at all on the testers.
> > This may be a test list related to only the EWS
> This is clearly failing on EWS consistently, we need to do something about
> this test. editing/pasteboard/smart-paste-paragraph-003.html
> 
> e.g.: https://ews-build.webkit.org/#/builders/24/builds/2652

I saw an example on GuardMalloc internally.
Comment 7 Truitt Savell 2019-10-25 15:50:12 PDT
I was able to reproduce this test failure locally by just running the test by itself. It fails every time. I thought it would require a test list
Comment 8 Truitt Savell 2019-10-25 16:01:52 PDT
So this is occurring on EWS becasue:

on regular testers the sharding of tests is like this:
editing/pasteboard/smart-paste-008.html
editing/pasteboard/smart-paste-in-text-control.html
editing/pasteboard/smart-paste-paragraph-001.html
editing/pasteboard/smart-paste-paragraph-002.html
editing/pasteboard/smart-paste-paragraph-003.html
editing/pasteboard/smart-paste-paragraph-004.html
editing/pasteboard/style-from-rules.html
editing/pasteboard/styled-element-markup.html

while on EWS the sharding is like this:
editing/pasteboard/smart-paste-006.html
editing/pasteboard/smart-paste-007.html
editing/pasteboard/smart-paste-008.html
editing/pasteboard/smart-paste-in-text-control.html
editing/pasteboard/smart-paste-paragraph-003.html
editing/pasteboard/style-from-rules.html
editing/pasteboard/styled-element-markup.html

because editing/pasteboard/smart-paste-paragraph-003.html is running without these other three tests it is failing. some state is not being reset.
Comment 9 Truitt Savell 2019-10-25 16:05:10 PDT
editing/pasteboard/smart-paste-paragraph-003.html fails even on a checkout of r246110 which I chose arbitrarily. so this is probably not a new failure but the test order must have changed recently
Comment 10 Truitt Savell 2019-10-25 16:13:04 PDT
marked test as failing in https://trac.webkit.org/changeset/251609/webkit to remedy this for now
Comment 11 Alexey Proskuryakov 2019-10-25 16:37:42 PDT
This makes me wonder if this has the same root cause as the bug Wenson just fixed, with software keyboard appearing unexpectedly.
Comment 12 Wenson Hsieh 2019-10-25 16:41:50 PDT
(In reply to Alexey Proskuryakov from comment #11)
> This makes me wonder if this has the same root cause as the bug Wenson just
> fixed, with software keyboard appearing unexpectedly.

I just ran this test with and without having the test force the software keyboard (using `UIHelper.setHardwareKeyboardAttached`), and confirmed that I see the same output in both cases.
Comment 13 Wenson Hsieh 2019-10-25 16:42:09 PDT
(In reply to Wenson Hsieh from comment #12)
> (In reply to Alexey Proskuryakov from comment #11)
> > This makes me wonder if this has the same root cause as the bug Wenson just
> > fixed, with software keyboard appearing unexpectedly.
> 
> I just ran this test with and without having the test force the software
> keyboard (using `UIHelper.setHardwareKeyboardAttached`), and confirmed that
> I see the same output in both cases.

(The same test failure output, that is)
Comment 14 Wenson Hsieh 2019-10-25 16:47:05 PDT
(In reply to Wenson Hsieh from comment #13)
> (In reply to Wenson Hsieh from comment #12)
> > (In reply to Alexey Proskuryakov from comment #11)
> > > This makes me wonder if this has the same root cause as the bug Wenson just
> > > fixed, with software keyboard appearing unexpectedly.
> > 
> > I just ran this test with and without having the test force the software
> > keyboard (using `UIHelper.setHardwareKeyboardAttached`), and confirmed that
> > I see the same output in both cases.
> 
> (The same test failure output, that is)

It seems that this line:

    await UIHelper.selectWordByDoubleTapOrClick(document.getElementById('test'));

…is failing to actually produce a ranged selection.
Comment 15 Wenson Hsieh 2019-10-25 16:50:29 PDT
(In reply to Wenson Hsieh from comment #14)
> (In reply to Wenson Hsieh from comment #13)
> > (In reply to Wenson Hsieh from comment #12)
> > > (In reply to Alexey Proskuryakov from comment #11)
> > > > This makes me wonder if this has the same root cause as the bug Wenson just
> > > > fixed, with software keyboard appearing unexpectedly.
> > > 
> > > I just ran this test with and without having the test force the software
> > > keyboard (using `UIHelper.setHardwareKeyboardAttached`), and confirmed that
> > > I see the same output in both cases.
> > 
> > (The same test failure output, that is)
> 
> It seems that this line:
> 
>     await
> UIHelper.selectWordByDoubleTapOrClick(document.getElementById('test'));
> 
> …is failing to actually produce a ranged selection.

The test seems to start passing if I perturb the double tap location a bit, like so:

diff --git a/LayoutTests/resources/ui-helper.js b/LayoutTests/resources/ui-helper.js
index 3aaba293a2c..49c3d526023 100644
--- a/LayoutTests/resources/ui-helper.js
+++ b/LayoutTests/resources/ui-helper.js
@@ -201,7 +201,8 @@ window.UIHelper = class UIHelper {
         const y = boundingRect.y + relativeY;
         if (this.isIOSFamily()) {
             await UIHelper.activateAndWaitForInputSessionAt(x, y);
-            await UIHelper.doubleTapAt(x, y);
+            await UIHelper.activateAt(x + 1, y + 1);
+            await UIHelper.activateAt(x + 1, y + 1);
             // This is only here to deal with async/sync copy/paste calls, so
             // once <rdar://problem/16207002> is resolved, should be able to remove for faster tests.
             await new Promise(resolve => testRunner.runUIScript("uiController.uiScriptComplete()", resolve));

…this appears to share the same root cause as https://bugs.webkit.org/show_bug.cgi?id=203392. We should really consider fixing position information request caching in WKContentViewInteraction, so that we don’t end up with a stale value for `_positionInformation.nodeAtPositionIsFocusedElement` after focusing an element.
Comment 16 Wenson Hsieh 2019-10-25 20:14:10 PDT Comment hidden (obsolete)
Comment 17 Wenson Hsieh 2019-10-25 20:18:57 PDT
Created attachment 381991 [details]
Rename a variable
Comment 18 Wenson Hsieh 2019-10-26 16:02:12 PDT
Comment on attachment 381991 [details]
Rename a variable

> Found 2 new test failures: fast/scrolling/ios/reveal-focused-element-right-above-keyboard-on-ipad.html, fast/scrolling/ios/frame-scrolling-no.html (failure)23 analyze-layout-tests-results

I wasn’t able to reproduce these crashes locally.
Comment 19 Aakash Jain 2019-10-27 15:22:25 PDT
(In reply to Wenson Hsieh from comment #18)
> Comment on attachment 381991 [details]
> Rename a variable
> 
> > Found 2 new test failures: fast/scrolling/ios/reveal-focused-element-right-above-keyboard-on-ipad.html, fast/scrolling/ios/frame-scrolling-no.html (failure)23 analyze-layout-tests-results
> 
> I wasn’t able to reproduce these crashes locally.
I retried the build, and the tests passed this time.
Comment 20 Tim Horton 2019-10-28 12:11:37 PDT
Comment on attachment 381991 [details]
Rename a variable

View in context: https://bugs.webkit.org/attachment.cgi?id=381991&action=review

> Source/WebKit/ChangeLog:31
> +        While this may potentially leads to an additional synchronous position information request when tapping at the
> +        same location after focusing an element, this is very difficult to achieve in practice, since the tap location
> +        would need to be _exactly_ at the same location.

As mentioned on IRC, there is a solution that avoids this pitfall
Comment 21 Wenson Hsieh 2019-10-28 12:30:26 PDT
(In reply to Tim Horton from comment #20)
> Comment on attachment 381991 [details]
> Rename a variable
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=381991&action=review
> 
> > Source/WebKit/ChangeLog:31
> > +        While this may potentially leads to an additional synchronous position information request when tapping at the
> > +        same location after focusing an element, this is very difficult to achieve in practice, since the tap location
> > +        would need to be _exactly_ at the same location.
> 
> As mentioned on IRC, there is a solution that avoids this pitfall

Yep! Investigating it here: https://bugs.webkit.org/show_bug.cgi?id=203498

> IRC

😢
Comment 22 WebKit Commit Bot 2019-10-28 12:58:09 PDT
Comment on attachment 381991 [details]
Rename a variable

Clearing flags on attachment: 381991

Committed r251665: <https://trac.webkit.org/changeset/251665>
Comment 23 WebKit Commit Bot 2019-10-28 12:58:11 PDT
All reviewed patches have been landed.  Closing bug.