Bug 208777 - Create a flag to disable in-app browser quirks
Summary: Create a flag to disable in-app browser quirks
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:
 
Reported: 2020-03-07 16:15 PST by Brent Fulgham
Modified: 2020-03-07 21:39 PST (History)
8 users (show)

See Also:


Attachments
Patch (11.09 KB, patch)
2020-03-07 16:42 PST, Brent Fulgham
no flags Details | Formatted Diff | Diff
Patch (11.06 KB, patch)
2020-03-07 16:59 PST, Brent Fulgham
no flags Details | Formatted Diff | Diff
Patch (21.36 KB, patch)
2020-03-07 20:16 PST, Brent Fulgham
no flags Details | Formatted Diff | Diff
Patch for landing (19.26 KB, patch)
2020-03-07 20:50 PST, 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-07 16:15:39 PST
Add a runtime flag to control whether we use quirks when performing loads for AppBound pages. This is on by default, with the goal of turning it off in a future update.
Comment 1 Brent Fulgham 2020-03-07 16:17:34 PST
<rdar://problem/60062197>
Comment 2 Brent Fulgham 2020-03-07 16:42:44 PST
Created attachment 392913 [details]
Patch
Comment 3 Brent Fulgham 2020-03-07 16:59:30 PST
Created attachment 392915 [details]
Patch
Comment 4 Alex Christensen 2020-03-07 18:05:42 PST
Comment on attachment 392915 [details]
Patch

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

> Source/WebKit/Shared/WebPreferencesDefaultValues.cpp:220
> +#endif

#else

Otherwise we have unreachable code.

> Source/WebKit/Shared/WebProcessCreationParameters.h:128
> +    bool needsInAppBrowserPrivacyQuirks { false };

Let's put this on the WebPageCreationParameters instead.
Also, it needs to be serialized somewhere.

> Source/WebKit/WebProcess/WebProcess.cpp:475
> +    WebCore::RuntimeEnabledFeatures::sharedFeatures().setNeedsInAppBrowserPrivacyQuirks(parameters.needsInAppBrowserPrivacyQuirks);

Instead of using RuntimeEnabledFeatures, let's just put a bool on WebPage.
Comment 5 Brent Fulgham 2020-03-07 19:45:19 PST
Comment on attachment 392915 [details]
Patch

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

>> Source/WebKit/Shared/WebProcessCreationParameters.h:128
>> +    bool needsInAppBrowserPrivacyQuirks { false };
> 
> Let's put this on the WebPageCreationParameters instead.
> Also, it needs to be serialized somewhere.

Whoops!

>> Source/WebKit/WebProcess/WebProcess.cpp:475
>> +    WebCore::RuntimeEnabledFeatures::sharedFeatures().setNeedsInAppBrowserPrivacyQuirks(parameters.needsInAppBrowserPrivacyQuirks);
> 
> Instead of using RuntimeEnabledFeatures, let's just put a bool on WebPage.

Done.
Comment 6 Brent Fulgham 2020-03-07 20:16:35 PST
Created attachment 392934 [details]
Patch
Comment 7 Simon Fraser (smfr) 2020-03-07 20:44:27 PST
Comment on attachment 392934 [details]
Patch

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

> Source/WebKit/FeatureFlags/WebKit.plist:44
> +	<key>NeedsInAppBrowserPrivacyQuirks</key>
> +	<dict>
> +		<key>Enabled</key>
> +		<true/>
> +	</dict>

Remove.

> Source/WebKit/Shared/WebPreferencesDefaultValues.cpp:223
> +bool defaultNeedsInAppBrowserPrivacyQuirks()
> +{
> +#if HAVE(HAVE_SYSTEM_FEATURE_FLAGS)
> +    return os_feature_enabled(WebKit, NeedsInAppBrowserPrivacyQuirks);
> +#else
>      return false;
> +#endif
>  }

Remove.
Comment 8 Alex Christensen 2020-03-07 20:50:16 PST
Comment on attachment 392934 [details]
Patch

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

looks good.

> Source/WebKit/FeatureFlags/WebKit.plist:42
> +		<key>Enabled</key>

This indentation is a little strange.

>> Source/WebKit/Shared/WebPreferencesDefaultValues.cpp:223
>>  }
> 
> Remove.

This is definitely used to get the default value from os_feature_enabled.  If that's what's desired (and I think it is) then this must remain.
Comment 9 Brent Fulgham 2020-03-07 20:50:41 PST
Created attachment 392936 [details]
Patch for landing
Comment 10 WebKit Commit Bot 2020-03-07 21:39:14 PST
Comment on attachment 392936 [details]
Patch for landing

Clearing flags on attachment: 392936

Committed r258101: <https://trac.webkit.org/changeset/258101>
Comment 11 WebKit Commit Bot 2020-03-07 21:39:16 PST
All reviewed patches have been landed.  Closing bug.