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

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=49063


Andreas Kling <kling at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #73051|review?, commit-queue?      |review-
               Flag|                            |




--- Comment #2 from Andreas Kling <kling at webkit.org>  2010-11-07 19:06:16 PST ---
(From update of attachment 73051)
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.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.