Settings::defaultDeviceScaleFactor is redundant with Page::deviceScaleFactor
Created attachment 145894 [details] Work in progress
I'd like to offer this patch for discussion. The part that I'm least sure about is the interaction with applyDefaultDeviceScaleFactorInCompositor. There also appears to be one call site on the Chromium side for WebSettings::setDefaultDeviceScaleFactor, so I need to dig into that as well.
Comment on attachment 145894 [details] Work in progress View in context: https://bugs.webkit.org/attachment.cgi?id=145894&action=review > Source/WebKit/chromium/src/WebViewImpl.cpp:3500 > + if (m_webSettings->applyDefaultDeviceScaleFactorInCompositor()) { > + m_deviceScaleInCompositor = page()->deviceScaleFactor(); This looks correct to me.
Comment on attachment 145894 [details] Work in progress View in context: https://bugs.webkit.org/attachment.cgi?id=145894&action=review >> Source/WebKit/chromium/src/WebViewImpl.cpp:3500 >> + m_deviceScaleInCompositor = page()->deviceScaleFactor(); > > This looks correct to me. To me also. Strictly speaking, I think the (... && page()->deviceScaleFactor() == 1) check should be done still to guard the setPageScaleFactorLimits. But we are (re)moving that call in a separate CL anyhow. Should this patch rename WebSettings::applyDefaultDeviceScaleFactorInCompositor to applyDeviceScaleFactorInCompositor then?
> Should this patch rename WebSettings::applyDefaultDeviceScaleFactorInCompositor to applyDeviceScaleFactorInCompositor then? That sounds like a good idea, but I'd be inclined to do that in a separate patch.
Comment on attachment 145894 [details] Work in progress Ok. It sounds like this patch isn't too far off base, so maybe it makes sense to nominate it for a real review.
Comment on attachment 145894 [details] Work in progress R=me (although you have some merge conflicts somewhere)
Created attachment 146150 [details] Patch for landing
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Comment on attachment 146150 [details] Patch for landing Attachment 146150 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/12905979
9>WebKit.exp : error LNK2001: unresolved external symbol "public: void __thiscall WebCore::Settings::setDefaultDeviceScaleFactor(int)" (?setDefaultDeviceScaleFactor@Settings@WebCore@@QAEXH@Z)
Created attachment 146171 [details] Patch for landing
Comment on attachment 146171 [details] Patch for landing Rejecting attachment 146171 [details] from commit-queue. New failing tests: WebFrameTest.DeviceScaleFactorUsesDefaultWithoutViewportTag Full output: http://queues.webkit.org/results/12910287
Created attachment 146274 [details] Archive of layout-test-results from ec2-cq-03 The attached test failures were seen while running run-webkit-tests on the commit-queue. Bot: ec2-cq-03 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment on attachment 146171 [details] Patch for landing Attachment 146171 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12923008 New failing tests: WebFrameTest.DeviceScaleFactorUsesDefaultWithoutViewportTag
Created attachment 146276 [details] Archive of layout-test-results from ec2-cr-linux-04 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-04 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Committed r119752: <http://trac.webkit.org/changeset/119752>