[webkit-reviews] review requested: [Bug 47345] [Qt] Implement SharedMemory for WebKit2 : [Attachment 71722] proposed patch

[bugzilla-daemon at webkit.org]

Zoltan Horvath <zoltan at webkit.org> has asked  for review:
Bug 47345: [Qt] Implement SharedMemory for WebKit2
https://bugs.webkit.org/show_bug.cgi?id=47345

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

------- Additional Comments from Zoltan Horvath <zoltan at webkit.org>
(In reply to comment #16)
> (From update of attachment 70871 [details])
> 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

I renamed MappedMemory.h to MappedMemoryPool.h.

> - MappedMemory.h vs. impl in MappedMemoryPool.cpp

I put MappedMemory implementation into MappedMemoryPool.

> - Would it reduce confusion to move MappedMemory code to SharedMemoryQt.cpp
and make UpdateChunk use SharedMemory?

I didn't do this. In this mod, UpdateChunk uses MappedMemoryPool.

> - 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.

I put mapFile/mapMemory into MappedMemoryPool and removed unnecessary methods.

> - Nokia copyrights should be maintained when copying code to new files

Done.