Created attachment 360143[details]
Archive of layout-test-results from ews103 for mac-highsierra
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103 Port: mac-highsierra Platform: Mac OS X 10.13.6
Created attachment 360145[details]
Archive of layout-test-results from ews107 for mac-highsierra-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
Comment on attachment 360133[details]
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=360133&action=review> LayoutTests/editing/pasteboard/smart-paste-001.html:49
> + if (UIHelper.isIOS())
> + getSelection().setBaseAndExtent(test.firstChild, 1, test.firstChild, 5);
> + else
> + doubleClickAtSelectionStart();
We shouldn't have this kind of platform-branches in tests.
We should abstract UIHelper.doubleTapAt so that there is a single UIHelper function like
UIHelper.selectWordBoundaryByActivationAt(x, y)
An alternative approach is to use getSelection().modify('extend', 'forward', 'wordboundary')
r- because this makes all these tests behave differently in iOS compared to other platforms.
To put money where my mouth is, I have a test code change which makes it work across platforms. Give me another half an hour to clean things up & post the change.
Created attachment 360197[details]
With Mac rebaselines
Because of updates to the test, we'd have to rebaselien some results in macOS as well.
Perhaps we should update the tests in a separate patch.
Created attachment 360211[details]
Archive of layout-test-results from ews102 for mac-highsierra
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102 Port: mac-highsierra Platform: Mac OS X 10.13.6
Comment on attachment 360201[details]
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=360201&action=review
r=me, provided EWS is happy. It looks like either macOS needs rebaselined expectations, or the tests need to be tweaked in such a way that they fulfill existing expectations.
> Source/WebCore/ChangeLog:8
> + Turned on a modified tests:
Perhaps something like, "Enabled and modified existing tests:"?
> LayoutTests/editing/pasteboard/smart-paste-008.html:16
> + await UIHelper.selectWordByDoubleTapOrClick(document.getElementById('test'));
Nit - UIHelper.selectWordByDoubleTapOrClick(e);
> LayoutTests/platform/ios/TestExpectations:2140
> +
Nit - stray newline.
Created attachment 360214[details]
Archive of layout-test-results from ews112 for mac-highsierra
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112 Port: mac-highsierra Platform: Mac OS X 10.13.6
Created attachment 360215[details]
Archive of layout-test-results from ews122 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6
Created attachment 360218[details]
Archive of layout-test-results from ews104 for mac-highsierra-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
Comment on attachment 360201[details]
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=360201&action=review
ue
> LayoutTests/resources/ui-helper.js:124
> + await UIHelper.ensurePresentationUpdate();
I found this line very confusing, because I couldn't think of any reason why it should be necessarily to wait for a layer tree commit before continuing the test. But in both my (and Ryosuke's) testing, removing this causes these tests to be flaky! So I looked into this some more. In the case where the test fails after removing the call to UIHelper.ensurePresentationUpdate(), I observed the following sequence of events:
1. HID events for double tap are synthesized and dispatched to the UIApp.
2. The text interaction assistant's double tap gesture recognizer is fired, and it tells WKContentView to -changeSelectionWithGestureAt:…:, which then sends an IPC message to the web process.
3. The web process receives this message, selects the word at the location, and sends an IPC responds back to the UI process.
So far, this is pretty normal. Now here's where things get interesting...
4. When the UI process receives the IPC message (WebPageProxy::gestureCallback), it calls into UIKit, which then calls back into us, asking for an autocorrection context (-[WKContentView requestAutocorrectionContextWithCompletionHandler:]).
5. -[WKContentView requestAutocorrectionContextWithCompletionHandler:] actually blocks on sync IPC back into the web process by calling WebPageProxy::getAutocorrectionContext. While this is happening, UIKit blocks the main thread of the UI process.
6. While the UI process is blocked, let's suppose the promise returned by UIHelper.selectWordByDoubleTapOrClick is now resolved. We'll then attempt to execute "copy" and then "paste" while the UI process is still blocked on sync IPC (!)
7. Since the UI process is waiting on sync IPC, we'll resolve any sync IPC messages we get from the web process before we try and respond to non-blocking IPC messages in order to avoid a permanent deadlock.
8. Since "paste" sends sync IPC while "copy" just sends a regular IPC message, this means the UI process will handle the IPC to paste *before* it gets the IPC message to copy! This, of course, causes the test to fail because we'll read from the pasteboard before we've written to it.
A few more remarks:
- Without the ensurePresentationUpdate(), this test is flaky; in the case where it passes, the promise in step (6) is resolved after the sync IPC call for the autocorrection context.
- The FIXME in -requestAutocorrectionContextWithCompletionHandler: (<rdar://problem/16207002>) tracks making the autocorrection context request async.
- This also means the ensurePresentationUpdate() isn't actually necessary; we just need to make sure that the sync message for the autocorrection context is finished by the time we try and continue the test. This should be doable by ensuring a round trip to the UI process and back (rather than requiring layout and a compositing flush). Something like this: `await new Promise(resolve => testRunner.runUIScript("uiController.uiScriptComplete()", resolve))`.
Created attachment 360377[details]
Archive of layout-test-results from ews106 for mac-highsierra-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
Created attachment 360453[details]
Archive of layout-test-results from ews126 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6
Created attachment 360822[details]
Archive of layout-test-results from ews123 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6
Created attachment 360943[details]
Archive of layout-test-results from ews123 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6
Created attachment 360958[details]
Archive of layout-test-results from ews124 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6
It appears that the changes in https://trac.webkit.org/changeset/240902/webkit
Has caused one API failure on iOS.
Failed
TestWebKitAPI.WKAttachmentTests.InsertAndRemoveDuplicateAttachment
2019-02-03 02:19:51.653 TestWebKitAPI[81810:95344208] Expected removed attachments: (
) to match (
"<_WKAttachment 0x7fa71030c230 id='c098c0a4-33a3-431a-b4da-317bda9d4003'>"
).
/Volumes/Data/slave/ios-simulator-12-release/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm:145
Value of: removedAttachmentsMatch
Actual: false
Expected: true
(In reply to Truitt Savell from comment #45)
> It appears that the changes in
> https://trac.webkit.org/changeset/240902/webkit
>
> Has caused one API failure on iOS.
>
>
> Failed
>
> TestWebKitAPI.WKAttachmentTests.InsertAndRemoveDuplicateAttachment
> 2019-02-03 02:19:51.653 TestWebKitAPI[81810:95344208] Expected
> removed attachments: (
> ) to match (
> "<_WKAttachment 0x7fa71030c230
> id='c098c0a4-33a3-431a-b4da-317bda9d4003'>"
> ).
>
>
> /Volumes/Data/slave/ios-simulator-12-release/build/Tools/TestWebKitAPI/Tests/
> WebKitCocoa/WKAttachmentTests.mm:145
> Value of: removedAttachmentsMatch
> Actual: false
> Expected: true
I have a fix for this in https://bugs.webkit.org/show_bug.cgi?id=194207
2019-01-25 11:26 PST, Megan Gardner
2019-01-25 12:08 PST, EWS Watchlist
2019-01-25 12:34 PST, EWS Watchlist
2019-01-25 16:33 PST, Ryosuke Niwa
2019-01-25 17:10 PST, Ryosuke Niwa
2019-01-25 18:08 PST, Megan Gardner
2019-01-25 19:09 PST, EWS Watchlist
2019-01-25 19:51 PST, EWS Watchlist
2019-01-25 19:59 PST, EWS Watchlist
2019-01-25 20:25 PST, EWS Watchlist
2019-01-28 13:38 PST, Megan Gardner
2019-01-28 13:44 PST, Megan Gardner
2019-01-28 14:43 PST, EWS Watchlist
2019-01-28 14:55 PST, Megan Gardner
2019-01-29 01:29 PST, EWS Watchlist
2019-01-31 18:30 PST, Megan Gardner
2019-01-31 20:32 PST, EWS Watchlist
2019-02-01 15:49 PST, Megan Gardner
2019-02-01 17:58 PST, Megan Gardner
2019-02-01 18:26 PST, EWS Watchlist
2019-02-01 21:15 PST, EWS Watchlist
2019-02-02 23:28 PST, Megan Gardner
2019-02-03 00:27 PST, Megan Gardner