[Webkit-unassigned] [Bug 72949] [GTK] Add WebKitDownload to WebKit2 GTK+ API
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Jan 20 03:45:24 PST 2012
https://bugs.webkit.org/show_bug.cgi?id=72949
--- Comment #6 from Carlos Garcia Campos <cgarcia at igalia.com> 2012-01-20 03:45:24 PST ---
(In reply to comment #5)
> (From update of attachment 117631 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=117631&action=review
>
> Look nice! r- only because finished isn't emitted during failure.
Thanks for reviewing!
> > Source/WebKit2/UIProcess/API/gtk/WebKitDownload.cpp:125
> > + * The URI where the download will be saved.
>
> Might want to be a bit more explicit: The local URI to where the download...
Sure.
> > Source/WebKit2/UIProcess/API/gtk/WebKitDownload.cpp:138
> > + * The response of the download.
>
> How about: The #WebKitURIResponse associated with this download. ?
Better.
> > Source/WebKit2/UIProcess/API/gtk/WebKitDownload.cpp:193
> > + * This signal is emitted when download completes. In case of errors,
> > + * the download finishes when #WebKitDownload::failed signal is emitted.
>
> The loader client emits a finished message as well as a failed message. Let's copy that behavior here.
Right, this patch was written before the loader client simplification, no it makes sense to follow the same approach.
> > Source/WebKit2/UIProcess/API/gtk/WebKitDownload.cpp:260
> > + * This signal is emitted after #WebKitDownload::decide-destination to
> > + * notify that destination file has been created successfully at @destination.
>
> Might want to mention that this happens when the file is empty and not when the file is finished downloading.
Good point.
> > Source/WebKit2/UIProcess/API/gtk/WebKitDownload.cpp:312
> > + // in more then 0.7 secs from now, or the last notified progress
>
> "more than 0.7 seconds ago" is slightly clearer here.
Ok.
> > Source/WebKit2/UIProcess/API/gtk/WebKitDownload.cpp:319
> > + && (currentElapsed - priv->lastElapsed) < 0.7
>
> Why 0.7 seconds? Wouldn't it be better to use a value like 60 frames a second? That will ensure that download progress bars animate smoothly.
I have no idea, I copied this from wk1 code.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list