Bug 113821 – [GTK] Make libsecret optional

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Martin Robinson]

I believe that before I was hoping that libsecret would be ported to Windows by the time 2.0 was release. It seems that libsecret isn't an option on Windows, so we need to make the dependency optional.

[Martin Robinson]

Created attachment 196215 [details]
Patch

[Gustavo Noronha (kov)]

Comment on attachment 196215 [details]
Patch

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

> Source/autotools/ReadCommandLineArguments.m4:69
> +    [AS_HELP_STRING([--enable-credential-storage],[enable support for credential storage (introduces a dependency on libsecret)])],

Missing information on the default. Are you using 'credential-storage-' to get the option automatically mapped from build-webkit? Otherwise I think it may be simpler/easier to understand to use enable-libsecret, I trust your call, anyway =).

[Martin Robinson]

Committed r147499: <http://trac.webkit.org/changeset/147499>

[Carlos Garcia Campos]

Comment on attachment 196215 [details]
Patch

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

> Source/WebCore/platform/network/gtk/CredentialBackingStore.cpp:103
>  void CredentialBackingStore::credentialForChallenge(const AuthenticationChallenge& challenge, CredentialForChallengeCallback callback, void* data)

Have you checked this builds without credential storage? AuthenticationChallenge.h is only included when credential storage is enabled.

> Source/WebCore/platform/network/gtk/CredentialBackingStore.cpp:121
> +    callback(Credential(), data);

You should probably add here unused param macro to avoid compile warnings when building without libsecret.

> Source/WebCore/platform/network/gtk/CredentialBackingStore.cpp:141
> +#endif // ENABLE(CREDENTIAL_STORAGE)

Ditto.

[Carlos Garcia Campos]

Comment on attachment 196215 [details]
Patch

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

This patch broke credential storage. I've landed it fixed in the stable branch, but needs to be fixed in master.

>> Source/WebCore/platform/network/gtk/CredentialBackingStore.cpp:103
>>  void CredentialBackingStore::credentialForChallenge(const AuthenticationChallenge& challenge, CredentialForChallengeCallback callback, void* data)
> 
> Have you checked this builds without credential storage? AuthenticationChallenge.h is only included when credential storage is enabled.

Now we know it builds without credential storage, since we are indeed building without it :-P

> Source/autotools/SetupAutoconfHeader.m4:119
> +    AC_DEFINE([WTF_ENABLE_CREDENTIAL_STORAGE], [1], [ ])

This should be ENABLE_CREDENTIAL_STORAGE, see ENABLE macro definition:

#define ENABLE(WTF_FEATURE) (defined ENABLE_##WTF_FEATURE  && ENABLE_##WTF_FEATURE)