[webkit-reviews] review denied: [Bug 29666] ClipboardWin::files() isn't implemented in Windows : [Attachment 41405] patch #4

[bugzilla-daemon at webkit.org]

Adam Roben (aroben) <aroben at apple.com> has denied Marshall Culpepper
<mculpepper at appcelerator.org>'s request for review:
Bug 29666: ClipboardWin::files() isn't implemented in Windows
https://bugs.webkit.org/show_bug.cgi?id=29666

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

------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
> +    STGMEDIUM medium;
> +    if (FAILED(m_dataObject->GetData(cfHDropFormat(), &medium)))
> +	   return files.release();
> +
> +    HDROP hdrop = reinterpret_cast<HDROP>(GlobalLock(medium.hGlobal));
> +    if (!hdrop)
> +	   return files.release();
> +
> +    WCHAR filename[MAX_PATH];
> +    UINT fileCount = DragQueryFileW(hdrop, 0xFFFFFFFF, 0, 0);
> +    for (UINT i = 0; i < fileCount; i++) {
> +	   if (!DragQueryFileW(hdrop, i, filename, ARRAYSIZE(filename)))
> +	       continue;
> +	   files->append(File::create(reinterpret_cast<UChar*>(filename)));
> +    }
> +
> +    ReleaseStgMedium(&medium);
> +    GlobalUnlock(medium.hGlobal);

I think you should reverse the calls to ReleaseStgMedium and GlobalUnlock here.
Otherwise you're using the medium after releasing it, which, while it may
happen to work, doesn't seem like a good thing to do.

Once that's fixed, I think we're good to go!