Use UniqueRef<> instead of std::unique_ptr<> when passing IPC::Encoder to sendMessage(), to make it clear it cannot be null.
Created attachment 422761 [details] Patch
Created attachment 422771 [details] Patch
Created attachment 422774 [details] Patch
Comment on attachment 422774 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=422774&action=review > Source/WebKit/GPUProcess/GPUConnectionToWebProcess.cpp:-637 > - replyEncoder.reset(); There are two of these. Why don't you remove both of them? > Source/WebKit/NetworkProcess/WebStorage/StorageManagerSet.h:75 > + bool didReceiveSyncMessage(IPC::Connection&, IPC::Decoder&, UniqueRef<IPC::Encoder>& replyEncoder); We should probably use UniqueRef<>&& and move the ownership. It would be worth putting in the change log that we need to return a boolean because we are not checking the null status of the unique_ptr any more.
Comment on attachment 422774 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=422774&action=review >> Source/WebKit/GPUProcess/GPUConnectionToWebProcess.cpp:-637 >> - replyEncoder.reset(); > > There are two of these. Why don't you remove both of them? Those of these? I don't see any other replyEncoder.reset() in this function. If there was, I don't think it would build because UniqueRef::reset() is not a think AFAIK. I must be misunderstanding your comment? >> Source/WebKit/NetworkProcess/WebStorage/StorageManagerSet.h:75 >> + bool didReceiveSyncMessage(IPC::Connection&, IPC::Decoder&, UniqueRef<IPC::Encoder>& replyEncoder); > > We should probably use UniqueRef<>&& and move the ownership. > It would be worth putting in the change log that we need to return a boolean because we are not checking the null status of the unique_ptr any more. I will clarify the ChangeLog. The result I used a reference instead of a rvalue-reference is that the call site would look really scary with a r-value reference: if (didReceiveSyncMessage(connection, decoder, WTFMove(encoder)) return; // Use encoder here. I get that it would work since WTFMove() is just a cast. But it looks scary so I am not sure I want to make that change. WDYT?
Comment on attachment 422774 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=422774&action=review >>> Source/WebKit/GPUProcess/GPUConnectionToWebProcess.cpp:-637 >>> - replyEncoder.reset(); >> >> There are two of these. Why don't you remove both of them? > > Those of these? I don't see any other replyEncoder.reset() in this function. If there was, I don't think it would build because UniqueRef::reset() is not a think AFAIK. > I must be misunderstanding your comment? The Messages::RemoteGraphicsContextGL::messageReceiverName code for decoders does something like this too. Never mind.
Created attachment 422780 [details] Patch
Created attachment 422781 [details] Patch
Committed r274189: <https://commits.webkit.org/r274189> All reviewed patches have been landed. Closing bug and clearing flags on attachment 422781 [details].
<rdar://problem/75244991>