Bug 176910 – [Curl] Create a class dedicated to handle SSL related task

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Basuke Suzuki]

Create SSLSettings/SSLVerifier classes to gather SSL related processes in one place.
And fixed file name from SSLHandle to CurlSSLHandle for classification.

[Basuke Suzuki]

Created attachment 320772 [details]
patch

[Build Bot]

Attachment 320772 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/WebCore/ChangeLog:9:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 2 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Basuke Suzuki]

Created attachment 320827 [details]
fix

[Basuke Suzuki]

Created attachment 320829 [details]
fix

[Alex Christensen]

Comment on attachment 320829 [details]
fix

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

> Source/WebCore/platform/network/curl/CurlSSLHandle.cppSource/WebCore/platform/network/curl/SSLHandle.cpp:146
> +// CurlSSLVeritifer -------------------------------------------------

I don't think comments like this are helpful.  This one's spelled wrong.

> Source/WebCore/platform/network/curl/CurlSSLHandle.h:45
> +class CurlSSLSettings {

We should have one class per file.  This file contains no class called CURLSSLHandle.

[Basuke Suzuki]

Created attachment 320952 [details]
fixed

[Basuke Suzuki]

Fixed. Separate files, use proper name for classes and remove unnecessary comments. Also reduce usage of singleton(), but put them into CurlContext.

[Alex Christensen]

Comment on attachment 320952 [details]
fixed

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

> Source/WebCore/platform/network/curl/CurlSSLHandle.cpp:103
> +        auto valueIter = (it->value).begin();
> +        for (; valueIter != (it->value).end(); ++valueIter, ++certsIter) {

it->value should at least be given a name.  This loop can be made to be much more clear.

> Source/WebCore/platform/network/curl/CurlSSLVerifier.h:91
> +

This is a lot of whitespace.  Let's not.

[Basuke Suzuki]

Created attachment 320967 [details]
fixed

[Basuke Suzuki]

(In reply to Alex Christensen from comment #8)
> Comment on attachment 320952 [details]
> fixed
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=320952&action=review
> 
> > Source/WebCore/platform/network/curl/CurlSSLHandle.cpp:103
> > +        auto valueIter = (it->value).begin();
> > +        for (; valueIter != (it->value).end(); ++valueIter, ++certsIter) {
> 
> it->value should at least be given a name.  This loop can be made to be much
> more clear.

Replaced with std::equal()

> > Source/WebCore/platform/network/curl/CurlSSLVerifier.h:91
> > +
> 
> This is a lot of whitespace.  Let's not.

removed.

[WebKit Commit Bot]

Comment on attachment 320967 [details]
fixed

Clearing flags on attachment: 320967

Committed r222147: <http://trac.webkit.org/changeset/222147>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/34693307>