Bug 158887 – Use WTF::NoncopyableFunction in NetworkDataTaskClient

• Component: WebKit2
• Status: RESOLVED FIXED

[Chris Dumez]

Use WTF::NoncopyableFunction in NetworkDataTaskClient instead of std::function and consistently move it around. This avoids some unnecessary copying.

[Chris Dumez]

Created attachment 281587 [details]
Patch

[Alex Christensen]

Comment on attachment 281587 [details]
Patch

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

R=me

> Source/WebKit2/Shared/Authentication/mac/AuthenticationManager.mac.mm:69
> +bool AuthenticationManager::tryUseCertificateInfoForChallenge(const AuthenticationChallenge& challenge, const CertificateInfo& certificateInfo, const ChallengeCompletionHandler& completionHandler)

It would be cool if we could make a function pointer that can only be called once and then becomes null, because that is the intent of this callback.
Instead of returning bool, we could take a ChallengeCompletionHandler&& and return nullptr if it was used and return the completion handler if it wasn't used.

[Chris Dumez]

(In reply to comment #2)
> Comment on attachment 281587 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=281587&action=review
> 
> R=me
> 
> > Source/WebKit2/Shared/Authentication/mac/AuthenticationManager.mac.mm:69
> > +bool AuthenticationManager::tryUseCertificateInfoForChallenge(const AuthenticationChallenge& challenge, const CertificateInfo& certificateInfo, const ChallengeCompletionHandler& completionHandler)
> 
> It would be cool if we could make a function pointer that can only be called
> once and then becomes null, because that is the intent of this callback.
> Instead of returning bool, we could take a ChallengeCompletionHandler&& and
> return nullptr if it was used and return the completion handler if it wasn't
> used.

We may need to do this in cases where the implementation needs to take ownership of the NoncopyableFunction in some cases. However, in this case, it does not and therefore passing as a const reference works and is more readable IMHO.

I would prefer to leave as is.

[WebKit Commit Bot]

Comment on attachment 281587 [details]
Patch

Clearing flags on attachment: 281587

Committed r202184: <http://trac.webkit.org/changeset/202184>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.