Bug 186109

Summary: Add setting to allow override screen size to be disabled.
Product: WebKit Reporter: Megan Gardner <megan_gardner>
Component: New BugsAssignee: Megan Gardner <megan_gardner>
Status: RESOLVED FIXED    
Severity: Normal CC: aestes, bdakin, commit-queue, sam, thorton, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Megan Gardner 2018-05-30 14:31:53 PDT
Add setting to allow override screen size to be disabled.
Comment 1 Megan Gardner 2018-05-30 14:40:17 PDT
<rdar://problem/40277044>
Comment 2 Megan Gardner 2018-05-30 14:41:36 PDT
Created attachment 341603 [details]
Patch
Comment 3 Andy Estes 2018-05-30 14:53:49 PDT
Comment on attachment 341603 [details]
Patch

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

> Source/WebCore/page/DeprecatedGlobalSettings.cpp:75
> +bool DeprecatedGlobalSettings::gDisableScreenSizeOverride = true;

Shouldn't this default to false?

> Source/WebCore/page/Settings.yaml:154
> +disableScreenSizeOverride:
> +  initial: false

I don't think you need this for global settings.

> Source/WebKit/UIProcess/WebPageProxy.cpp:453
> +    DeprecatedGlobalSettings::setDisableScreenSizeOverride(preferencesStore().getBoolValueForKey(WebPreferencesKey::disableScreenSizeOverrideKey()));

Doesn't something need to send this setting to the web process?
Comment 4 Andy Estes 2018-05-30 15:14:33 PDT
(In reply to Andy Estes from comment #3)
> Comment on attachment 341603 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=341603&action=review
> 
> > Source/WebKit/UIProcess/WebPageProxy.cpp:453
> > +    DeprecatedGlobalSettings::setDisableScreenSizeOverride(preferencesStore().getBoolValueForKey(WebPreferencesKey::disableScreenSizeOverrideKey()));
> 
> Doesn't something need to send this setting to the web process?

I guess not, because we always query screen size in the UI process (then pass it to the web process in WebPageCreationParameters)!
Comment 5 Megan Gardner 2018-05-30 15:16:17 PDT
Created attachment 341609 [details]
Patch
Comment 6 Megan Gardner 2018-05-30 16:27:45 PDT
Created attachment 341616 [details]
Patch
Comment 7 Wenson Hsieh 2018-05-31 13:51:06 PDT
Comment on attachment 341616 [details]
Patch

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

> Source/WebCore/page/Settings.yaml:153
> +disableScreenSizeOverride:

I don't think we need this in Settings.yaml if we're putting this in DeprecatedGlobalSettings, since this will generate a derived getters and setters in Settings.
Comment 8 WebKit Commit Bot 2018-05-31 14:03:42 PDT
Comment on attachment 341616 [details]
Patch

Clearing flags on attachment: 341616

Committed r232372: <https://trac.webkit.org/changeset/232372>
Comment 9 WebKit Commit Bot 2018-05-31 14:03:43 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Sam Weinig 2018-05-31 16:28:59 PDT
Is there any way to achieve this without adding to DeprecatedGlobalSettings? The goal is to reduce usage of that class and eventually remove it, hence the 'Deprecated' part.
Comment 11 Megan Gardner 2018-05-31 16:35:43 PDT
There really isn't, given the places where override screen size is called from. This trajectory was cleared with Tim and Andy before it was embarked on.