[webkit-reviews] review granted: [Bug 135327]=?UTF-8?Q?=20Server=20trust=20authentication=20challenges=20aren=E2=80=99t=20sent=20to=20the=20navigation=20delegate=20?=: [Attachment 235587] I. Introduce CredentialBase and make Credential derive from it

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted mitz at webkit.org <mitz at webkit.org>'s
request for review:
Bug 135327: Server trust authentication challenges aren’t sent to the
navigation delegate
https://bugs.webkit.org/show_bug.cgi?id=135327

Attachment 235587: I. Introduce CredentialBase and make Credential derive from
it
https://bugs.webkit.org/attachment.cgi?id=235587&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=235587&action=review


r=me -- the comments I made are sort of “side notes”; not all necessarily
relevant to this file refactoring

> Source/WebCore/platform/network/CredentialBase.cpp:26
>  #include "config.h"
> +#include "CredentialBase.h"

Should add a blank line before the first include.

> Source/WebCore/platform/network/CredentialBase.cpp:36
>      : m_user("")
>      , m_password("")

Should use the more efficient emptyString() some day if not now.

> Source/WebCore/platform/network/CredentialBase.cpp:48
>      : m_user(user.length() ? user : "")
>      , m_password(password.length() ? password : "")

Should use the more efficient emptyString() some day if not now.

> Source/WebCore/platform/network/CredentialBase.h:25
> + */
> +#ifndef Credential_h

Should add a blank line before the ifndef.

> Source/WebCore/platform/network/CredentialBase.h:30
> +#define CERTIFICATE_CREDENTIALS_SUPPORTED (PLATFORM(COCOA))

The name of this conditional is pretty strange. The feature in question
involves interface that is done with platform-specific types, but yet this has
a name that sounds like an abstract concept that would not be
CoreFoundation-specific.

> Source/WebCore/platform/network/CredentialBase.h:25
>   */
> -#ifndef Credential_h
> -#define Credential_h
> +#ifndef CredentialBase_h

Should add a blank line before the #ifndef.