[Webkit-unassigned] [Bug 176894] [WinCairo] Add WTF files for wincairo webkit

[bugzilla-daemon at webkit.org]

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

Alex Christensen <achristensen at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #323012|review?                     |review-
              Flags|                            |

--- Comment #7 from Alex Christensen <achristensen at apple.com> ---
Comment on attachment 323012
  --> https://bugs.webkit.org/attachment.cgi?id=323012
[PATCH] Add WTF files for WinCairo WebKit

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

> Source/WTF/wtf/WorkQueue.h:114
> +    HashMap<HANDLE, RefPtr<HandleWorkItem> > m_items;

>> instead of > >
Also, could the value be made a Ref?

> Source/WTF/wtf/win/WorkQueueWin.cpp:76
> +    unregisterWaitAndDestroyItemSoon(item);

Would it be useful to assert that item is non-null?  Maybe unregisterWaitAndDestroyItemSoon could take a Ref.

> Source/WTF/wtf/win/WorkQueueWin.cpp:256
> +        DWORD error = ::GetLastError();

Do we want to return this?

> Source/WTF/wtf/win/WorkItemWin.cpp:44
> +    return adoptRef(new WorkItemWin(WTFMove(function), queue));

adoptRef(*new

> Source/WTF/wtf/win/WorkItemWin.cpp:54
> +    , m_waitHandle(0)

This should use C++11-style initializer lists in the header.

> Source/WTF/wtf/win/WorkItemWin.cpp:61
> +    return adoptRef(new HandleWorkItem(handle, WTFMove(function), queue));

This should return a Ref.
return adoptRef(*new ...)

> 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.

-- 
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/20171006/f28857ee/attachment-0001.html>