Summary: | [Qt][WK2] Layer QtDownloadManager on the C API | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jocelyn Turcotte <jturcotte> | ||||||||
Component: | New Bugs | Assignee: | Jocelyn Turcotte <jturcotte> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | allan.jensen, jturcotte | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | 109564 | ||||||||||
Bug Blocks: | |||||||||||
Attachments: |
|
Description
Jocelyn Turcotte
2013-02-12 03:51:14 PST
Created attachment 187828 [details]
Patch
Comment on attachment 187828 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=187828&action=review > Source/WebKit2/UIProcess/qt/QtDownloadManager.cpp:65 > + QtDownloadManager* q = toQtDownloadManager(clientInfo); manager? > Source/WebKit2/UIProcess/qt/QtDownloadManager.cpp:72 > + downloadItem->d->sourceUrl = WKURLCopyQUrl(WKURLResponseCopyURL(response)); > + downloadItem->d->mimeType = WKStringCopyQString(WKURLResponseCopyMIMEType(response)); I get confused when I see a Copy method and no adopt :-) but I get it, Qt types! > Source/WebKit2/UIProcess/qt/QtDownloadManager.cpp:101 > + QtDownloadManager* q = toQtDownloadManager(clientInfo); s/q/self/ ? that is what we use for EFL (In reply to comment #2) > (From update of attachment 187828 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=187828&action=review > > > Source/WebKit2/UIProcess/qt/QtDownloadManager.cpp:65 > > + QtDownloadManager* q = toQtDownloadManager(clientInfo); > > manager? > > > Source/WebKit2/UIProcess/qt/QtDownloadManager.cpp:101 > > + QtDownloadManager* q = toQtDownloadManager(clientInfo); > > s/q/self/ ? that is what we use for EFL The goal is to mimic the effect of the Q_Q and Q_D macros, q for a main class, d if clientInfo is a private class. This is the convention we agreed on ATM. Created attachment 188523 [details]
Patch
Add some missing adoptWK(<WK*Copy*()>).get().
Also use the new WKURLCopyQString in QtWebError instead of doing WKStringCopyQString(WKURLCopyString(...)).
Comment on attachment 188523 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=188523&action=review LGTM > Source/WebKit2/UIProcess/qt/QtDownloadManager.cpp:63 > +void QtDownloadManager::didReceiveResponse(WKContextRef, WKDownloadRef download, WKURLResponseRef response, const void *clientInfo) * alignment > Source/WebKit2/UIProcess/qt/QtDownloadManager.cpp:79 > +void QtDownloadManager::didCreateDestination(WKContextRef, WKDownloadRef download, WKStringRef path, const void *clientInfo) same Comment on attachment 188523 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=188523&action=review LGTM, too > Source/WebKit2/UIProcess/qt/QtWebError.cpp:60 > - return WKStringCopyQString(WKURLCopyString(WKErrorCopyFailingURL(error.get()))); > + return WKURLCopyQString(adoptWK(WKErrorCopyFailingURL(error.get())).get()); Nice catch :) Comment on attachment 188523 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=188523&action=review Good for WebKit2. > Source/WebKit2/UIProcess/qt/QtDownloadManager.cpp:71 > + downloadItem->d->sourceUrl = WKURLCopyQUrl(adoptWK(WKURLResponseCopyURL(response)).get()); a new adoptToQURL? > Source/WebKit2/UIProcess/qt/QtDownloadManager.cpp:72 > + downloadItem->d->mimeType = WKStringCopyQString(adoptWK(WKURLResponseCopyMIMEType(response)).get()); adoptToQString? > Source/WebKit2/UIProcess/qt/QtDownloadManager.cpp:74 > + downloadItem->d->suggestedFilename = WKStringCopyQString(adoptWK(WKURLResponseCopySuggestedFilename(response)).get()); adoptToQString? > Source/WebKit2/UIProcess/qt/QtWebError.cpp:65 > - return WKStringCopyQString(WKErrorCopyLocalizedDescription(error.get())); > + return WKStringCopyQString(adoptWK(WKErrorCopyLocalizedDescription(error.get())).get()); adoptToQString instead of adoptWK().get()? Created attachment 192459 [details]
Patch
Added adoptToQURL(WKURLRef) and adoptToQString(WKURLRef).
Committed r145519: <http://trac.webkit.org/changeset/145519> |