Summary: | [Qt] WebKit2 needs to support historyitem, QWKHistoryItem | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Juha Savolainen <juha.savolainen> | ||||||
Component: | WebKit2 | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, kenneth, kling | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Attachments: |
|
Description
Juha Savolainen
2010-10-29 01:40:00 PDT
Created attachment 72299 [details]
Implementation of QWKHistoryItem
Comment on attachment 72299 [details] Implementation of QWKHistoryItem View in context: https://bugs.webkit.org/attachment.cgi?id=72299&action=review > WebKit2/UIProcess/API/qt/qwkhistory.cpp:46 > +QWKHistoryItemPrivate::~QWKHistoryItemPrivate() > +{ > +} Why not just use the default destructor. > WebKit2/UIProcess/API/qt/qwkhistory.cpp:59 > +QString QWKHistoryItem::title() const > +{ > + return WKStringCopyQString(WKBackForwardListItemCopyTitle(d->m_backForwardListItem)); > +} I think that you are leaking here. As you are not freeing the WKString. Do something like: WKRetainPtr<WKString> string = WKBackForwardListItemCopyTitle(d->m_backForwardListItem); return WKStringCopyQString(string); When the WKRetainPtr goes out of scope, it will free the string. > WebKit2/UIProcess/API/qt/qwkhistory_p.h:43 > + WKBackForwardListItemRef m_backForwardListItem; Don't you need to ref this one? ie. change this into a WKRetainPtr<WKBackForwardListItemRef>? (In reply to comment #2) > (From update of attachment 72299 [details]) > > WebKit2/UIProcess/API/qt/qwkhistory.cpp:59 > > +QString QWKHistoryItem::title() const > > +{ > > + return WKStringCopyQString(WKBackForwardListItemCopyTitle(d->m_backForwardListItem)); > > +} > > I think that you are leaking here. As you are not freeing the WKString. Also I noticed that I forgot to do checkings: if (!d->m_backForwardListItem) return QString(); Created attachment 72501 [details]
Fixed patch
Fixed patch.
The commit-queue encountered the following flaky tests while processing attachment 72501 [details]: media/event-attributes.html Please file bugs against the tests. These tests were authored by eric.carlson@apple.com. The commit-queue is continuing to process your patch. Comment on attachment 72501 [details] Fixed patch Clearing flags on attachment: 72501 Committed r71089: <http://trac.webkit.org/changeset/71089> All reviewed patches have been landed. Closing bug. |