Bug 49063 – [Qt] WebKit2, QWKHistory needs more functionality and QWKHistoryItemPrivate should be shared object.

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

[Juha Savolainen]

History needs at least these methods:
- QWKHistoryItem currentItem() const
- QWKHistoryItem backItem() const
- QWKHistoryItem forwardItem() const
- QWKHistoryItem itemAtIndex(int index) const
- QList<QWKHistoryItem> backItems(int maxItems) const
- QList<QWKHistoryItem> forwardItems(int maxItems) const

Also QWKHistoryItemPrivate should be shared object, because
we cannot delete the QWKHistoryItemPrivate pointer in destructor of QWKHistoryItem,
there may have other history instances which share the same private implementation.

[Juha Savolainen]

Created attachment 73051 [details]
More functionality to history and QWKHistoryItemPrivate has changed to shared object

[Andreas Kling]

Comment on attachment 73051 [details]
More functionality to history and QWKHistoryItemPrivate has changed to shared object

View in context: https://bugs.webkit.org/attachment.cgi?id=73051&action=review

> WebKit2/UIProcess/API/qt/qwkhistory.cpp:39
>  #include <QUrl>
> +#include <QSharedData>
>  #include <WebKit2/WKRetainPtr.h>
> +#include <WebKit2/WKArray.h>

Include files should be in alphabetical order.

> WebKit2/UIProcess/API/qt/qwkhistory.cpp:52
> +QWKHistoryItem::QWKHistoryItem(const QWKHistoryItem &other)

Ampersand (&) placement, should be immediately after QWKHistoryItem.

> WebKit2/UIProcess/API/qt/qwkhistory.cpp:57
> +QWKHistoryItem& QWKHistoryItem::QWKHistoryItem::operator=(const QWKHistoryItem &other) 

Ditto.

> WebKit2/UIProcess/API/qt/qwkhistory.cpp:139
> +    QWKHistoryItem item;
> +    item.d = new QWKHistoryItemPrivate(itemRef.get());

This pattern is ugly, we should have the QWKHistoryItem constructor initialize its d-pointer instead.

> WebKit2/UIProcess/API/qt/qwkhistory.cpp:172
> +    for (int i = 0; i < size; i++) {

Use prefix increment (++i)

> WebKit2/UIProcess/API/qt/qwkhistory.cpp:187
> +    for (int i = 0; i < size; i++) {

Use prefix increment (++i)

> WebKit2/UIProcess/API/qt/qwkhistory.h:44
> +    QWKHistoryItem(const QWKHistoryItem &other);
> +    QWKHistoryItem &operator=(const QWKHistoryItem &other);

Ampersand (&) placement, should be immediately after QWKHistoryItem.

> WebKit2/UIProcess/API/qt/qwkhistory.h:69
> +    QWKHistoryItem itemAtIndex(int index) const;

Should be called "itemAt" to match the QWebHistory API.

> WebKit2/UIProcess/API/qt/qwkhistory_p.h:32
> +#include <QSharedData>

Include files should be in alphabetical order.

> WebKit2/UIProcess/API/qt/qwkhistory_p.h:45
> +    

Adding unnecessary whitespace.

[Juha Savolainen]

Created attachment 73228 [details]
fixed patch

[Andreas Kling]

Comment on attachment 73228 [details]
fixed patch

r=me

[WebKit Commit Bot]

Comment on attachment 73228 [details]
fixed patch

Clearing flags on attachment: 73228

Committed r71596: <http://trac.webkit.org/changeset/71596>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.