[webkit-reviews] review denied: [Bug 176894] [WinCairo] Add WTF files for wincairo webkit : [Attachment 322481] [PATCH] Add WTF files for WinCairo WebKit
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Oct 2 21:18:10 PDT 2017
Alex Christensen <achristensen at apple.com> has denied Yousuke Kimoto
<Yousuke.Kimoto at sony.com>'s request for review:
Bug 176894: [WinCairo] Add WTF files for wincairo webkit
https://bugs.webkit.org/show_bug.cgi?id=176894
Attachment 322481: [PATCH] Add WTF files for WinCairo WebKit
https://bugs.webkit.org/attachment.cgi?id=322481&action=review
--- Comment #2 from Alex Christensen <achristensen at apple.com> ---
Comment on attachment 322481
--> https://bugs.webkit.org/attachment.cgi?id=322481
[PATCH] Add WTF files for WinCairo WebKit
View in context: https://bugs.webkit.org/attachment.cgi?id=322481&action=review
> Source/WTF/wtf/WorkQueue.h:28
> -#ifndef WorkQueue_h
> -#define WorkQueue_h
> +#pragma once
Unfortunately we need to keep the c-style guard because of inconsistent
including in the cocoa ports for now.
> Source/WTF/wtf/threads/BinarySemaphore.h:51
> +#if PLATFORM(WIN)
> + HANDLE m_event;
I think it's a bad idea to have such a platform-dependent difference in design
of something like BinarySemaphore. What's wrong with Mutex and
ThreadCondition?
> Source/WTF/wtf/threads/win/BinarySemaphoreWin.cpp:39
> +BinarySemaphore::~BinarySemaphore()
> +{
> + ::CloseHandle(m_event);
The Win32 API has quite a bit of overhead for things like this. We have our
custom WTF types to make them thin and lightweight. c++11 added many things we
can use instead.
More information about the webkit-reviews
mailing list