See discussion in bug #144046. It ensures that the file descriptor ownership is always correctly transfered. This way we can remove the dispose() method to explicitly close the file descriptor and always close it in the Attachment destructor (unless explicitly transferred to IPC::Connection or SharedMemory). It simplifies the code and ensure we don't leak file descriptors.
Created attachment 251427 [details] Patch
Comment on attachment 251427 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=251427&action=review This is OK, but could be improved. > Source/WebKit2/Platform/IPC/ArgumentDecoder.cpp:217 > + attachment = WTF::move(m_attachments.last()); > m_attachments.removeLast(); Should instead be: attachment = m_attachments.takeLast(); > Source/WebKit2/Platform/IPC/Attachment.cpp:59 > + encoder.addAttachment(Attachment(*this)); > +#if USE(UNIX_DOMAIN_SOCKETS) > + // The encoder takes the onwership of our file descriptor. > + m_fileDescriptor = -1; > +#endif Should instead be: encoding.addAttachment(WTF::move(*this)); > Source/WebKit2/Platform/IPC/Attachment.h:85 > + Attachment(const Attachment&) = default; > + Attachment& operator=(Attachment&) = default; Why default? Why not delete? I think this is only needed because of the incorrect code in Attachment::encode.
Comment on attachment 251427 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=251427&action=review >> Source/WebKit2/Platform/IPC/Attachment.cpp:59 >> +#endif > > Should instead be: > > encoding.addAttachment(WTF::move(*this)); Might require const_cast. Itβs a design problem, I think, that encoding modifies the attachment, but the encode function is marked const.
Yes, I didn't know how to do the move(this), but this: encoder.addAttachment(WTF::move(*const_cast<Attachment*>(this))); made the trick :-) Thanks!
I don't think we need to use = delete, because that's the default when the move constructor and assignment operation are defined.
And we don't need to make the file descriptor mutable either
Created attachment 251437 [details] Patch for landing
Committed r183189: <http://trac.webkit.org/changeset/183189>