[EFL] REGRESSION(r188828): All performance tests and almost all layout tests crash

• Component: New Bugs
• Status: RESOLVED FIXED

[Csaba Osztrogonác]

https://build.webkit.org/builders/EFL%20Linux%2064-bit%20Release%20WK2%20%28Perf%29/builds/6629

error: Animation/balls.html
1   0x7fa009f39678
2   0x7fa005377eb0
3   0x420214 WTR::PlatformWebView::page()
4   0x4119b4 WTR::TestController::resetPreferencesToConsistentValues()
5   0x419921 WTR::TestController::initialize(int, char const**)
6   0x41a20a WTR::TestController::TestController(int, char const**)
7   0x40d0ab main
8   0x7fa005362ec5 __libc_start_main
9   0x40d107

- last known good revision: https://trac.webkit.org/changeset/188827
(however the testing failed with timeout during the test number 27)

- build failure between: r188828 - r188842

- first failing revision: https://trac.webkit.org/changeset/188843

[Csaba Osztrogonác]

I think https://trac.webkit.org/changeset/188828 might be the culprit,
because it touches PlatformWebView code path in this revision interval.

[Tim Horton]

Seems likely! I will look shortly.

[Tim Horton]

It looks like we don't have a view yet when TestController::initialize tries to set some prefs (via resetPreferencesToConsistentValues).

This is OK on PLATFORM(COCOA) because we keep around a global WKWebViewConfiguration and use that when retrieving the WKPreferences. Maybe someone from EFL or GTK (who can actually test this) can do the same with the WKPageConfiguration?

[Gyuyoung Kim]

(In reply to comment #3)
> It looks like we don't have a view yet when TestController::initialize tries
> to set some prefs (via resetPreferencesToConsistentValues).
> 
> This is OK on PLATFORM(COCOA) because we keep around a global
> WKWebViewConfiguration and use that when retrieving the WKPreferences. Maybe
> someone from EFL or GTK (who can actually test this) can do the same with
> the WKPageConfiguration?

I simply apply below fix in order to run layout test on EFL port.


--- a/Tools/WebKitTestRunner/TestController.cpp
+++ b/Tools/WebKitTestRunner/TestController.cpp
@@ -568,7 +568,11 @@ void TestController::ensureViewSupportsOptionsForTest(const TestInvocation& test
 void TestController::resetPreferencesToConsistentValues()
 {
     // Reset preferences
+#if PLATFORM(COCOA)
     WKPreferencesRef preferences = platformPreferences();
+#else
+    WKPreferencesRef preferences = WKPageGroupGetPreferences(m_pageGroup.get());
+#endif


EFL layout test begins to run again with this fix. However I'm not sure if this is correct fix for EFL and GTK. Tim, what do you think ?

[Gyuyoung Kim]

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

[Gyuyoung Kim]

Created attachment 259840 [details]
Patch

[Gyuyoung Kim]

I hope to fix EFL layout test and performance test as soon as possible. Tim, Ossy , please take a look this patch.

[Anders Carlsson]

Comment on attachment 259840 [details]
Patch

Please implement platformPreferences() in TestControllerEfl.cpp instead of adding more #ifdefs.

[Gyuyoung Kim]

Created attachment 259912 [details]
Patch

[Gyuyoung Kim]

(In reply to comment #8)
> Comment on attachment 259840 [details]
> Patch
> 
> Please implement platformPreferences() in TestControllerEfl.cpp instead of
> adding more #ifdefs.

It seems GTK layout test also has been broken since r188828. Fix it too.

[WebKit Commit Bot]

Comment on attachment 259912 [details]
Patch

Clearing flags on attachment: 259912

Committed r188948: <http://trac.webkit.org/changeset/188948>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Csaba Osztrogonác]

Comment on attachment 259912 [details]
Patch

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

> Tools/WebKitTestRunner/TestController.cpp:1609
> +#if !PLATFORM(EFL) && !PLATFORM(GTK) && !PLATFORM(COCOA)
>  WKPreferencesRef TestController::platformPreferences()
>  {
>      WKRetainPtr<WKPageConfigurationRef> configuration = adoptWK(WKPageCopyPageConfiguration(m_mainWebView->page())); 
>      return WKPageConfigurationGetPreferences(configuration.get());
>  }
> +#endif

Aren't these lines unused? As far as I know there isn't this kind 
of platform. WK2 platforms are only Mac and iOS (Cocoa), EFL, GTK.

[Gyuyoung Kim]

Comment on attachment 259912 [details]
Patch

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

>> Tools/WebKitTestRunner/TestController.cpp:1609
>> +#endif
> 
> Aren't these lines unused? As far as I know there isn't this kind 
> of platform. WK2 platforms are only Mac and iOS (Cocoa), EFL, GTK.

If so, this is unused code. I didn't know it. I'm sorry.

[Csaba Osztrogonác]

(In reply to comment #14)
> Comment on attachment 259912 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=259912&action=review
> 
> >> Tools/WebKitTestRunner/TestController.cpp:1609
> >> +#endif
> > 
> > Aren't these lines unused? As far as I know there isn't this kind 
> > of platform. WK2 platforms are only Mac and iOS (Cocoa), EFL, GTK.
> 
> If so, this is unused code. I didn't know it. I'm sorry.

No problem, let me remove them in bug148467

[Xabier Rodríguez Calvar]

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