Bug 194076 – Enable swipe tests on iOS

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Ryosuke Niwa]

Tests in LayoutTests/swipe are currently skipped on iOS. Enable all six of them.

[Ryosuke Niwa]

Created attachment 360688 [details]
Enables the tests

[Geoffrey Garen]

Comment on attachment 360688 [details]
Enables the tests

r=me

[Tim Horton]

Comment on attachment 360688 [details]
Enables the tests

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

> LayoutTests/platform/ios/swipe/main-frame-pinning-requirement-expected.txt:2
> +Failure. Should never begin a swipe, because we were in the middle of a scrolling gesture that started when the main frame was not pinned to the left.

This test doesn't make sense on iOS because we don't require the main frame to be pinned to swipe... but maybe it's OK to turn it on with the expected failure with the understanding that it's truly an *expected* failure?

[Ryosuke Niwa]

(In reply to Tim Horton from comment #3)
> Comment on attachment 360688 [details]
> Enables the tests
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=360688&action=review
> 
> > LayoutTests/platform/ios/swipe/main-frame-pinning-requirement-expected.txt:2
> > +Failure. Should never begin a swipe, because we were in the middle of a scrolling gesture that started when the main frame was not pinned to the left.
> 
> This test doesn't make sense on iOS because we don't require the main frame
> to be pinned to swipe... but maybe it's OK to turn it on with the expected
> failure with the understanding that it's truly an *expected* failure?

Yeah, I'm simply turning on the tests here. This is definitely not expected on iOS as you say. Having a failing test is better than no tests :)

[Ryosuke Niwa]

I'd mention that WebKitTestRunner doesn't seem to support back-forward swipe gesture even when we turn it on via testRunner.setNavigationGesturesEnabled(true). I can't manually trigger back-forward navigation via swiping for example.

[Ryosuke Niwa]

Comment on attachment 360688 [details]
Enables the tests

Clearing flags on attachment: 360688

Committed r240765: <https://trac.webkit.org/changeset/240765>

[Ryosuke Niwa]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/47694943>

[Simon Fraser (smfr)]

Comment on attachment 360688 [details]
Enables the tests

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

> Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm:-827
>      TestRunnerWKWebView *webView = TestController::singleton().mainWebView()->platformView();
>      [webView _beginBackSwipeForTesting];
> -
> -    unsigned callbackID = m_context->prepareForAsyncTask(callback, CallbackTypeNonPersistent);
> -    m_context->asyncTaskComplete(callbackID);

Is beginBackSwipe async or not? Who calls the callback?

> Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm:827
>  void UIScriptController::completeBackSwipe(JSValueRef callback)

Odd that the API is not just "doBackswipe" with an async completion handler.

[Ryosuke Niwa]

(In reply to Simon Fraser (smfr) from comment #9)
> Comment on attachment 360688 [details]
> Enables the tests
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=360688&action=review
> 
> > Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm:-827
> >      TestRunnerWKWebView *webView = TestController::singleton().mainWebView()->platformView();
> >      [webView _beginBackSwipeForTesting];
> > -
> > -    unsigned callbackID = m_context->prepareForAsyncTask(callback, CallbackTypeNonPersistent);
> > -    m_context->asyncTaskComplete(callbackID);
> 
> Is beginBackSwipe async or not? Who calls the callback?

It's a sync test API.

> > Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm:827
> >  void UIScriptController::completeBackSwipe(JSValueRef callback)
> 
> Odd that the API is not just "doBackswipe" with an async completion handler.

Yeah.

[Tim Horton]

(In reply to Ryosuke Niwa from comment #10)
> (In reply to Simon Fraser (smfr) from comment #9)
> > Comment on attachment 360688 [details]
> > Enables the tests
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=360688&action=review
> > 
> > > Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm:-827
> > >      TestRunnerWKWebView *webView = TestController::singleton().mainWebView()->platformView();
> > >      [webView _beginBackSwipeForTesting];
> > > -
> > > -    unsigned callbackID = m_context->prepareForAsyncTask(callback, CallbackTypeNonPersistent);
> > > -    m_context->asyncTaskComplete(callbackID);
> > 
> > Is beginBackSwipe async or not? Who calls the callback?
> 
> It's a sync test API.
> 
> > > Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm:827
> > >  void UIScriptController::completeBackSwipe(JSValueRef callback)
> > 
> > Odd that the API is not just "doBackswipe" with an async completion handler.
> 
> Yeah.

Don't some of the tests depend on being able to do things in the middle?

[Ryosuke Niwa]

(In reply to Tim Horton from comment #11)
> (In reply to Ryosuke Niwa from comment #10)
> > (In reply to Simon Fraser (smfr) from comment #9)
> > > Comment on attachment 360688 [details]
> > > Enables the tests
> > > 
> > > View in context:
> > > https://bugs.webkit.org/attachment.cgi?id=360688&action=review
> > > 
> > > > Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm:-827
> > > >      TestRunnerWKWebView *webView = TestController::singleton().mainWebView()->platformView();
> > > >      [webView _beginBackSwipeForTesting];
> > > > -
> > > > -    unsigned callbackID = m_context->prepareForAsyncTask(callback, CallbackTypeNonPersistent);
> > > > -    m_context->asyncTaskComplete(callbackID);
> > > 
> > > Is beginBackSwipe async or not? Who calls the callback?
> > 
> > It's a sync test API.
> > 
> > > > Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm:827
> > > >  void UIScriptController::completeBackSwipe(JSValueRef callback)
> > > 
> > > Odd that the API is not just "doBackswipe" with an async completion handler.
> > 
> > Yeah.
> 
> Don't some of the tests depend on being able to do things in the middle?

They do but you'd think we can have a higher level API which does the whole swipe too.

[Tim Horton]

> They do but you'd think we can have a higher level API which does the whole
> swipe too.

Ah, yes, sure.