Bug 48620 – [Qt] WebKit2 needs to support historyitem, QWKHistoryItem

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Juha Savolainen]

The QWKHistoryItem class represents one item in the history of a QWKPage.

[Juha Savolainen]

Created attachment 72299 [details]
Implementation of QWKHistoryItem

[Kenneth Rohde Christiansen]

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>?

[Juha Savolainen]

(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();

[Juha Savolainen]

Created attachment 72501 [details]
Fixed patch

Fixed patch.

[WebKit Commit Bot]

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 [e-mail].  The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 72501 [details]
Fixed patch

Clearing flags on attachment: 72501

Committed r71089: <http://trac.webkit.org/changeset/71089>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.