Bug 86819 – [chromium] Support mobile device rotation resizing

• Component: New Bugs
• Status: RESOLVED FIXED

[Alexandre Elias]

[chromium] Support mobile device rotation resizing

[Alexandre Elias]

Created attachment 142653 [details]
Patch

[Adam Barth]

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.

[James Robinson]

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

[Alexandre Elias]

Created attachment 143656 [details]
Patch

[Alexandre Elias]

Syntax problems fixed.

[WebKit Review Bot]

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

[WebKit Review Bot]

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

[Alexandre Elias]

Created attachment 143681 [details]
Patch

[Alexandre Elias]

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

[WebKit Review Bot]

Comment on attachment 143681 [details]
Patch

Clearing flags on attachment: 143681

Committed r118461: <http://trac.webkit.org/changeset/118461>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

Re-opened since this is blocked by 87468

[Vsevolod Vlasov]

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

[Vsevolod Vlasov]

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"

[Alexandre Elias]

Created attachment 148485 [details]
Patch

[Alexandre Elias]

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.

[WebKit Review Bot]

Comment on attachment 148485 [details]
Patch

Clearing flags on attachment: 148485

Committed r120877: <http://trac.webkit.org/changeset/120877>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.