Bug 199081 – Make HTTPCookieAcceptPolicy an enum class

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Alex Christensen]

Make HTTPCookieAcceptPolicy an enum class

[Alex Christensen]

Created attachment 372585 [details]
Patch

[Alex Christensen]

Created attachment 372588 [details]
Patch

[EWS Watchlist]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Alex Christensen]

Created attachment 372590 [details]
Patch

[Geoffrey Garen]

In file included from /Volumes/Data/EWS/WebKit/Source/WebKit/UIProcess/API/APIWebsiteDataStore.h:28:
In file included from /Volumes/Data/EWS/WebKit/Source/WebKit/UIProcess/API/APIHTTPCookieStore.h:29:
/Volumes/Data/EWS/WebKit/Source/WebKit/Shared/HTTPCookieAcceptPolicy.h:44:41: error: no member named 'Always_' in 'WebKit::HTTPCookieAcceptPolicy'
        WebKit::HTTPCookieAcceptPolicy::Always_,
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
In file included from /Volumes/Data/EWS/WebKit/WebKitBuild/Debug/DerivedSources/WebKit2/unified-sources/UnifiedSource20.cpp:1:
In file included from /Volumes/Data/EWS/WebKit/Source/WebKit/Shared/API/APIURLRequest.cpp:30:
In file included from /Volumes/Data/EWS/WebKit/Source/WebKit/UIProcess/WebProcessPool.h:33:
In file included from /Volumes/Data/EWS/WebKit/Source/WebKit/UIProcess/GenericCallback.h:33:
/Volumes/Data/EWS/WebKit/Source/WebKit/UIProcess/API/C/WKAPICast.h:370:40: error: no member named 'Always_' in 'WebKit::HTTPCookieAcceptPolicy'
        return HTTPCookieAcceptPolicy::Always_;
               ~~~~~~~~~~~~~~~~~~~~~~~~^
/Volumes/Data/EWS/WebKit/Source/WebKit/UIProcess/API/C/WKAPICast.h:380:36: error: no member named 'Always_' in 'WebKit::HTTPCookieAcceptPolicy'
    return HTTPCookieAcceptPolicy::Always_;
           ~~~~~~~~~~~~~~~~~~~~~~~~^
/Volumes/Data/EWS/WebKit/Source/WebKit/UIProcess/API/C/WKAPICast.h:386:34: error: no member named 'Always_' in 'WebKit::HTTPCookieAcceptPolicy'
    case HTTPCookieAcceptPolicy::Always_:
         ~~~~~~~~~~~~~~~~~~~~~~~~^
/Volumes/Data/EWS/WebKit/Source/WebKit/UIProcess/API/C/WKAPICast.h:385:13: error: enumeration value 'AlwaysAccept' not handled in switch [-Werror,-Wswitch]
    switch (policy) {
            ^
/Volumes/Data/EWS/WebKit/Source/WebKit/UIProcess/API/C/WKAPICast.h:385:13: note: add missing switch cases
    switch (policy) {
            ^

** BUILD FAILED **

[Michael Catanzaro]

Alex, you might still be interested in commenting on bug #195140

[Alex Christensen]

Created attachment 372847 [details]
Patch

[Michael Catanzaro]

Comment on attachment 372847 [details]
Patch

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

> Source/WebKit/ChangeLog:10
> +        WKPreferencesGetIncrementalRenderingSuppressionTimeout was using its toAPI function to convert a double to a double because HTTPCookieAcceptPolicy used to be an unsigned integer.
> +        toAPI(WebCore::MouseButton) was also using the toAPI(HTTPCookieAcceptPolicy) because HTTPCookieAcceptPolicy used to be an unsigned integer.

Wow

> Source/WebKit/NetworkProcess/Cookies/curl/WebCookieManagerCurl.cpp:49
> -    case HTTPCookieAcceptPolicyOnlyFromMainDocumentDomain:
> +    case HTTPCookieAcceptPolicy::OnlyFromMainDocumentDomain:
>          curlPolicy = CookieAcceptPolicy::OnlyFromMainDocumentDomain;
>          break;
> -    case HTTPCookieAcceptPolicyExclusivelyFromMainDocumentDomain:
> +    case HTTPCookieAcceptPolicy::ExclusivelyFromMainDocumentDomain:

Of course these names are nonsense and now would be a good time to rename them (discussed in bug #193458).

> Source/WebKit/NetworkProcess/Cookies/mac/WebCookieManagerMac.mm:40
> +    return static_cast<CFHTTPCookieStorageAcceptPolicy>(policy);

Seems fragile... we won't think to update this if we reorder the members of HTTPCookieAcceptPolicy, for instance. A switch would be safer, like you used below.

> Source/WebKit/UIProcess/WebCookieManagerProxy.h:81
> +    void setHTTPCookieAcceptPolicySynchronouslyForTesting(PAL::SessionID, HTTPCookieAcceptPolicy);

Oops, looks like this was WIP and should be deleted before landing? It's not implemented.

[Alex Christensen]

Created attachment 372855 [details]
Patch

[Alex Christensen]

Created attachment 372857 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 372857 [details]
Patch

Clearing flags on attachment: 372857

Committed r246807: <https://trac.webkit.org/changeset/246807>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/52132986>