WebKit2-qt needs history, which represents the history of a QWKPage and makes it possible to navigate it.
Created attachment 70844 [details] Initial implementation of QWKHistory. This is initial implementation of QWKHistory. The QWKHistory class represents the history of a QWKPage. More functionality(including QWKForwardListItem) will be added if this feature/patch is accepted. All comments/suggestions are welcome.
Comment on attachment 70844 [details] Initial implementation of QWKHistory. Obsoleted. EWS failed to run.
Created attachment 70853 [details] Initial implementation of QWKHistory. Just fixed directories in patch so EWS could run this.
Attachment 70853 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebKit2/UIProcess/API/qt/qwkhistory.cpp:25: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] WebKit2/UIProcess/API/qt/qwkhistory.cpp:27: Alphabetical sorting problem. [build/include_order] [4] WebKit2/UIProcess/API/qt/qwkhistory.h:25: #ifndef header guard has wrong style, please use: qwkhistory_h [build/header_guard] [5] Total errors found: 3 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 70853 [details] Initial implementation of QWKHistory. View in context: https://bugs.webkit.org/attachment.cgi?id=70853&action=review > WebKit2/UIProcess/API/qt/qwkhistory.cpp:31 > +QWKHistory::QWKHistory(WebBackForwardList* list) Doesn't look right from an ABI perspective... add a constructor to the private QWKHistoryPrivate::create(WebBackForwardList* list) or so > WebKit2/UIProcess/API/qt/qwkhistory.cpp:32 > +: m_backForwardList(list) needs indentaton > WebKit2/UIProcess/API/qt/qwkhistory.cpp:52 > + return backListCount()+forwardListCount(); needs spaces around +
Created attachment 71013 [details] patch Changes after Kenneth's comments. Created QWKHistoryPrivate-class which have one static function to create QWKHistory.
Comment on attachment 71013 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=71013&action=review > WebKit2/UIProcess/API/qt/qwkhistory.cpp:39 > +QWKHistory::QWKHistory(WebKit::WebBackForwardList* list) This is not BC compatible. Our public classes (even in private: section) cannot refer to WebCore / WebKit internals > WebKit2/UIProcess/API/qt/qwkhistory.h:49 > + WebKit::WebBackForwardList* m_backForwardList; This class should take a QWKHistoryPrivate* d pointer, and the WebKit::WebBackForwardList* should be inside that private class.
Created attachment 71146 [details] patch Thank you Kenneth for a informative comments. Should we make methods also available to QWKHistoryPrivate, eg. int QWKHistoryPrivate::backListCount() const { return WKBackForwardListGetBackListCount(toAPI(m_backForwardList)); }
Comment on attachment 71146 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=71146&action=review Almost there! > WebKit2/UIProcess/API/qt/qwkhistory.h:45 > + QWKHistory(QWKHistoryPrivate* historyPrivate); Please leave this as QWKHistory(); Just make it a friend and you can set the d pointer manually. > WebKit2/UIProcess/API/qt/qwkpage.cpp:486 > + return d->m_history; in privates we do not add m_. > WebKit2/UIProcess/API/qt/qwkpage_p.h:88 > + QWKHistory* m_history; make this QWKHistory* history;
(In reply to comment #8) > Created an attachment (id=71146) [details] > patch > > Thank you Kenneth for a informative comments. > > Should we make methods also available to QWKHistoryPrivate, eg. > int QWKHistoryPrivate::backListCount() const > { > return WKBackForwardListGetBackListCount(toAPI(m_backForwardList)); > } That is not really needed, we only do that for helper methods.
(In reply to comment #9) > Almost there! So near, so near, I can almost feel it! > > WebKit2/UIProcess/API/qt/qwkhistory.h:45 > > + QWKHistory(QWKHistoryPrivate* historyPrivate); > > Please leave this as QWKHistory(); Just make it a friend and you can set the d pointer manually. Just one clarification, do you mean that we set d pointer from QWKPagePrivate or QWKHistoryPrivate::createHistory? I quess in QWKHistoryPrivate.
(In reply to comment #11) > (In reply to comment #9) > > Almost there! > > So near, so near, I can almost feel it! > > > > WebKit2/UIProcess/API/qt/qwkhistory.h:45 > > > + QWKHistory(QWKHistoryPrivate* historyPrivate); > > > > Please leave this as QWKHistory(); Just make it a friend and you can set the d pointer manually. > > Just one clarification, do you mean that we set d pointer from QWKPagePrivate or QWKHistoryPrivate::createHistory? I quess in QWKHistoryPrivate. The latter ofcourse :-) Just take a look at WebKit/qt/Api - I'm sure we do this various places :-)
Created attachment 71307 [details] patch fixed patch.
Comment on attachment 71307 [details] patch Great, keep up the good work!
The commit-queue encountered the following flaky tests while processing attachment 71307 [details]: http/tests/navigation/post-goback-same-url.html java/lc3/JSObject/ToObject-001.html Please file bugs against the tests. The author(s) of the test(s) have been CCed on this bug. The commit-queue is continuing to process your patch.
Comment on attachment 71307 [details] patch Clearing flags on attachment: 71307 Committed r70218: <http://trac.webkit.org/changeset/70218>
All reviewed patches have been landed. Closing bug.