[webkit-reviews] review requested: [Bug 176894] [WinCairo] Add WTF files for wincairo webkit : [Attachment 322784] [PATCH] Add WTF files for WinCairo WebKit
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Oct 4 22:29:34 PDT 2017
Yousuke Kimoto <Yousuke.Kimoto at sony.com> has asked for review:
Bug 176894: [WinCairo] Add WTF files for wincairo webkit
https://bugs.webkit.org/show_bug.cgi?id=176894
Attachment 322784: [PATCH] Add WTF files for WinCairo WebKit
https://bugs.webkit.org/attachment.cgi?id=322784&action=review
--- Comment #3 from Yousuke Kimoto <Yousuke.Kimoto at sony.com> ---
Created attachment 322784
--> https://bugs.webkit.org/attachment.cgi?id=322784&action=review
[PATCH] Add WTF files for WinCairo WebKit
Thank you for your review.
(In reply to Alex Christensen from comment #2)
> > 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.
Should only be WorkQueue.h considered for cocoa ports to keep c-style guard?
I'm interested in which file should use the guard to avoid the inconsistency.
> > 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.
I fixed this point, WTF/wtf/BinarySemaphore.cpp is used in this patch.
More information about the webkit-reviews
mailing list