[webkit-reviews] review denied: [Bug 86296] Implement LayoutTest's eventSender.beginDragWithFiles interface in windows platform : [Attachment 143991] patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri May 25 10:39:35 PDT 2012


Tony Chang <tony at chromium.org> has denied huangxueqing
<huangxueqing at baidu.com>'s request for review:
Bug 86296: Implement LayoutTest's eventSender.beginDragWithFiles interface in
windows platform
https://bugs.webkit.org/show_bug.cgi?id=86296

Attachment 143991: patch
https://bugs.webkit.org/attachment.cgi?id=143991&action=review

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=143991&action=review


Looking much better.  Can you file a bug about check-webkit-style not running
on Chinese Windows?  Please include a stack trace.

> Tools/DumpRenderTree/win/DRTDataObject.cpp:373
> +	       m_formats[m_formats.size() - 1] = 0;
> +	       m_formats.removeLast();

Do we need to set "m_formats[m_formats.size() - 1] = 0" if we're going to
remove it?

> Tools/DumpRenderTree/win/DRTDataObject.h:68
> +    Vector<FORMATETC*> m_formats;
> +    Vector<STGMEDIUM*> m_medium;

Can we use Vector<OwnPtr<FORMATETC> > and Vector<OwnPtr<STGMEDIUM> >?

> Tools/DumpRenderTree/win/DRTDropSource.h:36
> +    virtual ULONG STDMETHODCALLTYPE AddRef(void);
> +    virtual ULONG STDMETHODCALLTYPE Release(void);

Can we remove void?

> Tools/DumpRenderTree/win/EventSender.cpp:668
> +    DROPFILES* dropFiles = reinterpret_cast<DROPFILES
*>(GlobalLock(medium.hGlobal));

Nit: No space between DROPFILES and *.


More information about the webkit-reviews mailing list