Bug 161932

Summary: Merge Element::ScrollToOptions and DOMWindow::ScrollToOptions
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: DOMAssignee: Chris Dumez <cdumez>
Status: RESOLVED DUPLICATE    
Severity: Normal CC: cdumez, cgarcia, cmarcelo, commit-queue, darin, dbates, esprehn+autocc, gyuyoung.kim, kangil.han, kondapallykalyan, ryanhaddad, sam, simon.fraser
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 162912    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch none

Chris Dumez
Reported 2016-09-13 15:38:44 PDT
Merge Element::ScrollToOptions and DOMWindow::ScrollToOptions.
Attachments
Patch (14.61 KB, patch)
2016-09-13 15:41 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (14.64 KB, patch)
2016-09-13 22:00 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Simon Fraser (smfr)
Comment 1 2016-09-13 15:40:19 PDT
*** Bug 161643 has been marked as a duplicate of this bug. ***
Chris Dumez
Comment 2 2016-09-13 15:41:44 PDT
Created attachment 288739 [details] Patch
Simon Fraser (smfr)
Comment 3 2016-09-13 15:42:41 PDT
Comment on attachment 288739 [details] Patch Sneaky inline!
WebKit Commit Bot
Comment 4 2016-09-13 16:29:54 PDT
Comment on attachment 288739 [details] Patch Clearing flags on attachment: 288739 Committed r205887: <http://trac.webkit.org/changeset/205887>
WebKit Commit Bot
Comment 5 2016-09-13 16:30:01 PDT
All reviewed patches have been landed. Closing bug.
Ryan Haddad
Comment 6 2016-09-13 21:46:26 PDT
This change appears to have broken the Windows build: https://build.webkit.org/builders/Apple%20Win%20Release%20%28Build%29/builds/80457
Chris Dumez
Comment 7 2016-09-13 21:53:02 PDT
(In reply to comment #6) > This change appears to have broken the Windows build: > > https://build.webkit.org/builders/Apple%20Win%20Release%20%28Build%29/builds/ > 80457 C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\Release\DerivedSources\WebCore\JSElement.cpp(145): error C2766: explicit specialization; 'WTF::Optional<WebCore::ScrollToOptions> WebCore::convertDictionary<WebCore::ScrollToOptions>(JSC::ExecState &,JSC::JSValue)' has already been defined (compiling source file C:\cygwin\home\buildbot\slave\win-release\build\Source\WebCore\DerivedSources.cpp) [C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\Release\Source\WebCore\WebCoreDerivedSources.vcxproj] C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\Release\DerivedSources\WebCore\JSDOMWindow.cpp(647): note: see previous definition of 'convertDictionary' (compiling source file C:\cygwin\home\buildbot\slave\win-release\build\Source\WebCore\DerivedSources.cpp) Hmm. The inlining trick did not work on Windows :(
Chris Dumez
Comment 8 2016-09-13 21:59:06 PDT
Reverted r205887 for reason: Broke the Windows build Committed r205897: <http://trac.webkit.org/changeset/205897>
Chris Dumez
Comment 9 2016-09-13 22:00:21 PDT
Created attachment 288771 [details] Patch
Chris Dumez
Comment 10 2016-09-13 22:18:29 PDT
ALWAYS_INLINE does not seem to work on Windows either. Looks like we're going to have to find another way.
Sam Weinig
Comment 11 2016-09-13 23:43:10 PDT
Instead of inlining, does wrapping the functions in an anonymous namespace work?
Chris Dumez
Comment 12 2016-09-14 09:37:58 PDT
(In reply to comment #11) > Instead of inlining, does wrapping the functions in an anonymous namespace > work? This does not seem to work. I believe the template declaration and its specializations need to be in the same namespace. Error: /Volumes/Data/WebKit/OpenSource/WebKitBuild/Release/DerivedSources/WebCore/JSElement.cpp:116:58: error: no function template matches function template specialization 'convertDictionary' namespace { template<> Optional<Element::ShadowRootInit> convertDictionary<Element::ShadowRootInit>(ExecState& state, JSValue value)
Chris Dumez
Comment 13 2016-10-06 13:25:39 PDT
*** This bug has been marked as a duplicate of bug 162912 ***
Note You need to log in before you can comment on or make changes to this bug.