Bug 177111 – [Curl] improve the implementation of FormDataStream

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Basuke Suzuki]

The implementation of FormDataStream in Curl port was old. Improve it using latest classes.

[Basuke Suzuki]

Created attachment 321156 [details]
Fix

[Alex Christensen]

Comment on attachment 321156 [details]
Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=321156&action=review

> Source/WebCore/platform/network/curl/ResourceHandleCurlDelegate.h:109
> +    RefPtr<FormDataStream> m_formDataStream { nullptr };

nullptr initializer isn't needed for RefPtr.
Couldn't you just use a std::unique_ptr instead of making FormDataStream refcounted?

[Basuke Suzuki]

(In reply to Alex Christensen from comment #2)
> Comment on attachment 321156 [details]
> Fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=321156&action=review
> 
> > Source/WebCore/platform/network/curl/ResourceHandleCurlDelegate.h:109
> > +    RefPtr<FormDataStream> m_formDataStream { nullptr };
> 
> nullptr initializer isn't needed for RefPtr.

right. Will do.

How do you think of integer value initialization? They are default to zero if empty braces are placed like pointers will be nullptr. For clarity { 0 } is much acceptable for me compared to { nullptr }, but either works for me. Sooner or later people get understand when they see:
   int foobar { };


> Couldn't you just use a std::unique_ptr instead of making FormDataStream
> refcounted?

Do you mean use std::unique_ptr even FormData is RefCounted object? If so I don't understand the point. If you mean changing the whole implementation, because it is shared among the ports, that won't be one day patch and had better to be separated.

[Alex Christensen]

FormDataStream is not refcounted, and might not need to be made refcounted like you did in this patch if you use std::unique_ptr instead of RefPtr.

int m_member { 0 }; is more explicit.  I'm not sure what people prefer.

[Alex Christensen]

Comment on attachment 321156 [details]
Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=321156&action=review

> Source/WebCore/platform/network/curl/FormDataStreamCurl.cpp:8
>   * All rights reserved.
> + * Copyright (C) 2017 Sony Interactive Entertainment Inc.

Copyright should probably go above the All rights reserved.

[Basuke Suzuki]

(In reply to Alex Christensen from comment #4)
> FormDataStream is not refcounted, and might not need to be made refcounted
> like you did in this patch if you use std::unique_ptr instead of RefPtr.

My apology I misunderstood you mentioned FromData, not FormDataStream. 

> int m_member { 0 }; is more explicit.  I'm not sure what people prefer.

Okay for me.

[Basuke Suzuki]

Created attachment 321295 [details]
fix

[WebKit Commit Bot]

Comment on attachment 321295 [details]
fix

Clearing flags on attachment: 321295

Committed r222269: <http://trac.webkit.org/changeset/222269>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/34693088>