Bug 212787 - Disable CFNetwork AppSSO interception for Mac Catalyst
Summary: Disable CFNetwork AppSSO interception for Mac Catalyst
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jiewen Tan
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-06-04 15:44 PDT by Jiewen Tan
Modified: 2020-06-04 17:09 PDT (History)
4 users (show)

See Also:


Attachments
Patch (1.36 KB, patch)
2020-06-04 15:48 PDT, Jiewen Tan
no flags Details | Formatted Diff | Diff
Patch (2.61 KB, patch)
2020-06-04 15:58 PDT, Jiewen Tan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jiewen Tan 2020-06-04 15:44:40 PDT
Disable CFNetwork AppSSO interception for Mac Catalyst.
Comment 1 Jiewen Tan 2020-06-04 15:45:01 PDT
<rdar://problem/63738783>
Comment 2 Jiewen Tan 2020-06-04 15:48:28 PDT
Created attachment 401082 [details]
Patch
Comment 3 Jiewen Tan 2020-06-04 15:51:48 PDT
The current patch only disable CFNetwork interception per network session. Let me use the global switch to do it for other processes.
Comment 4 Jiewen Tan 2020-06-04 15:58:13 PDT
Created attachment 401083 [details]
Patch
Comment 5 Chris Dumez 2020-06-04 15:58:33 PDT
Comment on attachment 401082 [details]
Patch

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

> Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:1148
> +#if HAVE(APP_SSO) || PLATFORM(MACCATALYST)

What about TVOS and WatchOS? It looks like APP_SSO is 1 on macOS and iOS only.
Comment 6 Chris Dumez 2020-06-04 15:59:04 PDT
Comment on attachment 401083 [details]
Patch

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

> Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:1148
> +#if HAVE(APP_SSO) || PLATFORM(MACCATALYST)

What about TVOS and WatchOS? It looks like APP_SSO is 1 on macOS and iOS only.
Comment 7 Jiewen Tan 2020-06-04 16:16:49 PDT
Comment on attachment 401083 [details]
Patch

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

Thanks Chris for the r+.

>> Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:1148
>> +#if HAVE(APP_SSO) || PLATFORM(MACCATALYST)
> 
> What about TVOS and WatchOS? It looks like APP_SSO is 1 on macOS and iOS only.

The AppSSO framework is only available for iOS, macOS and Catalyst. So we are good for the other two platforms.
Comment 8 Tim Horton 2020-06-04 16:39:02 PDT
Comment on attachment 401083 [details]
Patch

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

>>> Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:1148
>>> +#if HAVE(APP_SSO) || PLATFORM(MACCATALYST)
>> 
>> What about TVOS and WatchOS? It looks like APP_SSO is 1 on macOS and iOS only.
> 
> The AppSSO framework is only available for iOS, macOS and Catalyst. So we are good for the other two platforms.

Is the NSURLSessionConfiguration property available on all platforms? It might be nice to just remove this ifdef for future-proofing 🤷‍♂️
Comment 9 Chris Dumez 2020-06-04 16:39:54 PDT
(In reply to Tim Horton from comment #8)
> Comment on attachment 401083 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=401083&action=review
> 
> >>> Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:1148
> >>> +#if HAVE(APP_SSO) || PLATFORM(MACCATALYST)
> >> 
> >> What about TVOS and WatchOS? It looks like APP_SSO is 1 on macOS and iOS only.
> > 
> > The AppSSO framework is only available for iOS, macOS and Catalyst. So we are good for the other two platforms.
> 
> Is the NSURLSessionConfiguration property available on all platforms? It
> might be nice to just remove this ifdef for future-proofing 🤷‍♂️

Yes, it is available on all platform, merely a no-op on platform that don't support AppSSO.
Comment 10 EWS 2020-06-04 16:54:06 PDT
Committed r262585: <https://trac.webkit.org/changeset/262585>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 401083 [details].
Comment 11 Jiewen Tan 2020-06-04 17:09:46 PDT
(In reply to Chris Dumez from comment #9)
> (In reply to Tim Horton from comment #8)
> > Comment on attachment 401083 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=401083&action=review
> > 
> > >>> Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:1148
> > >>> +#if HAVE(APP_SSO) || PLATFORM(MACCATALYST)
> > >> 
> > >> What about TVOS and WatchOS? It looks like APP_SSO is 1 on macOS and iOS only.
> > > 
> > > The AppSSO framework is only available for iOS, macOS and Catalyst. So we are good for the other two platforms.
> > 
> > Is the NSURLSessionConfiguration property available on all platforms? It
> > might be nice to just remove this ifdef for future-proofing 🤷‍♂️
> 
> Yes, it is available on all platform, merely a no-op on platform that don't
> support AppSSO.

I think we need to support -2 macOS. Therefore, we can't do that now.