Bug 199081 - Make HTTPCookieAcceptPolicy an enum class
Summary: Make HTTPCookieAcceptPolicy an enum class
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-06-20 14:38 PDT by Alex Christensen
Modified: 2019-06-25 14:09 PDT (History)
8 users (show)

See Also:


Attachments
Patch (15.43 KB, patch)
2019-06-20 14:42 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (22.67 KB, patch)
2019-06-20 14:57 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (22.69 KB, patch)
2019-06-20 15:12 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (22.77 KB, patch)
2019-06-25 11:16 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (24.21 KB, patch)
2019-06-25 13:09 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (24.21 KB, patch)
2019-06-25 13:41 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2019-06-20 14:38:23 PDT
Make HTTPCookieAcceptPolicy an enum class
Comment 1 Alex Christensen 2019-06-20 14:42:04 PDT
Created attachment 372585 [details]
Patch
Comment 2 Alex Christensen 2019-06-20 14:57:39 PDT
Created attachment 372588 [details]
Patch
Comment 3 Build Bot 2019-06-20 14:59:37 PDT
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
Comment 4 Alex Christensen 2019-06-20 15:12:16 PDT
Created attachment 372590 [details]
Patch
Comment 5 Geoffrey Garen 2019-06-21 09:46:18 PDT
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 **
Comment 6 Michael Catanzaro 2019-06-22 06:54:14 PDT
Alex, you might still be interested in commenting on bug #195140
Comment 7 Alex Christensen 2019-06-25 11:16:01 PDT
Created attachment 372847 [details]
Patch
Comment 8 Michael Catanzaro 2019-06-25 12:26:17 PDT
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.
Comment 9 Alex Christensen 2019-06-25 13:09:17 PDT
Created attachment 372855 [details]
Patch
Comment 10 Alex Christensen 2019-06-25 13:41:18 PDT
Created attachment 372857 [details]
Patch
Comment 11 WebKit Commit Bot 2019-06-25 14:08:29 PDT
Comment on attachment 372857 [details]
Patch

Clearing flags on attachment: 372857

Committed r246807: <https://trac.webkit.org/changeset/246807>
Comment 12 WebKit Commit Bot 2019-06-25 14:08:31 PDT
All reviewed patches have been landed.  Closing bug.
Comment 13 Radar WebKit Bug Importer 2019-06-25 14:09:35 PDT
<rdar://problem/52132986>