Created attachment 142653 [details] Patch

Comment on attachment 142653 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=142653&action=review This looks reasonable, but I'm not an expert on such things. Is there someone else you'd like to review your patch as well? > Source/WebKit/chromium/src/WebViewImpl.cpp:1350 > + int dpiIndependentViewportWidth = newSize.width / > + page()->settings()->defaultDeviceScaleFactor(); > + settings()->setLayoutFallbackWidth( > + std::max(kStandardFallbackWidth, dpiIndependentViewportWidth)); There's no 80 col limit in WebKit. These can each be on one line.

Comment on attachment 142653 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=142653&action=review > Source/WebKit/chromium/src/WebViewImpl.cpp:1346 > + const int kStandardFallbackWidth = 980; we don't typically use the "k" prefix for constants in WebKit - constants are named the same way as normal variables

Created attachment 143656 [details] Patch

Syntax problems fixed.

Comment on attachment 143656 [details] Patch Attachment 143656 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12780310 New failing tests: WebFrameTest.DeviceScaleFactorUsesDefaultWithoutViewportTag WebFrameTest.FixedLayoutInitializeAtMinimumPageScale

Created attachment 143667 [details] Archive of layout-test-results from ec2-cr-linux-03 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-03 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

Created attachment 143681 [details] Patch

Ouch, while fixing style warnings to remove "== 0", I had forgotten to replace them with ! symbols. Good thing these unit tests just landed.

Comment on attachment 143681 [details] Patch Clearing flags on attachment: 143681 Committed r118461: <http://trac.webkit.org/changeset/118461>

All reviewed patches have been landed. Closing bug.

Re-opened since this is blocked by 87468

This patch breaks two chromium browser_tests: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=browser_tests&tests=AutomationRendererHelperTest.ScrollingSnapshot%2CAutomationRendererHelperTest.RTLSnapshot Reverted in: http://trac.webkit.org/changeset/118490

AutomationRendererHelperTest.RTLSnapshot: chrome/renderer/automation/automation_renderer_helper_browsertest.cc:40: Failure Value of: base::MD5String(png_data_str).c_str() Actual: "d81f041da73f0e5eee86ecb995410b4b" Expected: reference_md5.c_str() Which is: "3adefecb4472e6ba1812f9af1fdea3e4" AutomationRendererHelperTest.ScrollingSnapshot: chrome/renderer/automation/automation_renderer_helper_browsertest.cc:40: Failure Value of: base::MD5String(png_data_str).c_str() Actual: "d81f041da73f0e5eee86ecb995410b4b" Expected: reference_md5.c_str() Which is: "3adefecb4472e6ba1812f9af1fdea3e4"

Created attachment 148485 [details] Patch

OK, replaced the fixedLayoutEnabled() guards with settings()->viewportEnabled() instead, because AutomationRendererHelper is using fixed layout for the purpose of grabbing a snapshot of the contents while having resizes ignored.

Comment on attachment 148485 [details] Patch Clearing flags on attachment: 148485 Committed r120877: <http://trac.webkit.org/changeset/120877>

All reviewed patches have been landed. Closing bug.