Bug 192165

<rdar://problem/46346682>

I suspect this also accounts for at least one of the API test failures seen on the bots after r238635: https://build.webkit.org/builders/Apple%20iOS%2012%20Simulator%20Debug%20WK2%20(Tests)/builds/1068/steps/run-api-tests/logs/stdio

(In reply to Wenson Hsieh from comment #2) > I suspect this also accounts for at least one of the API test failures seen > on the bots after r238635: > > https://build.webkit.org/builders/ > Apple%20iOS%2012%20Simulator%20Debug%20WK2%20(Tests)/builds/1068/steps/run- > api-tests/logs/stdio This codepath is exercised by these following tests, which started timing out: TestWebKitAPI.DragAndDropTests.ContentEditableToTextarea TestWebKitAPI.DragAndDropTests.TextAreaToInput TestWebKitAPI.DragAndDropTests.ContentEditableToContentEditable TestWebKitAPI.DragAndDropTests.SinglePlainTextWordTypeIdentifiers TestWebKitAPI.DragAndDropTests.ContentEditableMoveParagraphs TestWebKitAPI.DragAndDropTests.SinglePlainTextURLTypeIdentifiers However, even after fixing the bug, the tests still time out! This needs further investigation...

Created attachment 356106 [details] Fixes all 11 API tests

*** Bug 192178 has been marked as a duplicate of this bug. ***

Created attachment 356107 [details] Fix open source build

Comment on attachment 356107 [details] Fix open source build View in context: https://bugs.webkit.org/attachment.cgi?id=356107&action=review How do we handle the case where the drag is cancelled? Does _conclude* get called with the source WebView such that we make it first responder agin. Now that we have balanced resigning and making first responder for drag and drop do we still need the concept of “retaining active focus state” concept? > Source/WebKit/UIProcess/ios/PageClientImplIOS.mm:169 > + if ([m_webView _isRetainingActiveFocusedState]) The checking of this condition first makes it asymmetric to where we check it in isViewWindowActive(). It would be good to either fix this code or fix isViewWindowActice() such that they have symmetry. If we choose to keep the order the way it is here then I would suggest flattening this if statement into a disjunction that is part of the return statement to avoid a branch (though the compiler is likely smart enough to do this for us :/). On another note and one that can be addressed in another patch, I find this “retaining active focused state” concept mysterious. If this is only used for drag and drop then it would be good to rename this to reflect that. Otherwise, I suggest we add a comment to provide examples of when this state is set. > Tools/TestWebKitAPI/ios/DragAndDropSimulatorIOS.mm:467 > + [_webView becomeFirstResponder]; I am assuming _webView is the drop destination. > Tools/TestWebKitAPI/ios/DragAndDropSimulatorIOS.mm:555 > + [_webView resignFirstResponder]; I am assuming _webView is the source WebView.

Comment on attachment 356107 [details] Fix open source build View in context: https://bugs.webkit.org/attachment.cgi?id=356107&action=review >> Source/WebKit/UIProcess/ios/PageClientImplIOS.mm:169 >> + if ([m_webView _isRetainingActiveFocusedState]) > > The checking of this condition first makes it asymmetric to where we check it in isViewWindowActive(). It would be good to either fix this code or fix isViewWindowActice() such that they have symmetry. If we choose to keep the order the way it is here then I would suggest flattening this if statement into a disjunction that is part of the return statement to avoid a branch (though the compiler is likely smart enough to do this for us :/). On another note and one that can be addressed in another patch, I find this “retaining active focused state” concept mysterious. If this is only used for drag and drop then it would be good to rename this to reflect that. Otherwise, I suggest we add a comment to provide examples of when this state is set. Ok! I'll make this symmetric with isViewWindowActive() by changing to: return (isViewInWindow() && ![m_webView _isBackground] && [m_webView _contentViewIsFirstResponder]) || [m_webView _isRetainingActiveFocusedState]; As for “retaining active focused state”, this is SPI on WKWebView and used by various internal clients in two ways: UITextInputMultiDocument (a private UIKit protocol which the content view implements) and -[WKWebView _retainActiveFocusedState] (which is just WebKit SPI). >> Tools/TestWebKitAPI/ios/DragAndDropSimulatorIOS.mm:467 >> + [_webView becomeFirstResponder]; > > I am assuming _webView is the drop destination. This is actually meant to be the source web view (this "reset" call resets the "preserve" from earlier). That being said, DragAndDropSimulator doesn't support testing drag and drop with more than one web view, there really isn't a distinction — the source and destination are always the same view. >> Tools/TestWebKitAPI/ios/DragAndDropSimulatorIOS.mm:555 >> + [_webView resignFirstResponder]; > > I am assuming _webView is the source WebView. Yes.

(In reply to Daniel Bates from comment #7) > Comment on attachment 356107 [details] > Fix open source build > > View in context: > https://bugs.webkit.org/attachment.cgi?id=356107&action=review > > How do we handle the case where the drag is cancelled? Does _conclude* get > called with the source WebView such that we make it first responder again? If a drag is cancelled, UIKit should tell the source to stop preserving focus. > Now that we have balanced resigning and making first responder for drag and > drop do we still need the concept of “retaining active focus state” concept?

Created attachment 356132 [details] Patch for landing

The commit-queue encountered the following flaky tests while processing attachment 356132 [details]: webgl/1.0.2/conformance/more/functions/drawArraysOutOfBounds.html bug 192219 (author: roger_fong@apple.com) webgl/1.0.2/conformance/more/functions/uniformfArrayLen1.html bug 192220 (author: roger_fong@apple.com) The commit-queue is continuing to process your patch.

The commit-queue encountered the following flaky tests while processing attachment 356132 [details]: inspector/dom-debugger/event-animation-frame-breakpoints.html bug 192221 (author: drousso@apple.com) The commit-queue is continuing to process your patch.

The commit-queue encountered the following flaky tests while processing attachment 356132 [details]: http/wpt/css/css-animations/start-animation-001.html bug 190903 (authors: dino@apple.com, fred.wang@free.fr, and graouts@apple.com) The commit-queue is continuing to process your patch.

The commit-queue encountered the following flaky tests while processing attachment 356132 [details]: workers/bomb.html bug 171985 (author: fpizlo@apple.com) The commit-queue is continuing to process your patch.

Comment on attachment 356132 [details] Patch for landing Clearing flags on attachment: 356132 Committed r238728: <https://trac.webkit.org/changeset/238728>