Bug 55875

Created attachment 89987 [details] Native Symbian IPC implementation

I split out shared memory changes from Bug 55877 to simplify life.

Created attachment 89991 [details] Updated patch

Comment on attachment 89991 [details] Updated patch View in context: https://bugs.webkit.org/attachment.cgi?id=89991&action=review Shou;dn't we make this work in all cases before committing it? > Source/WebKit2/Platform/SharedMemory.h:76 > + mutable uint32_t m_name; im not sure this is a good variable name. > Source/WebKit2/Platform/qt/SharedMemorySymbian.cpp:38 > +_LIT(KFormatToString, "%d"); What does this mean? > Source/WebKit2/Platform/qt/SharedMemorySymbian.cpp:41 > + : m_name(0), m_size(0) We normally put these on separate lines > Source/WebKit2/Platform/qt/SharedMemorySymbian.cpp:58 > + // name of the global chunk (masquarading as int32_t for ease of serialization) isn't masquerading spelled wrongly. > Source/WebKit2/Platform/qt/SharedMemorySymbian.cpp:71 > + uint32_t name; > + if (!decoder->decodeUInt32(name)) > + return false; > + indentation issue here > Source/WebKit2/Platform/qt/SharedMemorySymbian.cpp:77 > +// Create a new shared memory segment useless comment :) the method already has a descriptive name > Source/WebKit2/Platform/qt/SharedMemorySymbian.cpp:81 > + // On Symbian, global chunks (shared memory segments) have system-unique names, so we pick a random > + // number from the kernel's random pool and use it as a string. Are you sure these will never be reused by the kernel? > Source/WebKit2/Platform/qt/SharedMemorySymbian.cpp:102 > +// Take a shared memory handle given usually by another process, and open it locally for read or write > +PassRefPtr<SharedMemory> SharedMemory::create(const Handle& handle, Protection protection) Maybe rename the method? or is it a global one being implemented here? > Source/WebKit2/Platform/qt/SharedMemorySymbian.cpp:146 > + // convert the name (string form) to uint32_t Please use proper sentences (start with capital, ends with punctuation mark of some kind)

(In reply to comment #4) > (From update of attachment 89991 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=89991&action=review > > Shou;dn't we make this work in all cases before committing it? > Since you as a reviewer appear to be OK with everything being together in one (working on Symbian) patch, I am removing the Shared Memory implementation from here, and including all deltas in one patch in Bug 55877. :)

(In reply to comment #5) > (In reply to comment #4) > > (From update of attachment 89991 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=89991&action=review > > > > Shou;dn't we make this work in all cases before committing it? > > > > Since you as a reviewer appear to be OK with everything being together in one (working on Symbian) patch, I am removing the Shared Memory implementation from here, and including all deltas in one patch in Bug 55877. :) I asked this because according to the comments in the patch there are open issues

Created attachment 92256 [details] Updated patch with only SharedMemory impl for Symbian This patch has addressed most feedback from Kenneth in this bug and from Balazs in Bug 55877. It's ready for review from my side. Kenneth: I did discuss the unresolved issue of when to Close() the chunk (mentioned in the Changelog and in the SharedMemory d'tor) with other Symbian and Qt gurus, and there are no clear solutions since Symbian client-server IPC is not being used for this patch. I chose to use the less pure half-duplex pipes here as they fit better with how other OSes do the WebKit2's IPC setup, process launching, ownership transfer of the endpoint and read/writes. If we decide to productize the Symbian port later, we can revisit the leaks.

Attachment 92256 [details] did not build on qt: Build output: http://queues.webkit.org/results/8556566

Created attachment 92274 [details] Updated patch Add edited Platform.h which excludes Symbian from USE(UNIX_DOMAIN_SOCKETS) flag.

Comment on attachment 92274 [details] Updated patch Looks good to me as one step towards porting WebKit2 to Symbian. As other pieces of the port get implemented, the SharedMemory destructor needs to be revisited.

Comment on attachment 92274 [details] Updated patch Clearing flags on attachment: 92274 Committed r86560: <http://trac.webkit.org/changeset/86560>

All reviewed patches have been landed. Closing bug.