Bug 208793 - Revise In-App Browser Privacy checks to better reflect the meaning of the flag
Summary: Revise In-App Browser Privacy checks to better reflect the meaning of the flag
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: Brent Fulgham
URL:
Keywords: InRadar
Depends on:
Blocks: 208818
  Show dependency treegraph
 
Reported: 2020-03-08 16:46 PDT by Brent Fulgham
Modified: 2020-03-24 12:20 PDT (History)
5 users (show)

See Also:


Attachments
Patch (3.42 KB, patch)
2020-03-08 16:50 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>.