[webkit-reviews] review granted: [Bug 25294] All WebKit/win classes should return COMPtrs from their static constructor members : [Attachment 29616] patch for MemoryStream

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Adam Roben (aroben)
<aroben at apple.com>'s request for review:
Bug 25294: All WebKit/win classes should return COMPtrs from their static
constructor members
https://bugs.webkit.org/show_bug.cgi?id=25294

Attachment 29616: patch for MemoryStream
https://bugs.webkit.org/attachment.cgi?id=29616&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
>  HRESULT STDMETHODCALLTYPE MemoryStream::Clone( 
>      /* [out] */ IStream** ppstm)
>  {
> -    *ppstm = MemoryStream::createInstance(m_buffer);
> +    *ppstm = MemoryStream::createInstance(m_buffer).releaseRef();
> +    // FIXME: MSDN says we should be returning STG_E_INSUFFICIENT_MEMORY
instead of E_OUTOFMEMORY here.
>      return (*ppstm) ? S_OK : E_OUTOFMEMORY;

Why not copyRefTo here?

> -    COMPtr<IStream> stream(AdoptCOM,
MemoryStream::createInstance(buffer.release()));
> +    COMPtr<MemoryStream> stream =
MemoryStream::createInstance(buffer.release());
>      m_view->didReceiveData(stream.get());

Would this read better without the local variable?

> -    COMPtr<MemoryStream> result(AdoptCOM,
MemoryStream::createInstance(SharedBuffer::wrapCFData(historyData.get())));
> +    COMPtr<MemoryStream> result =
MemoryStream::createInstance(SharedBuffer::wrapCFData(historyData.get()));
>      return result.copyRefTo(stream);

Would this read better without the local variable?

r=me