[webkit-reviews] review requested: [Bug 176894] [WinCairo] Add WTF files for wincairo webkit : [Attachment 323984] [PATCH] Add WTF files for WinCairo WebKit
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Oct 16 20:45:31 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 323984: [PATCH] Add WTF files for WinCairo WebKit
https://bugs.webkit.org/attachment.cgi?id=323984&action=review
--- Comment #8 from Yousuke Kimoto <Yousuke.Kimoto at sony.com> ---
Created attachment 323984
--> https://bugs.webkit.org/attachment.cgi?id=323984&action=review
[PATCH] Add WTF files for WinCairo WebKit
(In reply to Alex Christensen from comment #7)
> > Source/WTF/wtf/WorkQueue.h:114
> > + HashMap<HANDLE, RefPtr<HandleWorkItem> > m_items;
> > + unregisterWaitAndDestroyItemSoon(item);
> > + return adoptRef(new WorkItemWin(WTFMove(function), queue));
> > + return adoptRef(new HandleWorkItem(handle, WTFMove(function), queue));
Fixed these points which used RefPtr.
> > Source/WTF/wtf/win/WorkQueueWin.cpp:256
> > + DWORD error = ::GetLastError();
>
> Do we want to return this?
"error" is unnecessary. Those codes to just get errors are deleted, and those
cases are treated as ASSERT cases.
> > Source/WTF/wtf/win/WorkItemWin.cpp:54
> > + , m_waitHandle(0)
>
> This should use C++11-style initializer lists in the header.
Fixed.
> > Source/WTF/wtf/win/WorkItemWin.h:61
> > + void setWaitHandle(HANDLE waitHandle) { m_waitHandle = waitHandle; }
>
> Can we not put this waitHandle in the constructor somehow? It's usually a
> bad idea to make an object and then have to remember to call setters in
> order to make that object valid. It makes it easy to forget or to have
> complicated flow paths that makes it hard to tell that you forgot.
In this patch, WorkItemContext is added to handle context for tasks to solve
the point.
It is not a good method but setWaitHandle() was deleted.
More information about the webkit-reviews
mailing list