[webkit-reviews] review denied: [Bug 77125] [chromium] Add setter/getter to expose drag data as a list of items : [Attachment 124893] Patch

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has denied Daniel Cheng <dcheng at chromium.org>'s
request for review:
Bug 77125: [chromium] Add setter/getter to expose drag data as a list of items
https://bugs.webkit.org/show_bug.cgi?id=77125

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

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


> Source/WebKit/chromium/public/platform/WebDragData.h:53
> +    // Internally, we represent data in a drag as a list of

I can't tell if this sentence stops intentionally or if 'struct Item' is the
end of the sentence.  Please complete the sentence and end with a period.

> Source/WebKit/chromium/public/platform/WebDragData.h:63
> +	   // Only valid when storageType == STRING

Nit: End the sentence with a period, here and below.

> Source/WebKit/chromium/src/WebDragData.cpp:72
> +    HashSet<String> types = m_private->types();

Can |types| be a const reference?

> Source/WebKit/chromium/src/WebDragData.cpp:128
> +void WebDragData::setItemList(const WebVector<Item>& itemList)
> +{
> +    for (size_t i = 0; i < itemList.size(); ++i)
> +	   addItem(itemList[i]);

Do we need to clear out any old items?