Bug 113821

Summary: [GTK] Make libsecret optional
Product: WebKit Reporter: Martin Robinson <mrobinson>
Component: WebKitGTKAssignee: Martin Robinson <mrobinson>
Status: RESOLVED FIXED    
Severity: Normal CC: cgarcia, gustavo
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch gustavo: review+

Description Martin Robinson 2013-04-02 12:56:50 PDT
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.
Comment 1 Martin Robinson 2013-04-02 13:10:49 PDT
Created attachment 196215 [details]
Patch
Comment 2 Gustavo Noronha (kov) 2013-04-02 13:18:01 PDT
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 =).
Comment 3 Martin Robinson 2013-04-02 14:22:37 PDT
Committed r147499: <http://trac.webkit.org/changeset/147499>
Comment 4 Carlos Garcia Campos 2013-04-02 23:53:32 PDT
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.
Comment 5 Carlos Garcia Campos 2013-04-15 02:31:52 PDT
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)