[Webkit-unassigned] [Bug 189257] Resurrect WebKitTestRunner for Windows port

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=189257

--- Comment #11 from Fujii Hironori <Hironori.Fujii at sony.com> ---
Comment on attachment 354199
  --> https://bugs.webkit.org/attachment.cgi?id=354199
Patch

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

> ChangeLog:3
> +        Implement WebKitTestRunner for WinCairo

The summary doesn't match. Did you use prepare-ChangeLog? Change either the ChangeLog or Bugzilla summary.

> Source/cmake/OptionsWin.cmake:87
> +    WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_ACCESSIBILITY PRIVATE ON)

I want to know the reason why ENABLE_ACCESSIBILITY is needed.

> Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:-28
> -

Do not remove this blank line.
https://webkit.org/code-style-guidelines/#include-others

> Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:32
> +

Do not add a blank line here.
https://webkit.org/code-style-guidelines/#include-others

> Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:882
> +#if USE(CF) && !PLATFORM(WIN_CAIRO)

Why do you need this guard?

> Tools/WebKitTestRunner/PlatformWin.cmake:29
> +    ${DERIVED_SOURCES_DIR}/WebKit/Interfaces

Remove ${DERIVED_SOURCES_DIR}/WebKit/Interfaces. This is WK1.

> Tools/WebKitTestRunner/PlatformWin.cmake:52
> +    ${WEBKIT_TESTRUNNER_BINDINGS_DIR}/JSWrapper.cpp

I want to know the reason why you remove JSWrapper.cpp.

> Tools/WebKitTestRunner/PlatformWin.cmake:60
> +    "${WEBKIT_TESTRUNNER_DIR}/win/WebKitTestRunnerPrefix.cpp"

You don't need these double-quote here. CMake string quoting rule is different from shell scripts.

> Tools/WebKitTestRunner/TestController.cpp:77
> +#endif

Original code was sorted alphabetically.
Move these code bellow in this case.

> Tools/WebKitTestRunner/TestController.cpp:881
> +#if !PLATFORM(COCOA) && !PLATFORM(WPE) && !PLATFORM(WIN)

#if PLATFORM(GTK)
because WKTextCheckerContinuousSpellCheckingEnabledStateChanged is a GTK-only API.

> Tools/WebKitTestRunner/TestInvocation.cpp:50
> +#endif

Original code was sorted alphabetically.
Move these code bellow in this case.

> Tools/WebKitTestRunner/win/TestControllerWin.cpp:179
> +    TCHAR exePath[MAX_PATH];

Theoretically you are right, but TCHAR is not used nowadays. Simply use wchar_t. Don't include tchar.h. Don't use _t* API.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20181108/8c811288/attachment-0001.html>