Bug 208793

Summary: Revise In-App Browser Privacy checks to better reflect the meaning of the flag
Product: WebKit Reporter: Brent Fulgham <bfulgham>
Component: WebKit Misc.Assignee: Brent Fulgham <bfulgham>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, cdumez, commit-queue, webkit-bug-importer, wilander
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 208818    
Attachments:
Description Flags
Patch none

Description Brent Fulgham 2020-03-08 16:46:55 PDT 
The 'isInAppBrowserPrivacyEnabled' seems like a feature, but it's really an internal debugging flag.

Revise the use of these calls to make it clearer.
Comment 1 Radar WebKit Bug Importer 2020-03-08 16:47:54 PDT 
<rdar://problem/60206256>
Comment 2 Brent Fulgham 2020-03-08 16:50:59 PDT 
Created attachment 393000 [details]
Patch
Comment 3 John Wilander 2020-03-08 16:56:45 PDT 
Comment on attachment 393000 [details]
Patch

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

r=me with one comment.

> Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:1219
> +        if (m_isInAppBrowserPrivacyEnabled || NETWORK_SESSION_COCOA_ADDITIONS)

Is there a reason for doing this is two if clauses and not if (isNavigatingToAppBoundDomain == NavigatingToAppBoundDomain::Yes && (m_isInAppBrowserPrivacyEnabled || NETWORK_SESSION_COCOA_ADDITIONS)) ? You do write a single if clause in the second case below.
Comment 4 Brent Fulgham 2020-03-08 16:59:04 PDT 
(In reply to John Wilander from comment #3)
> Comment on attachment 393000 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=393000&action=review
> 
> r=me with one comment.
> 
> > Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:1219
> > +        if (m_isInAppBrowserPrivacyEnabled || NETWORK_SESSION_COCOA_ADDITIONS)
> 
> Is there a reason for doing this is two if clauses and not if
> (isNavigatingToAppBoundDomain == NavigatingToAppBoundDomain::Yes &&
> (m_isInAppBrowserPrivacyEnabled || NETWORK_SESSION_COCOA_ADDITIONS)) ? You
> do write a single if clause in the second case below.

I probably should have done both identically.
Comment 5 Brent Fulgham 2020-03-08 17:55:21 PDT 
Committed r258124: <https://trac.webkit.org/changeset/258124>
Comment 6 WebKit Commit Bot 2020-03-08 18:03:07 PDT 
The commit-queue encountered the following flaky tests while processing attachment 393000 [details]:

imported/w3c/web-platform-tests/xhr/open-send-open.htm bug 208794 (author: youennf@gmail.com)
The commit-queue is continuing to process your patch.
Comment 7 Chris Dumez 2020-03-24 12:20:09 PDT 
Comment on attachment 393000 [details]
Patch

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

> Source/WebKit/UIProcess/WebPageProxy.cpp:3100
> +    if (isMainFrame && (m_preferences->isInAppBrowserPrivacyEnabled() || WEB_PAGE_PROXY_ADDITIONS_SETISNAVIGATINGTOAPPBOUNDDOMAIN_2)) {

So now this can get enabled even if the pref is disabled? See <rdar://problem/60518694>.