[Webkit-unassigned] [Bug 86296] Implement LayoutTest's eventSender.beginDragWithFiles interface in windows platform

[bugzilla-daemon at webkit.org]

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


Tony Chang <tony at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #143554|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #8 from Tony Chang <tony at chromium.org>  2012-05-23 09:59:46 PST ---
(From update of attachment 143554)
View in context: https://bugs.webkit.org/attachment.cgi?id=143554&action=review

Please run check-webkit-style.  It would be nice to figure out why your diff isn't applying properly.  Are you using webkit-patch or svn-create-patch?

> Source/WebCore/ChangeLog:8
> +        Test: This modification was an requirement for eventSender.beginDragWithiFiles, update win/skipped file to reomve drag-drop releated test case will valiate this. 

Typo: reomve -> remove.

Do any of the tests in win/Skipped pass for you on Win or WinCE after this change?  If so, we can remove them from win/Skipped in this change.

> Tools/DumpRenderTree/win/DRTDataObject.cpp:47
> +    WCEnumFormatEtc(const Vector<FORMATETC>& formats);
> +    WCEnumFormatEtc(const Vector<FORMATETC*>& formats);

explicit

> Tools/DumpRenderTree/win/DRTDataObject.cpp:71
> +    for (size_t i = 0; i < formats.size(); ++i)
> +        m_formats.append(formats[i]);

Nit: I think you can just use assignment here.

> Tools/DumpRenderTree/win/DRTDataObject.cpp:102
> +    if (refCount == 0)

if (!refCount).  I think check-webkit-style would have caught this.

> Tools/DumpRenderTree/win/DRTDataObject.cpp:109
> +    if (pceltFetched != 0)

if (pceltFetched)

> Tools/DumpRenderTree/win/DRTDataObject.cpp:114
> +    if (celt <= 0 || lpFormatEtc == 0 || m_current >= m_formats.size())

!lpFormatEtc

> Tools/DumpRenderTree/win/DRTDataObject.cpp:117
> +    if (pceltFetched == 0 && celt != 1) // pceltFetched can be 0 only for 1 item request

Here too

> Tools/DumpRenderTree/win/DRTDataObject.cpp:124
> +    if (pceltFetched != 0)

And here

> Tools/DumpRenderTree/win/DRTDataObject.cpp:127
> +    return cReturn == 0 ? S_OK : S_FALSE;

And here

> Tools/DumpRenderTree/win/DRTDataObject.cpp:203
> +    if (refCount == 0)

if (!refCount)

> Tools/DumpRenderTree/win/DRTDataObject.cpp:236
> +    HRESULT  hr = DV_E_TYMED;

extra space?  Also, try not to abbreviate variable names.  hr -> hresult

> Tools/DumpRenderTree/win/DRTDataObject.cpp:260
> +    FORMATETC* fetc = new FORMATETC;

fetc -> formatetc

> Tools/DumpRenderTree/win/DRTDataObject.cpp:272
> +    ZeroMemory(fetc,sizeof(FORMATETC));
> +    ZeroMemory(pStgMed,sizeof(STGMEDIUM));

Spaces after ,

> Tools/DumpRenderTree/win/DRTDataObject.cpp:319
> +    if (pMedSrc->pUnkForRelease != 0) {

Comparison to zero.

> Tools/DumpRenderTree/win/DRTDataObject.cpp:364
> +    size_t ptr = 0;

ptr -> position

> Tools/DumpRenderTree/win/DRTDataObject.cpp:367
> +            FORMATETC* current = m_formats[ptr];

Can we just delete m_formats[ptr] and void using a temporary variable?

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

Can we just use m_formats.remove(ptr)?

> Tools/DumpRenderTree/win/EventSender.cpp:644
> +    if (filesArray) {
> +        JSStringRef lengthProperty = JSStringCreateWithUTF8CString("length");

Nit: We normally early return rather than indenting the majority of the function.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.