Bug 216655

Summary: Enable ITP in WKWebViews for apps with the full browser entitlement who are not linked to iOS 14.0
Product: WebKit Reporter: Kate Cheney <katherine_cheney>
Component: WebKit Misc.Assignee: Kate Cheney <katherine_cheney>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, webkit-bug-importer, wilander
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Kate Cheney 2020-09-17 10:51:37 PDT 
Apps with the full browser entitlement who are not linked to iOS 14.0 should have ITP enabled in WKWebViews, and should not be treated as a full web browser in other WebKit cases until upgrading to iOS 14.0.
Comment 1 Radar WebKit Bug Importer 2020-09-17 10:52:00 PDT 
<rdar://problem/69081227>
Comment 2 Kate Cheney 2020-09-17 11:22:45 PDT 
Created attachment 409051 [details]
Patch
Comment 3 John Wilander 2020-09-17 11:54:24 PDT 
Comment on attachment 409051 [details]
Patch

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

> Source/WebKit/Shared/Cocoa/DefaultWebBrowserChecks.mm:246
> +    return appWasLinkedOnOrAfter && isFullWebBrowser(WebCore::applicationBundleIdentifier());

Couldn't this be
return linkedOnOrAfter(SDKVersion::FirstWithSessionCleanupByDefault, AssumeSafariIsAlwaysLinkedOnAfter::Yes) && isFullWebBrowser(WebCore::applicationBundleIdentifier())?
Comment 4 Brent Fulgham 2020-09-17 11:54:45 PDT 
Comment on attachment 409051 [details]
Patch

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

I don't think this is quite right (see inline comments)

> Source/WebKit/Shared/Cocoa/DefaultWebBrowserChecks.mm:109
> +    if (isFullWebBrowser(bundleIdentifier) && !appWasLinkedOnOrAfter)

Instead of this new test, I think we should change the existing linkedOnOrAfter check to only bail out if the default browser entitlement was NOT used.

if (!appWasLinkedOnOrAfter && !isFullWebBrowser(bundleIdentifier))
    return false;

This way, we fall through and use the User's choice about ITP state for a default browser, even if it was built against the older SDK.

> Source/WebKit/Shared/Cocoa/DefaultWebBrowserChecks.mm:246
> +    return appWasLinkedOnOrAfter && isFullWebBrowser(WebCore::applicationBundleIdentifier());

After thinking about it, I don't think we need this check. We don't care if the app was linked against an older SDK. We just want to make sure that default browsers honor the user's ITP setting which you handle in your change to 'determineITPStateInternal'.
Comment 5 Kate Cheney 2020-09-17 12:19:41 PDT 
Created attachment 409056 [details]
Patch
Comment 6 Brent Fulgham 2020-09-17 12:55:26 PDT 
Comment on attachment 409056 [details]
Patch

Looks good! r=me
Comment 7 EWS 2020-09-17 13:42:57 PDT 
Committed r267208: <https://trac.webkit.org/changeset/267208>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 409056 [details].