[webkit-reviews] review granted: [Bug 63039] FileChooser should be only created when we need to choose files. : [Attachment 98114] Silly mistake fixed.

[bugzilla-daemon at webkit.org]

Kent Tamura <tkent at chromium.org> has granted Dimitri Glazkov (Google)
<dglazkov at chromium.org>'s request for review:
Bug 63039: FileChooser should be only created when we need to choose files.
https://bugs.webkit.org/show_bug.cgi?id=63039

Attachment 98114: Silly mistake fixed.
https://bugs.webkit.org/attachment.cgi?id=98114&action=review

------- Additional Comments from Kent Tamura <tkent at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=98114&action=review

> Source/WebCore/ChangeLog:24
> +	   1) Introduce FileChooserSettings to decouple setting querying from
> +	   FileChooser. It's a simple copyable settings object, which allows us

> +	   to capture the settings atomically and treat them as discardable
data.
> +
> +	   2) Encapsulate lifetime management of FileChooser entirely in
> +	   FileChooserClient. It's now a "smart" client, and allows us to
> +	   completely remove FileChooser management concerns from a
FileChooserClient
> +	   implementor.
> +
> +	   3) Change creation of FileChooser to be on-demand, only when we
actually
> +	   need to choose file.
> +
> +	   4) Rearrange calling of dispatchFormControlChangeEvent to be at the
end
> +	   of a function and remove "am-I-dead" checks that are now
unnecessary.
> +
> +	   5) Clean up directory upload code a bit, and make use of
FileChooserSettings
> +	   to pass directory name.

In general, doing 5 things in a single patch is not good :-)

> Source/WebCore/rendering/RenderFileUploadControl.cpp:57
> +static void filenamesFromFileList(FileList* list, Vector<String>& filenames)


FileList* should be const FileList*.

> Source/WebCore/rendering/RenderFileUploadControl.cpp:94
> +    inputElement->dispatchFormControlChangeEvent();

nit: We had better have a comment that this dispatchFormControlChangeEvent()
call should be put on the bottom of the function.