[webkit-reviews] review denied: [Bug 47345] [Qt] Implement SharedMemory for WebKit2 : [Attachment 70871] svn patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Oct 19 06:16:30 PDT 2010


Antti Koivisto <koivisto at iki.fi> has denied Zoltan Horvath
<zoltan at webkit.org>'s request for review:
Bug 47345: [Qt] Implement SharedMemory for WebKit2
https://bugs.webkit.org/show_bug.cgi?id=47345

Attachment 70871: svn patch
https://bugs.webkit.org/attachment.cgi?id=70871&action=review

------- Additional Comments from Antti Koivisto <koivisto at iki.fi>
We discussed this on irc. The direction looks good but I would prefer a patch
with more completed refactoring.
 
- MappedMemory.h has two classes and it is not clear which interfaces are
public
- MappedMemory.h vs. impl in MappedMemoryPool.cpp
- Would it reduce confusion to move MappedMemory code to SharedMemoryQt.cpp and
make UpdateChunk use SharedMemory?
- as is, mapFile/mapMemory should be declared in header, not in .cpp
- mapFile/mapMemory should probably be in class scope
- MappedMemoryPool::cleanUp() (and possibly the whole class) might be
unnecessary since we unlink the shared memory files right after mapping.
- Nokia copyrights should be maintained when copying code to new files


More information about the webkit-reviews mailing list