[webkit-reviews] review denied: [Bug 62637] [Qt] [WK2] Qt WebKit2 needs undo/redo support : [Attachment 97145] fix patch 2

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Jun 14 15:36:50 PDT 2011


Kenneth Rohde Christiansen <kenneth at webkit.org> has denied Chang Shu
<cshu at webkit.org>'s request for review:
Bug 62637: [Qt] [WK2] Qt WebKit2 needs undo/redo support
https://bugs.webkit.org/show_bug.cgi?id=62637

Attachment 97145: fix patch 2
https://bugs.webkit.org/attachment.cgi?id=97145&action=review

------- Additional Comments from Kenneth Rohde Christiansen
<kenneth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=97145&action=review

> Source/WebKit2/UIProcess/API/qt/qwkpage.cpp:116
> +    delete undoStack;

I agree that we should start using smart pointers. OwnPtr works quite well with
Qt types, just like QScopedPointer. delete should be a thing of the past or at
least you should explain why a smart pointer cannot be used

> Source/WebKit2/UIProcess/API/qt/qwkpage.cpp:226
> +	   undoStack->push(new WebUndoCommandQt(command));

Who are freeing these?

> Source/WebKit2/UIProcess/API/qt/qwkpage.cpp:234
> +    undoStack->clear();

Will this free everything in the stack? That is not normal behavior in webcore
afaik so we might need a comment if that is Qt behavior

> Source/WebKit2/UIProcess/qt/WebUndoCommandQt.h:49
> +	   bool m_inUndoRedo; // our undo stack works differently - don't
re-enter!

Our*

Maybe a better variable name can be found.


More information about the webkit-reviews mailing list