Created attachment 130008 [details] proposed patch Attempting to cross-compile WebKitGtk+ for windows results in error: CXXLD libwebkitgtk-3.0.la Creating library file: .libs/libwebkitgtk-3.0.dll.a ./.libs/libWebCore.a(libWebCore_la-ScrollAnimatorWin.o):ScrollAnimatorWin.cpp:(.text+0x6d0): multiple definition of `WebCore::ScrollAnimator::create(WebCore::ScrollableArea*)' ./.libs/libWebCore.a(libWebCore_la-ScrollAnimatorNone.o):ScrollAnimatorNone.cpp:(.text+0x1330): first defined here collect2: error: ld returned 1 exit status make[1]: *** [libwebkitgtk-3.0.la] Error 1 make[1]: Leaving directory `/home/pawel/src/WebKit' make: *** [all] Error 2
Have you tried a clean build? We had this issue on the bots too (Linux) and a clean build worked out.
Comment on attachment 130008 [details] proposed patch I don't see any X11 specific code in ScrollAnimatorNone, moving these files to TARGET_X11 looks only like a workaround for your cross-compiling issue.
Yes i tried clean build (as in "make clean"). Build stopped again in this same place. With TARGET_WIN32 you have if TARGET_WIN32 webcore_sources += \ <------>Source/WebCore/platform/ScrollAnimatorWin.cpp \ <------>Source/WebCore/platform/ScrollAnimatorWin.h \ Which in turn doubles the definitions from ScrollAnimatorNone. So i think it should be like in attached patchlet or the other way around, removing ScrollAnimatorWin. Moving just ScrollAnimatorNone into TARGET_X11 guard seemed logical to me and it feels a bit more cautious.
Indeed both conflict, I'm no expert about this code but maybe we should choose one and keep it whatever the platform is to keep scrolling experience as consistent as we can. CCing Zan and Martin. What do you think?
Yea, sounds reasonable. The less duplication, the better. I'm sure they will come up with something nice.
(In reply to comment #4) > CCing Zan and Martin. What do you think? Oddly enough, the ScrollAnimatorWin.cpp and the header do not include any Windows-specific headers either. Anyway, like mentioned, I also think it would be best to use one class on every target, so I'd say removing ScrollAnimatorWin from the build and using ScrollAnimatorNone instead. ScrollAnimatorWin is also used by the WinCE port and it'd be best to avoid causing trouble if that class grows out of today's boundaries.
(In reply to comment #6) > Anyway, like mentioned, I also think it would be best to use one class on every target, so I'd say removing ScrollAnimatorWin from the build and using ScrollAnimatorNone instead. ScrollAnimatorWin is also used by the WinCE port and it'd be best to avoid causing trouble if that class grows out of today's boundaries. I think the important thing is to determine which animator best follows the scrolling behavior of GTK+ applications on Windows and use that one. If that turns out to be ScrollAnimatorWin.cpp, I think the risk of using it is very low.
(In reply to comment #7) > (In reply to comment #6) > > > Anyway, like mentioned, I also think it would be best to use one class on every target, so I'd say removing ScrollAnimatorWin from the build and using ScrollAnimatorNone instead. ScrollAnimatorWin is also used by the WinCE port and it'd be best to avoid causing trouble if that class grows out of today's boundaries. > > I think the important thing is to determine which animator best follows the scrolling behavior of GTK+ applications on Windows and use that one. If that turns out to be ScrollAnimatorWin.cpp, I think the risk of using it is very low. I'd like to hear bug reporter's feedback on the scrolling behavior with ScrollAnimatorWin class. If the behavior is acceptable/superior to ScrollAnimatorNone, the patch should be reuploaded with ChangeLog entries and reviewed.
Created attachment 131391 [details] Patch with Changelog entry I think behaviour with ScrollAnimatorNone is fine, updated patch to remove ScrollAnimatorWin
Comment on attachment 131391 [details] Patch with Changelog entry Rejecting attachment 131391 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: artin Rob..." exit_code: 1 cwd: /mnt/git/webkit-commit-queue/ Parsed 2 diffs from patch file(s). patching file Source/WebCore/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file Source/WebCore/GNUmakefile.list.am Hunk #1 FAILED at 4645. 1 out of 1 hunk FAILED -- saving rejects to file Source/WebCore/GNUmakefile.list.am.rej Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Martin Rob..." exit_code: 1 cwd: /mnt/git/webkit-commit-queue/ Full output: http://queues.webkit.org/results/11941183
*** This bug has been marked as a duplicate of bug 80737 ***