[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]

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.