[webkit-reviews] review denied: [Bug 32473] [Chromium] Add 2 parameters to WebViewClient::runFileChooser() : [Attachment 44740] Proposed patch

[bugzilla-daemon at webkit.org]

Darin Fisher (:fishd, Google) <fishd at chromium.org> has denied TAMURA, Kent
<tkent at chromium.org>'s request for review:
Bug 32473: [Chromium] Add 2 parameters to WebViewClient::runFileChooser()
https://bugs.webkit.org/show_bug.cgi?id=32473

Attachment 44740: Proposed patch
https://bugs.webkit.org/attachment.cgi?id=44740&action=review

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
> +++ b/WebKit/chromium/public/WebViewClient.h
...
>      virtual bool runFileChooser(
>	   bool multiSelect, const WebString& title,
> -	   const WebString& initialValue, WebFileChooserCompletion*) { return
false; }
> +	   const WebString& initialValue,
> +	   const WebVector<WebString>& selectedFiles, const WebString&
acceptTypes,
> +	   WebFileChooserCompletion*) { return false; }

Instead of dumping more parameters on this function, I think we should
create a WebFileChooserParams structure (much like WebPluginParams).

This way it'll be easier to add additional parameters in the future, and
we can document the parameters in WebFileChooserParams.h instead of here
in WebViewClient.h (which should improve readability a bit).

To avoid the issue of breaking the chromium build, please preserve the old
WebViewClient::runFileChooser method temporarily.  Add a comment to declare
that that old method is deprecated.

You can even use the return value of runFileChooser to failover to the old
deprecated method if the new method returns false.  Then, once the chromium
side picks up this new version of WebKit, you should go back and remove the
deprecated function.  This way there is no burden on the chromium tree
sheriffs.