Bug 171483 – Updates to _WKWebsiteDataStoreConfiguration cookie storage location SPI

• Component: WebKit2
• Status: RESOLVED FIXED

[Brady Eidson]

Updates to _WKWebsiteDataStoreConfiguration cookie storage location SPI

These include:
-Point to a file instead of a directory
-Proper sandboxing
-Proper testing

[Brady Eidson]

<rdar://problem/31906397>

[Brady Eidson]

Created attachment 308686 [details]
Patch

[Brady Eidson]

Created attachment 308687 [details]
Patch

[Brady Eidson]

Created attachment 308694 [details]
Patch

[Geoffrey Garen]

Comment on attachment 308694 [details]
Patch

r=me

[Brady Eidson]

Created attachment 308695 [details]
Patch

[Brady Eidson]

Created attachment 308696 [details]
Patch

[Brady Eidson]

Created attachment 308697 [details]
Patch

[Brady Eidson]

The mac-debug failures are happening on the test bots without this patch.

The ios-sim failures are happening on the test bots without this patch, too.

cq+'ing

[WebKit Commit Bot]

Comment on attachment 308697 [details]
Patch

Clearing flags on attachment: 308697

Committed r215991: <http://trac.webkit.org/changeset/215991>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

Comment on attachment 308697 [details]
Patch

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

> Source/WebCore/platform/spi/cf/CFNetworkSPI.h:175
> +#if (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED <= 101200)

Using less-than-or-equal in a deployment version check is almost always wrong. I think in this case you meant __MAC_OS_X_VERSION_MIN_REQUIRED < 101300.

> Source/WebKit2/NetworkProcess/cocoa/NetworkProcessCocoa.mm:214
> +#if (__MAC_OS_X_VERSION_MIN_REQUIRED <= 101200)

Ditto.

[Brady Eidson]

(In reply to mitz from comment #12)
> Comment on attachment 308697 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=308697&action=review
> 
> > Source/WebCore/platform/spi/cf/CFNetworkSPI.h:175
> > +#if (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED <= 101200)
> 
> Using less-than-or-equal in a deployment version check is almost always
> wrong. I think in this case you meant __MAC_OS_X_VERSION_MIN_REQUIRED <
> 101300.
> 
> > Source/WebKit2/NetworkProcess/cocoa/NetworkProcessCocoa.mm:214
> > +#if (__MAC_OS_X_VERSION_MIN_REQUIRED <= 101200)
> 
> Ditto.

Can you describe why it is wrong?

Is this documented anywhere?

(First I've heard of it)

(In reply to WebKit Commit Bot from comment #10)
> Comment on attachment 308697 [details]
> Patch
> 
> Clearing flags on attachment: 308697
> 
> Committed r215991: <http://trac.webkit.org/changeset/215991>

This also broke builds that use the Apple-internal SDK because _CFHTTPCookieStorageFlushCookieStores is annotated as deprecated in that SDK.

[Brady Eidson]

(In reply to mitz from comment #14)
> (In reply to WebKit Commit Bot from comment #10)
> > Comment on attachment 308697 [details]
> > Patch
> > 
> > Clearing flags on attachment: 308697
> > 
> > Committed r215991: <http://trac.webkit.org/changeset/215991>
> 
> This also broke builds that use the Apple-internal SDK because
> _CFHTTPCookieStorageFlushCookieStores is annotated as deprecated in that SDK.

On which OS?

[Brady Eidson]

(In reply to Brady Eidson from comment #15)
> (In reply to mitz from comment #14)
> > (In reply to WebKit Commit Bot from comment #10)
> > > Comment on attachment 308697 [details]
> > > Patch
> > > 
> > > Clearing flags on attachment: 308697
> > > 
> > > Committed r215991: <http://trac.webkit.org/changeset/215991>
> > 
> > This also broke builds that use the Apple-internal SDK because
> > _CFHTTPCookieStorageFlushCookieStores is annotated as deprecated in that SDK.
> 
> On which OS?

I see the problem.

I'll just have disable the deprecation warnings there.

(In reply to Brady Eidson from comment #13)
> (In reply to mitz from comment #12)
> > Comment on attachment 308697 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=308697&action=review
> > 
> > > Source/WebCore/platform/spi/cf/CFNetworkSPI.h:175
> > > +#if (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED <= 101200)
> > 
> > Using less-than-or-equal in a deployment version check is almost always
> > wrong. I think in this case you meant __MAC_OS_X_VERSION_MIN_REQUIRED <
> > 101300.
> > 
> > > Source/WebKit2/NetworkProcess/cocoa/NetworkProcessCocoa.mm:214
> > > +#if (__MAC_OS_X_VERSION_MIN_REQUIRED <= 101200)
> > 
> > Ditto.
> 
> Can you describe why it is wrong?

It’s wrong because API availability intervals are half-open, they include the left endpoint and exclude the right endpoint. For example, the interval of macOS versions v in which WKWebView’s certificateChain property is available but not deprecated is 10.11 <= v < 10.12.

> Is this documented anywhere?

I believe that the semantics of availability annotations are documented in Availability.h as well as in the clang documentation. Everything else follows from that.

> (First I've heard of it)

It’s also easy to look for prior examples in the source code (as long as there aren’t many similar mistakes).

[Brady Eidson]

Followups in https://trac.webkit.org/changeset/215994/webkit