Bug 169486 – Cleanup "addCookie" and its sole user

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Brady Eidson]

Cleanup "addCookie" and its sole user

addCookie doesn't cleanly fit in with the native cookie storage API, and a lot of dead code was added to support it as well.

Refactoring this to be "setCookies" like NSHTTPCookieStorage supports will be better going forward.

[Brady Eidson]

Created attachment 304098 [details]
Patch

[Brady Eidson]

Already reviewed. Waiting for EWS.

[Brady Eidson]

Created attachment 304117 [details]
Patch

[Brady Eidson]

Attempt to fix GTK, and hopefully the rest of the EWS bots will actually be responsive this time.

[Brady Eidson]

Created attachment 304120 [details]
Patch

[Blaze Burg]

Comment on attachment 304120 [details]
Patch

r=me

GTK is still sad.

[Carlos Garcia Campos]

Created attachment 304160 [details]
Patch for landing

[Carlos Garcia Campos]

(In reply to comment #6)
> Comment on attachment 304120 [details]
> Patch
> 
> r=me
> 
> GTK is still sad.

This is a known issue with our forwarding headers generator. It doesn't scan generated sources, so in this case #include <WebCoreCookie.h> is only added to the generated header DerivedSources/WebKit2/WebCookieManagerMessages.h. WebCookieManager.cpp needs that include, but it doesn't include it explicitly becase it already includes WebCookieManagerMessages.h. Our forwarding header generator doesn't generate the forwarding header in this case. To fix it we only need to make sure the header is included in a non-generated source file, in this case WebCookieManager.cpp. We could probably change the generator to also scan generated sources, ensuring the script is always run after all generated sources during the build. I'm not sure how easy that would be, though.

[WebKit Commit Bot]

Comment on attachment 304160 [details]
Patch for landing

Clearing flags on attachment: 304160

Committed r213759: <http://trac.webkit.org/changeset/213759>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Michael Catanzaro]

(In reply to comment #8) 
> This is a known issue with our forwarding headers generator. It doesn't scan
> generated sources, so in this case #include <WebCoreCookie.h> is only added
> to the generated header DerivedSources/WebKit2/WebCookieManagerMessages.h.
> WebCookieManager.cpp needs that include, but it doesn't include it
> explicitly becase it already includes WebCookieManagerMessages.h. Our
> forwarding header generator doesn't generate the forwarding header in this
> case. To fix it we only need to make sure the header is included in a
> non-generated source file, in this case WebCookieManager.cpp. We could
> probably change the generator to also scan generated sources, ensuring the
> script is always run after all generated sources during the build. I'm not
> sure how easy that would be, though.

Is there a bug report for this...?

[Csaba Osztrogonác]

(In reply to comment #9)
> Comment on attachment 304160 [details]
> Patch for landing
> 
> Clearing flags on attachment: 304160
> 
> Committed r213759: <http://trac.webkit.org/changeset/213759>

It broke the Apple Mac cmake build.

[Csaba Osztrogonác]

Just to document, the Windows build was fixed in bug169546