Bug 61350 – [CMAKE] Split JSC related files from WebCore/CMakeLists.txt

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[Ryuan Choi]

I already moved JSC related files to UseJSC.cmake at bug 56624, but some files were missing.

[Ryuan Choi]

Created attachment 94582 [details]
Patch

[Patrick R. Gansterer]

LGTM

[Raphael Kubo da Costa (:rakuco)]

It looks mostly OK to me too. Should the platform-specific ScriptController${PLATFORM}.cpp files still be included in WebCore/CMakeLists{Efl,WinCE}.cmake?

[Ryuan Choi]

(In reply to comment #3)
> It looks mostly OK to me too. Should the platform-specific ScriptController${PLATFORM}.cpp files still be included in WebCore/CMakeLists{Efl,WinCE}.cmake?

It looks reasonable to me,
but WinCE use ScriptControllerWin.cpp.

Patrick, can we clone or rename ScriptControllerWin.cpp to ScriptControllerWinCE.cpp?

[Patrick R. Gansterer]

(In reply to comment #4)
> (In reply to comment #3)
> > It looks mostly OK to me too. Should the platform-specific ScriptController${PLATFORM}.cpp files still be included in WebCore/CMakeLists{Efl,WinCE}.cmake?
> 
> It looks reasonable to me,
> but WinCE use ScriptControllerWin.cpp.
> 
> Patrick, can we clone or rename ScriptControllerWin.cpp to ScriptControllerWinCE.cpp?

Don't WinApple and WinCairo use this file too?
I don't think this is a good idea. IMHO using ${PLATFORM} for filenames and directories is a very bad idea, because different platforms share different code. An example where we have this problem already is http://trac.webkit.org/browser/trunk/Source/WebKit/CMakeLists.txt?rev=83792#L65. When I Create a WinApple, WinCairo and WinGDI port, all of them use the win-directory, but have different implementations of GraphicsCntext, NetworkInterface and so on.
I'd like to keep adding of _all_ the platform specific files in different files.

[Patrick R. Gansterer]

What's about merging ScriptControllerBrew, ScriptControllerEfl, ScriptControllerGtk, ScriptControllerHaiku, ScriptControllerWin and ScriptControllerWx and give it a better name like ScriptControllerDefault?

[Ryuan Choi]

(In reply to comment #6)
> What's about merging ScriptControllerBrew, ScriptControllerEfl, ScriptControllerGtk, ScriptControllerHaiku, ScriptControllerWin and ScriptControllerWx and give it a better name like ScriptControllerDefault?

Sure, It looks quite better.
I wish to make different bug for that.

[Ryuan Choi]

(In reply to comment #6)
> What's about merging ScriptControllerBrew, ScriptControllerEfl, ScriptControllerGtk, ScriptControllerHaiku, ScriptControllerWin and ScriptControllerWx and give it a better name like ScriptControllerDefault?

I consider it little more.
ScripControllerDefault.cpp can be used to reduce duplication.
but, we can't include this file in UseJSC.cmake because some port which use CMake may want own ScriptController${PORT}.cpp.

For ScriptController${PLATFORM}.cpp now,
I don't have idea without adding if statement to check JavaScript engine in each WebCore/CMakeLists{Efl,WinCE}.txt.

[Ryuan Choi]

Created attachment 95604 [details]
Patch

[Patrick R. Gansterer]

Comment on attachment 95604 [details]
Patch

LGTM

[Ryuan Choi]

could anyone review this?

[Ryuan Choi]

Created attachment 100466 [details]
rebased_patch

[Daniel Bates]

Comment on attachment 100466 [details]
rebased_patch

OK

[WebKit Review Bot]

Comment on attachment 100466 [details]
rebased_patch

Clearing flags on attachment: 100466

Committed r91742: <http://trac.webkit.org/changeset/91742>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.