[webkit-reviews] review requested: [Bug 112155] [Qt][WK2] Don't require a callback from the application to start a download : [Attachment 194233] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Mar 21 05:00:16 PDT 2013


Jocelyn Turcotte <jocelyn.turcotte at digia.com> has asked  for review:
Bug 112155: [Qt][WK2] Don't require a callback from the application to start a
download
https://bugs.webkit.org/show_bug.cgi?id=112155

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

------- Additional Comments from Jocelyn Turcotte <jocelyn.turcotte at digia.com>
(In reply to comment #5)
> > Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:1062
> > +bool QQuickWebViewExperimental::downloadUnsupportedContent() const
> 
> Just a generic idea: maybe providing a list of mimetypes of interest would be
more interesting than a boolean?
> 
> It depends on what people use this code for obviously :)

The current call is to remain similar to QWebPage::forwardUnsupportedContent,
but I can carry this idea to the API review.

> > Source/WebKit2/UIProcess/qt/QtDownloadManager.cpp:125
> > +	 for (int i = 1; i < 100; ++i) {
> 
> Could the limit of 100 be a problem?
> Is it covered by a test?

Right now it is going to fail the download if all of them are taken.
I've increased it to 10000 to make it very unlikely that a user will face this
limitation while keeping the number readable in the file name. The goal is to
let the application handle extreme use cases by moving/renaming the file once
the download is finished.

There is no test but I'm adding a download UI to MiniBrowser to be able to test
the general download code manually at least. Having an automated test to cover
the 10000 limit wouldn't be profitable I think (long execution time and
potential flackiness).


More information about the webkit-reviews mailing list