Bug 203247 – [iOS 13] imported/mozilla/svg/text/textpath-selection.svg is flaky

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Wenson Hsieh]

<rdar://problem/52124292>

[Wenson Hsieh]

Created attachment 381547 [details]
Repro attempt #1

[Wenson Hsieh]

> 09:47:11.316 70031 worker/4 imported/w3c/web-platform-tests/IndexedDB/idbfactory-deleteDatabase-opaque-origin.html passed
> 09:47:11.363 68447 [27899/47157] imported/mozilla/svg/text/textpath-selection-305.svg failed unexpectedly (reference mismatch)
> 09:47:11.361 70023 worker/3 imported/mozilla/svg/text/textpath-selection-305.svg failed:
> 09:47:11.362 70023 worker/3  reference mismatch

😲

[Wenson Hsieh]

> failed 1/500 times

this is going to be rough :/

[Wenson Hsieh]

Created attachment 381603 [details]
Repro attempt #2

[Wenson Hsieh]

(In reply to Wenson Hsieh from comment #4)
> Created attachment 381603 [details]
> Repro attempt #2

Failures: 0/1000

This is going to take a few more tries.

[Wenson Hsieh]

Created attachment 381635 [details]
Repro attempt #3

[Wenson Hsieh]

Created attachment 381636 [details]
Repro attempt #3

[Wenson Hsieh]

Created attachment 381653 [details]
Attempt #4

[Wenson Hsieh]

Created attachment 381654 [details]
Attempt #4 (2)

[Wenson Hsieh]

Created attachment 381655 [details]
Attempt #4 (3)

[Wenson Hsieh]

Created attachment 381656 [details]
Attempt #4 (4)

[Wenson Hsieh]

Created attachment 381657 [details]
Attempt #4 (5)

[Wenson Hsieh]

Here’s the smoking gun (finally, after 3000+ attempts!)

09:17:16.341 74194 worker/4 imported/mozilla/svg/text/textpath-selection-35.svg output stderr lines:
09:17:16.341 74194   2019-10-23 09:17:15.873 WebKitTestRunnerApp[75982:105086234] <WK>: 
09:17:16.341 74194   2019-10-23 09:17:15.873 WebKitTestRunnerApp[75982:105086234] <WK>: *** TAKING SNAPSHOT FOR /Volumes/Data/worker/iOS-13-Simulator-WK2-Tests-EWS/build/LayoutTests/imported/mozilla/svg/text/textpath-selection-34-expected.svg ***
09:17:16.341 74194   2019-10-23 09:17:15.874 WebKitTestRunnerApp[75982:105086234] <WK>: The animations on <UITextSelectionView: 0x7f8e43602280; frame = (0 0; 800 600); userInteractionEnabled = NO; layer = <CALayer: 0x60000209d000>> are: []
09:17:16.342 74194   2019-10-23 09:17:15.874 WebKitTestRunnerApp[75982:105086234] <WK>: 
09:17:16.342 74194 worker/4 imported/mozilla/svg/text/textpath-selection-35-expected.svg output stderr lines:
09:17:16.342 74194   2019-10-23 09:17:16.033 WebKitTestRunnerApp[75982:105086234] <WK>: 
09:17:16.342 74194   2019-10-23 09:17:16.033 WebKitTestRunnerApp[75982:105086234] <WK>: *** TAKING SNAPSHOT FOR /Volumes/Data/worker/iOS-13-Simulator-WK2-Tests-EWS/build/LayoutTests/imported/mozilla/svg/text/textpath-selection-35.svg ***
09:17:16.342 74194   2019-10-23 09:17:16.033 WebKitTestRunnerApp[75982:105086234] <WK>: The animations on <UITextSelectionView: 0x7f8e43602280; frame = (0 0; 800 600); userInteractionEnabled = NO; layer = <CALayer: 0x60000209d000>> are: []
09:17:16.342 74194   2019-10-23 09:17:16.034 WebKitTestRunnerApp[75982:105086234] <WK>: The animations on <UITextRangeView: 0x7f8e5580c540; frame = (0 0; 0 0); opaque = NO; layer = <CALayer: 0x6000020cd9e0>> are: []
09:17:16.342 74194   2019-10-23 09:17:16.034 WebKitTestRunnerApp[75982:105086234] <WK>: The animations on <UIView: 0x7f8e5580ab80; frame = (0 0; 0 0); opaque = NO; userInteractionEnabled = NO; layer = <CALayer: 0x6000020ce920>> are: []
09:17:16.342 74194   2019-10-23 09:17:16.035 WebKitTestRunnerApp[75982:105086234] <WK>: The animations on <UIView: 0x7f8e55b047d0; frame = (117 77; 57 136); userInteractionEnabled = NO; layer = <CALayer: 0x6000020a3720>> are: []
09:17:16.342 74194   2019-10-23 09:17:16.035 WebKitTestRunnerApp[75982:105086234] <WK>: The animations on <UIView: 0x7f8e55b01600; frame = (19 77; 117 31); hidden = YES; userInteractionEnabled = NO; layer = <CALayer: 0x6000020a3b00>> are: []
09:17:16.342 74194   2019-10-23 09:17:16.035 WebKitTestRunnerApp[75982:105086234] <WK>: The animations on <UIView: 0x7f8e55b012e0; frame = (142 195; 32 18); hidden = YES; userInteractionEnabled = NO; layer = <CALayer: 0x6000020a3640>> are: []
09:17:16.342 74194   2019-10-23 09:17:16.036 WebKitTestRunnerApp[75982:105086234] <WK>: The animations on <UISelectionGrabber: 0x7f8e5370a400; frame = (116.5 77; 2 136); opaque = NO; layer = <CALayer: 0x6000020d65e0>> are: []
09:17:16.342 74194   2019-10-23 09:17:16.036 WebKitTestRunnerApp[75982:105086234] <WK>: The animations on <UISelectionGrabber: 0x7f8e53709c90; frame = (172.5 77; 2 136); opaque = NO; layer = <CALayer: 0x6000020d66a0>> are: []
09:17:16.342 74194   2019-10-23 09:17:16.036 WebKitTestRunnerApp[75982:105086234] <WK>: 
09:17:16.887 72611 [26152/48156] imported/mozilla/svg/text/textpath-selection-35.svg failed unexpectedly (reference mismatch)
09:17:16.886 74194 worker/4 imported/mozilla/svg/text/textpath-selection-35.svg failed:

…it normally looks like this in the passing case:

09:17:17.236 74194 worker/4 imported/mozilla/svg/text/textpath-selection-36.svg output stderr lines:
09:17:17.236 74194   2019-10-23 09:17:16.238 WebKitTestRunnerApp[75982:105086234] <WK>: 
09:17:17.236 74194   2019-10-23 09:17:16.239 WebKitTestRunnerApp[75982:105086234] <WK>: *** TAKING SNAPSHOT FOR /Volumes/Data/worker/iOS-13-Simulator-WK2-Tests-EWS/build/LayoutTests/imported/mozilla/svg/text/textpath-selection-35-expected.svg ***
09:17:17.236 74194   2019-10-23 09:17:16.239 WebKitTestRunnerApp[75982:105086234] <WK>: The animations on <UITextSelectionView: 0x7f8e43602280; frame = (0 0; 800 600); userInteractionEnabled = NO; layer = <CALayer: 0x60000209d000>> are: []
09:17:17.236 74194   2019-10-23 09:17:16.239 WebKitTestRunnerApp[75982:105086234] <WK>: 
09:17:17.237 74194 worker/4 imported/mozilla/svg/text/textpath-selection-36-expected.svg output stderr lines:
09:17:17.237 74194   2019-10-23 09:17:16.940 WebKitTestRunnerApp[75982:105086234] <WK>: 
09:17:17.237 74194   2019-10-23 09:17:16.940 WebKitTestRunnerApp[75982:105086234] <WK>: *** TAKING SNAPSHOT FOR /Volumes/Data/worker/iOS-13-Simulator-WK2-Tests-EWS/build/LayoutTests/imported/mozilla/svg/text/textpath-selection-36.svg ***
09:17:17.237 74194   2019-10-23 09:17:16.941 WebKitTestRunnerApp[75982:105086234] <WK>: The animations on <UITextSelectionView: 0x7f8e43602280; frame = (0 0; 800 600); userInteractionEnabled = NO; layer = <CALayer: 0x60000209d000>> are: []
09:17:17.237 74194   2019-10-23 09:17:16.941 WebKitTestRunnerApp[75982:105086234] <WK>: 
09:17:17.238 74194 worker/4 imported/mozilla/svg/text/textpath-selection-36.svg passed

Calling off the rest of the queued EWS jobs now, as I continue to investigate.

[Wenson Hsieh]

Created attachment 381689 [details]
actual result

[Wenson Hsieh]

Created attachment 381690 [details]
expected result

[Wenson Hsieh]

Maybe, just maybe we can get away with suppressing native iOS text selection views while taking a snapshot of the web view for the purposes of ref testing.

The timing with which the views are added and animated depend heavily on implementation details in UIKit, and I’m not sure it makes sense to try and make any assumptions here.

We still have a considerable number of layout tests that inspect these native selection views on iOS, but they all work by waiting until the native selection views reach a particular state (e.g. by polling for the number of ranged selection subviews, or the presence of selection handles, or waiting for a caret selection with a given geometry, etc.).

[Wenson Hsieh]

Created attachment 381721 [details]
Patch

[Tim Horton]

Comment on attachment 381721 [details]
Patch

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

> Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:6866
> +    [_contentView _startSuppressingSelectionAssistantForReason:WebKit::HideForTesting];

Can we/should we just hide the view instead of doing this thing that might have additional side effects?

[Alexey Proskuryakov]

Comment on attachment 381721 [details]
Patch

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

> LayoutTests/platform/ios-wk2/TestExpectations:1335
> -webkit.org/b/199196 [ Release ] imported/mozilla/svg/text/textpath-selection.svg [ Pass ImageOnlyFailure ]
>  webkit.org/b/199196 [ Release ] imported/mozilla/svg/text/selectSubString-2.svg [ Pass ImageOnlyFailure ]

I think that <rdar://problem/52124292> tracks both of these.

[Wenson Hsieh]

(In reply to Alexey Proskuryakov from comment #19)
> Comment on attachment 381721 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=381721&action=review
> 
> > LayoutTests/platform/ios-wk2/TestExpectations:1335
> > -webkit.org/b/199196 [ Release ] imported/mozilla/svg/text/textpath-selection.svg [ Pass ImageOnlyFailure ]
> >  webkit.org/b/199196 [ Release ] imported/mozilla/svg/text/selectSubString-2.svg [ Pass ImageOnlyFailure ]
> 
> I think that <rdar://problem/52124292> tracks both of these.

Good catch — I’ll remove these from the TestExpectations as well.

[Wenson Hsieh]

(In reply to Tim Horton from comment #18)
> Comment on attachment 381721 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=381721&action=review
> 
> > Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:6866
> > +    [_contentView _startSuppressingSelectionAssistantForReason:WebKit::HideForTesting];
> 
> Can we/should we just hide the view instead of doing this thing that might
> have additional side effects?

I had considered that, but was afraid that would cause more incompatibility (for instance, there are a few places where UITextSelectionView checks if self.alpha == 0, and makes some decisions based on that).

I don’t see any checks for `self.hidden` though, so maybe this is okay.

[Tim Horton]

(In reply to Wenson Hsieh from comment #21)
> (In reply to Tim Horton from comment #18)
> > Comment on attachment 381721 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=381721&action=review
> > 
> > > Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:6866
> > > +    [_contentView _startSuppressingSelectionAssistantForReason:WebKit::HideForTesting];
> > 
> > Can we/should we just hide the view instead of doing this thing that might
> > have additional side effects?
> 
> I had considered that, but was afraid that would cause more incompatibility
> (for instance, there are a few places where UITextSelectionView checks if
> self.alpha == 0, and makes some decisions based on that).
> 
> I don’t see any checks for `self.hidden` though, so maybe this is okay.

Could be on a container we own, instead?

[Wenson Hsieh]

(In reply to Tim Horton from comment #22)
> (In reply to Wenson Hsieh from comment #21)
> > (In reply to Tim Horton from comment #18)
> > > Comment on attachment 381721 [details]
> > > Patch
> > > 
> > > View in context:
> > > https://bugs.webkit.org/attachment.cgi?id=381721&action=review
> > > 
> > > > Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:6866
> > > > +    [_contentView _startSuppressingSelectionAssistantForReason:WebKit::HideForTesting];
> > > 
> > > Can we/should we just hide the view instead of doing this thing that might
> > > have additional side effects?
> > 
> > I had considered that, but was afraid that would cause more incompatibility
> > (for instance, there are a few places where UITextSelectionView checks if
> > self.alpha == 0, and makes some decisions based on that).
> > 
> > I don’t see any checks for `self.hidden` though, so maybe this is okay.
> 
> Could be on a container we own, instead?

I think the lowest ancestor of UITextSelectionView that we own is the content view itself :/

[Wenson Hsieh]

Created attachment 381754 [details]
Just hide UITextSelectionView.

[WebKit Commit Bot]

Comment on attachment 381754 [details]
Just hide UITextSelectionView.

Clearing flags on attachment: 381754

Committed r251526: <https://trac.webkit.org/changeset/251526>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.