Bug 90346

I'm looking into it.

Seems like an export problem, and I have no clue why does it work on other platforms or even qt-x86 (if I recall correctly this is the second time we have such a strange export issue on ARM). I thing we have to export these functions to be able to use them in WTR.

Here is the diff on Qt: -viewport size 320x356 scale 1.000000 with limits [1.000000, 5.000000] and userScalable -1.000000 +viewport size 480x534 scale 1.000000 with limits [1.000000, 5.000000] and userScalable -1.000000 Relevant parts of the test: <meta name="viewport" content="width=device-width"> layoutTestController.dumpConfigurationForViewport(240, 480, 564, 480, 534); So we don't divide the width and height dimensions with 1.5 (240/160), which is surprising, since the patch keeps the division, just organize it to the call sites.

(In reply to comment #3) > Here is the diff on Qt: > > -viewport size 320x356 scale 1.000000 with limits [1.000000, 5.000000] and userScalable -1.000000 > +viewport size 480x534 scale 1.000000 with limits [1.000000, 5.000000] and userScalable -1.000000 > > Relevant parts of the test: > <meta name="viewport" content="width=device-width"> > layoutTestController.dumpConfigurationForViewport(240, 480, 564, 480, 534); > > So we don't divide the width and height dimensions with 1.5 (240/160), which is surprising, since the patch keeps the division, just organize it to the call sites. Ups, wrong bug...

Created attachment 150432 [details] Patch

Comment on attachment 150432 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=150432&action=review I think this is the right solution, exporting those symbols via export macros. But a little bit of fine tuning is needed :) > Source/WebCore/ChangeLog:8 > + [Qt] REGRESSION(r121550): It broke the Qt5 ARM build > + https://bugs.webkit.org/show_bug.cgi?id=90346 > + > + Reviewed by NOBODY (OOPS!). > + > + No new tests, this is a build fix. I suggest to correct the title and the description of the change log entry here. The title makes it look like that this is an ARM specific problem, but that is incorrect. It is a general build problem of the WK2 part of the Qt port for platforms that require symbol exports. This is the line in default_post.prf that controls this: !linux-g++*:!linux-icc*:contains(QT_CONFIG, reduce_exports):CONFIG += hide_symbols and as it so happens the arm mkspec doesn't match linux-g++*. But _that_ line itself is incorrect and I should've removed the linux-g++* part as part of r106650. Then this problem would manifest itself also on the x86 WK2 bot. I think the bug description should briefly include how the patch accomplishes to fix the issue. > Source/WebCore/testing/js/WebCoreTestSupport.h:35 > +#include <wtf/ExportMacros.h> > + > +#if PLATFORM(QT) > +#define TEST_SUPPORT_EXPORT WTF_EXPORT_PRIVATE > +#else > +#define TEST_SUPPORT_EXPORT > +#endif I think those belong into WebCore/platform/PlatformExportMacros.h and maybe should be prefixed with WEBKIT, i.e. WEBKIT_EXPORT_TEST_SUPPORT.

Created attachment 150627 [details] Patch

Comment on attachment 150627 [details] Patch Attachment 150627 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/13130514

Comment on attachment 150627 [details] Patch Attachment 150627 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/13136453

Created attachment 150629 [details] Patch

Comment on attachment 150629 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=150629&action=review > Source/WebCore/platform/PlatformExportMacros.h:76 > +// Special cases. > + > +#if PLATFORM(QT) > +#if defined(BUILDING_WebCore) || defined(BUILDING_WebKit) || \ > + defined(STATICALLY_LINKED_WITH_WebCore) || defined(STATICALLY_LINKED_WITH_WebKit) > +#define WEBKIT_TEST_SUPPORT_EXPORT WTF_EXPORT > +#else > +#define WEBKIT_TEST_SUPPORT_EXPORT WTF_IMPORT > +#endif > +#else > +#define WEBKIT_TEST_SUPPORT_EXPORT > +#endif // PLATFORM(QT) > + I would prefer something like this just after "#endif // USE(EXPORT_MACROS)": #if PLATFORM(QT) #define WEBKIT_TEST_SUPPORT_EXPORT WEBKIT_EXPORTDATA #else #define WEBKIT_TEST_SUPPORT_EXPORT #endif // PLATFORM(QT)

It turned out that other ports have a special lib for these things, for example on Gtk it is libWebCoreTestSupport. The better solution is to set this up for Qt as well.

Comment on attachment 150629 [details] Patch Attachment 150629 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/13134487

*** This bug has been marked as a duplicate of bug 90978 ***

(In reply to comment #6) [...] > The title makes it look like that this is an ARM specific problem, but that is incorrect. It is a general build problem of the WK2 part of the Qt port for platforms that require symbol exports. This is the line in default_post.prf that controls this: > > !linux-g++*:!linux-icc*:contains(QT_CONFIG, reduce_exports):CONFIG += hide_symbols > > and as it so happens the arm mkspec doesn't match linux-g++*. But _that_ line itself is incorrect and > I should've removed the linux-g++* part as part of r106650. Then this problem would manifest itself also > on the x86 WK2 bot. > > I think the bug description should briefly include how the patch accomplishes to fix the issue. I've filed bug #90981 to address this.

Let's start with this and do the separation later. It seems like a bit trickier than I expected (idl-s for example).

*** This bug has been marked as a duplicate of bug 90262 ***