Bug 148963 – Implement EventSenderProxy for WebKitTestRunner on iOS

• Component: Tools / Tests
• Status: RESOLVED INVALID

[Wenson Hsieh]

The methods in EventSenderProxyIOS.mm are currently unimplemented, preventing us from testing iOS using JavaScript via window.eventSender.

[Wenson Hsieh]

Note: This will also require some (unobtrusive) changes to UIKit.

[Wenson Hsieh]

Created attachment 260776 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/22615821>

[Wenson Hsieh]

WebFrame::fromCoreFrame(*frameRespondingToClick)->firstLayerTreeTransactionIDAfterDidCommitLoad() when it is called in the body's onload handler. This currently causes tests that tap elements immediately after onload to fail.

I'm investigating why this is the case, and how we can fix it.

[Wenson Hsieh]

(In reply to comment #4)
> WebFrame::fromCoreFrame(*frameRespondingToClick)-
> >firstLayerTreeTransactionIDAfterDidCommitLoad() when it is called in the
> body's onload handler. This currently causes tests that tap elements
> immediately after onload to fail.
> 
> I'm investigating why this is the case, and how we can fix it.

(the comment on the top was cut off. Here's what I meant to write):

The current issue is that WebPage::commitPotentialTap hits an early return due to lastLayerTreeTransactionId < WebFrame::fromCoreFrame(*frameRespondingToClick)->firstLayerTreeTransactionIDAfterDidCommitLoad() when it is called in the body's onload handler. This currently causes tests that tap elements immediately after onload to fail.

I'm investigating why this is the case, and how we can fix it.

[Alexey Proskuryakov]

Comment on attachment 260776 [details]
Patch

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

> LayoutTests/fast/forms/disabled-search-input.html:46
> -    }, 0);
> +    }, 500);

This is not right. Nothing in regression tests is guaranteed to happen within 500 ms - there are many processes competing for the CPU, and with hyperthreading, some of the virtual cores can be very slow. Furthermore, CPU load increases even further when there is a crash, or even media playing.

> LayoutTests/platform/ios-simulator/TestExpectations:1767
> -fast/forms/disabled-search-input.html [ Failure ]
> +fast/forms/disabled-search-input.html [ Pass ]

We now need separate results per iOS version, as we can't get this pass without UIKit changes.

[Wenson Hsieh]

Comment on attachment 260776 [details]
Patch

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

>> LayoutTests/fast/forms/disabled-search-input.html:46
>> +    }, 500);
> 
> This is not right. Nothing in regression tests is guaranteed to happen within 500 ms - there are many processes competing for the CPU, and with hyperthreading, some of the virtual cores can be very slow. Furthermore, CPU load increases even further when there is a crash, or even media playing.

Sorry about that -- I don't intend on landing with this timeout here. There's one last major issue I've been investigating regarding WKTR, which is that sending a tap immediately on body load by calling eventSender.mouseDown, eventSender.mouseUp causes us to hit an early return and ignore the tap. I added the timeout here for now just to circumvent this and fix the rest of the issues with eventSender. When I figure out how to best address this, the timeout will not be necessary.

[Wenson Hsieh]

Created attachment 260937 [details]
Patch

[Wenson Hsieh]

Created attachment 260938 [details]
Patch

[Wenson Hsieh]

I addressed the early return in commitPotentialTap by forcing a paint and spinning until the paint finishes the first time we try to interact with the page.

[Wenson Hsieh]

It seems the repainting strategy only works when the test uses eventSender in a setTimeout after the body loads. Investigating further into this...

[Wenson Hsieh]

Created attachment 261024 [details]
Patch

[Wenson Hsieh]

The latest patches addresses the above problem. As long as eventSender is used after the body loads (which it should be for these tests), no more flakiness is observed. To do this, I block the web process from immediately synchronously messaging the UI process in EventSendingController until the web process has received a 'DidUpdate' message from the UI process' drawing area proxy indicating that it is safe to force a compositing layer flush right away. This allows us to start sending touch events in the onload handler of the body element rather than calling eventSender through a setTimeout.

Other outstanding issues:
- EventSender is currently unable to type more than a couple characters before text input starts becoming flaky.
- The testing harness occasionally crashes in Springboard while running the tests, but I'm not currently sure if that's due to my patch or if it's a preexisting issue.
- Still need to find a way to correctly map document coords to screen coords so that we can send touch events when the document is zoomed.
- Clean up the key window context ID hack in EventSenderProxy::sendIOHIDEvent which I'm using to make touch events hit-test successfully in UIKit. Namely, figure out why the application's key window appears to be nil when I try to access it.

[Wenson Hsieh]

Created attachment 261069 [details]
Patch

[Wenson Hsieh]

Created attachment 261079 [details]
Patch

[Darin Adler]

Comment on attachment 261079 [details]
Patch

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

> Source/WebKit2/UIProcess/WebPageProxy.h:1082
> +    void sendForceRepaintWithCallback(RefPtr<VoidCallback>);

Argument types should almost never be RefPtr. Please consider a plain reference or pointer or RefPtr&& or Ref&&. Details at <http://www.webkit.org/coding/RefPtr.html>.

[Wenson Hsieh]

(In reply to comment #16)
> Comment on attachment 261079 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=261079&action=review
> 
> > Source/WebKit2/UIProcess/WebPageProxy.h:1082
> > +    void sendForceRepaintWithCallback(RefPtr<VoidCallback>);
> 
> Argument types should almost never be RefPtr. Please consider a plain
> reference or pointer or RefPtr&& or Ref&&. Details at
> <http://www.webkit.org/coding/RefPtr.html>.

Got it -- I'll change this. Thanks for the link!

[Wenson Hsieh]

Created attachment 261580 [details]
Patch

[Wenson Hsieh]

Created attachment 261582 [details]
Patch

[Wenson Hsieh]

This patch makes it unnecessary to expose the size of the HID event queue in UIKit by adding WK API functions that allow us to wait until WebKit receives touch and key events. This also solves the problem of key events flaking when sending more than a few at a time, which is a big plus.

After r189959, the weird key window context id hack I was using to make touch events hit-test properly is also no longer needed. Thanks Simon!

I'm in the process of landing the necessary UIKeyboardImpl changes in UIKit, which will need to happen before this patch can land.

[Enrica Casucci]

I don't think it is necessary to expose the ability to show the keyboard on programmatic focus via the eventSendingController.
This should be a preference set at the level of the TestController like many other ones (see resetPreferencesToConsistentValues in TestController.cpp).
This was you don't need to modify the actual tests and simplifies your patch.

[Wenson Hsieh]

Good point -- this should also make it not necessary to plumb the focus enabling message from the web process to UI process.

My only concern is that we may want to write platform iOS tests in the future where we test that the keyboard does not show when an input has autofocus or the focus() method is programmatically called. However, these tests should be much more rare than existing tests that expect the keyboard to come up during programmatic focus, so it probably makes sense to make the TestController allow programmatic focus to assist the keyboard by preference.

[Wenson Hsieh]

Closing as invalid, since we've switched over to UI-side-script-driven testing instead of the existing synchronous model.

[Csaba Osztrogonác]

Comment on attachment 261582 [details]
Patch

Cleared review? from attachment 261582 [details] so that this bug does not appear in http://webkit.org/pending-review.  If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).