Bug 89274

Summary: internals.settings.setDeviceScaleFactor doesn't work for WebKit2 and must be removed
Product: WebKit Reporter: Adam Barth <abarth>
Component: New BugsAssignee: Adam Barth <abarth>
Status: RESOLVED FIXED    
Severity: Normal CC: aelias, bdakin, cdumez, cmarcelo, dglazkov, eric.carlson, feature-media-reviews, fsamuel, jamesr, kenneth, menard, rakuco, simon.fraser, tdanderson, webkit.review.bot, zoltan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 89383    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ec2-cr-linux-04
none
Patch
none
Patch jamesr: review+

Adam Barth
Reported 2012-06-15 21:56:23 PDT
layoutTestController.setBackingScaleFactor is redundant with (and less awesome than) internals.settings.setDeviceScaleFactor
Attachments
Patch (29.72 KB, patch)
2012-06-15 21:59 PDT, Adam Barth
no flags
Archive of layout-test-results from ec2-cr-linux-04 (1.69 MB, application/zip)
2012-06-16 09:35 PDT, WebKit Review Bot
no flags
Patch (4.83 KB, patch)
2012-06-19 20:51 PDT, Adam Barth
no flags
Patch (4.84 KB, patch)
2012-06-19 20:59 PDT, Adam Barth
jamesr: review+
Adam Barth
Comment 1 2012-06-15 21:59:06 PDT
Adam Barth
Comment 2 2012-06-15 22:01:36 PDT
Comment on attachment 147949 [details] Patch I ran these tests on apple-mac on Lion in WebKit2 and they seem to still work (even when running --pixel-tests).
Levi Weintraub
Comment 3 2012-06-15 22:17:58 PDT
Comment on attachment 147949 [details] Patch Yay simplicity!
WebKit Review Bot
Comment 4 2012-06-16 09:35:06 PDT
Comment on attachment 147949 [details] Patch Attachment 147949 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12960902 New failing tests: svg/custom/focus-ring.svg svg/custom/createImageElement.svg svg/custom/group-opacity.svg svg/custom/image-parent-translation.xhtml
WebKit Review Bot
Comment 5 2012-06-16 09:35:12 PDT
Created attachment 147974 [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
WebKit Review Bot
Comment 6 2012-06-16 17:39:43 PDT
Comment on attachment 147949 [details] Patch Clearing flags on attachment: 147949 Committed r120541: <http://trac.webkit.org/changeset/120541>
WebKit Review Bot
Comment 7 2012-06-16 17:39:50 PDT
All reviewed patches have been landed. Closing bug.
Chris Dumez
Comment 8 2012-06-17 00:09:23 PDT
The following tests started to fail on EFL after this patch: fast/canvas/2d.backingStorePixelRatio.html fast/canvas/2d.imageDataHD.html The diffs look like: --- /home/buildslave-1/webkit-buildslave/efl-linux-64-debug/build/layout-test-results/fast/canvas/2d.backingStorePixelRatio-expected.txt +++ /home/buildslave-1/webkit-buildslave/efl-linux-64-debug/build/layout-test-results/fast/canvas/2d.backingStorePixelRatio-actual.txt @@ -1,3 +1,3 @@ -context1.webkitBackingStorePixelRatio is 1 -context2.webkitBackingStorePixelRatio is 1 +CONSOLE MESSAGE: line 22: TypeError: 'undefined' is not a function (evaluating 'layoutTestController.setBackingScaleFactor') +FAIL: Timed out waiting for notifyDone to be called --- /home/buildslave-1/webkit-buildslave/efl-linux-64-debug/build/layout-test-results/fast/canvas/2d.imageDataHD-expected.txt +++ /home/buildslave-1/webkit-buildslave/efl-linux-64-debug/build/layout-test-results/fast/canvas/2d.imageDataHD-actual.txt @@ -1,2 +1,3 @@ -High-DPI canvas is not enabled. +CONSOLE MESSAGE: line 117: TypeError: 'undefined' is not a function (evaluating 'layoutTestController.setBackingScaleFactor') +FAIL: Timed out waiting for notifyDone to be called
Adam Barth
Comment 9 2012-06-17 00:13:01 PDT
Hopefully these will be fixed by http://trac.webkit.org/changeset/120547
Chris Dumez
Comment 10 2012-06-17 00:45:12 PDT
(In reply to comment #9) > Hopefully these will be fixed by http://trac.webkit.org/changeset/120547 Yes, all good now. Thanks.
WebKit Review Bot
Comment 11 2012-06-18 14:30:11 PDT
Re-opened since this is blocked by 89383
Adam Barth
Comment 12 2012-06-19 20:51:30 PDT
James Robinson
Comment 13 2012-06-19 20:53:48 PDT
Comment on attachment 148496 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=148496&action=review > Source/WebCore/ChangeLog:8 > + Using this itnernals API causes the apple-mac port to ASSERT in WebKit2 itnernals -> internals. perhaps "window.internals" to be clearer about what internals this means? > Source/WebCore/ChangeLog:13 > + Tests should use setBackScalingFactor instead. BackScaling -> BackingScale > LayoutTests/fast/hidpi/device-scale-factor-paint.html:12 > + setTimeout(function() { layoutTestController.notifyDone(); }, 0); how about: setTimeout(layoutTestController.notifyDone); ? why do we need a timeout at all?
Adam Barth
Comment 14 2012-06-19 20:59:14 PDT
> why do we need a timeout at all? Dunno. I'm just following the idiom used by the rest of the hidpi tests. I assume it's because the callback from the injected bundle has a strange callstack and we want a clean call stack when we re-enter the LayoutTestController.
Adam Barth
Comment 15 2012-06-19 20:59:43 PDT
James Robinson
Comment 16 2012-06-20 12:58:13 PDT
Comment on attachment 148497 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=148497&action=review > LayoutTests/fast/hidpi/device-scale-factor-paint.html:12 > + setTimeout(function() { layoutTestController.notifyDone(); }, 0); setTimeout(layoutTestController.notifyDone); would be simpler
Adam Barth
Comment 17 2012-06-20 16:37:53 PDT
Note You need to log in before you can comment on or make changes to this bug.