Bug 113821 - [GTK] Make libsecret optional
Summary: [GTK] Make libsecret optional
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Martin Robinson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-04-02 12:56 PDT by Martin Robinson
Modified: 2013-04-15 02:31 PDT (History)
2 users (show)

See Also:


Attachments
Patch (9.33 KB, patch)
2013-04-02 13:10 PDT, Martin Robinson
gustavo: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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)