Bug 186109 - Add setting to allow override screen size to be disabled.
Summary: Add setting to allow override screen size to be disabled.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Megan Gardner
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-05-30 14:31 PDT by Megan Gardner
Modified: 2018-05-31 16:35 PDT (History)
6 users (show)

See Also:


Attachments
Patch (6.52 KB, patch)
2018-05-30 14:41 PDT, Megan Gardner
no flags Details | Formatted Diff | Diff
Patch (6.52 KB, patch)
2018-05-30 15:16 PDT, Megan Gardner
no flags Details | Formatted Diff | Diff
Patch (6.61 KB, patch)
2018-05-30 16:27 PDT, Megan Gardner
no flags Details | Formatted Diff | Diff

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