[webkit-reviews] review granted: [Bug 71324] Expand DragController to provide more information about the dragging session : [Attachment 113376] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Jon Lee <jonlee at apple.com>'s request
for review:
Bug 71324: Expand DragController to provide more information about the dragging
session
https://bugs.webkit.org/show_bug.cgi?id=71324

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=113376&action=review


Not sure I love the name DragSession, but I guess it’s OK.

> Source/WebCore/page/DragController.cpp:353
> +	   } else
> +	       // We are not over a file input element. The dragged item(s)
will only
> +	       // be loaded into the view the number of dragged items is 1.
> +	       dragSession.numberOfItemsToBeAccepted = numberOfFiles != 1 ? 0 :
1;

WebKit coding style says you should use braces here. It’s the number of lines
that determines the braces, not the number of statements.

> Source/WebCore/page/DragSession.h:37
> +    DragSession()
> +    : operation(DragOperationNone)
> +    , mouseIsOverFileInput(false)
> +    , numberOfItemsToBeAccepted(0) { }

This is not the formatting we use in WebKit code. That would be:

    DragSession()
	: operation(DragOperationNone)
	, mouseIsOverFileInput(false)
	, numberOfItemsToBeAccepted(0)
    {
    }

Also, we normally put the constructor after the data members in a separate
paragraph when it’s a struct rather than a class.

> Source/WebKit2/UIProcess/WebPageProxy.cpp:186
> +    , m_currentDragSession()

You should be able to delete this line of code entirely. The default
constructor is called if you don’t explicitly initialize the data member.