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.
Created attachment 73051 [details] More functionality to history and QWKHistoryItemPrivate has changed to shared object
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.
Created attachment 73228 [details] fixed patch
Comment on attachment 73228 [details] fixed patch r=me
Comment on attachment 73228 [details] fixed patch Clearing flags on attachment: 73228 Committed r71596: <http://trac.webkit.org/changeset/71596>
All reviewed patches have been landed. Closing bug.