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: 2021-07-26 13:11 PDT (History)
12 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.
Comment 24 ayumi_kojima 2021-07-26 12:05:32 PDT
For 

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


iPhone debug is a flaky failure. Update expectations https://trac.webkit.org/changeset/280305/webkit
Comment 25 Wenson Hsieh 2021-07-26 12:11:36 PDT
(In reply to ayumi_kojima from comment #24)
> For 
> 
> editing/pasteboard/smart-paste-paragraph-002.html
> editing/pasteboard/smart-paste-paragraph-003.html
> editing/pasteboard/smart-paste-paragraph-004.html
> 
> 
> iPhone debug is a flaky failure. Update expectations
> https://trac.webkit.org/changeset/280305/webkit

This Bugzilla bug is pretty old (from 2019), and the radar is closed, indicating that the failures from back then were resolved by this change.

If these tests are starting to fail again, I suspect it might be due to a different root cause.
Comment 26 ayumi_kojima 2021-07-26 13:11:16 PDT
(In reply to Wenson Hsieh from comment #25)
> (In reply to ayumi_kojima from comment #24)
> > For 
> > 
> > editing/pasteboard/smart-paste-paragraph-002.html
> > editing/pasteboard/smart-paste-paragraph-003.html
> > editing/pasteboard/smart-paste-paragraph-004.html
> > 
> > 
> > iPhone debug is a flaky failure. Update expectations
> > https://trac.webkit.org/changeset/280305/webkit
> 
> This Bugzilla bug is pretty old (from 2019), and the radar is closed,
> indicating that the failures from back then were resolved by this change.
> 
> If these tests are starting to fail again, I suspect it might be due to a
> different root cause.

New bug is filed here: https://bugs.webkit.org/show_bug.cgi?id=228285