Use constrained values for shouldDisableDesktopWorkarounds.

Use constrained values for shouldDisableDesktopWorkarounds.

shouldDisableDesktopWorkarounds() used parsed ViewportArguments instead of the
constrained values, which meant auto and percentage sized @viewport would not
be considered as documents adapted to small screens.

This fix uses the constrained values from PageScaleConstraints instead.

A test for using auto-sizing of @viewport was added.

BUG=280759
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=157173

[kenneth.r.christiansen, 2013-08-29 13:35:55]

lgtm but someone else should take a look

https://codereview.chromium.org/23592015/diff/4001/Source/web/WebViewImpl.cpp
File Source/web/WebViewImpl.cpp (right):

https://codereview.chromium.org/23592015/diff/4001/Source/web/WebViewImpl.cpp...
Source/web/WebViewImpl.cpp:4118: bool
WebViewImpl::shouldDisableDesktopWorkarounds()
If these are quirks, should we suffix the methods with Quirk?

But I guess the desktop workaround is the actual quirk.

Why not make aapplyDesktopWorkaroundQuirk() or similar.

https://codereview.chromium.org/23592015/diff/4001/Source/web/tests/data/disa...
File Source/web/tests/data/disambiguation_popup_viewport_site.html (right):

https://codereview.chromium.org/23592015/diff/4001/Source/web/tests/data/disa...
Source/web/tests/data/disambiguation_popup_viewport_site.html:6: width: auto;
I guess we might need to add this to other viewport tests as well.. I would add
a comment why it is needed.

[rune, 2013-08-29 13:57:53]

https://codereview.chromium.org/23592015/diff/4001/Source/web/WebViewImpl.cpp
File Source/web/WebViewImpl.cpp (right):

https://codereview.chromium.org/23592015/diff/4001/Source/web/WebViewImpl.cpp...
Source/web/WebViewImpl.cpp:4118: bool
WebViewImpl::shouldDisableDesktopWorkarounds()
On 2013/08/29 13:35:55, kenneth.r.christiansen wrote:
> If these are quirks, should we suffix the methods with Quirk?
> 
> But I guess the desktop workaround is the actual quirk.
> 
> Why not make aapplyDesktopWorkaroundQuirk() or similar.

You mean negate it and call it shouldApplyDesktopWorkaroundQuirks instead?

Sounds like a good thing since the result is currently negated the only place
it's invoked from. It's not a quirk in the standards sense and workaround-quirk
sounds excessive to me. I'll see what others think about the name.

https://codereview.chromium.org/23592015/diff/4001/Source/web/tests/data/disa...
File Source/web/tests/data/disambiguation_popup_viewport_site.html (right):

https://codereview.chromium.org/23592015/diff/4001/Source/web/tests/data/disa...
Source/web/tests/data/disambiguation_popup_viewport_site.html:6: width: auto;
On 2013/08/29 13:35:55, kenneth.r.christiansen wrote:
> I guess we might need to add this to other viewport tests as well.. I would
add
> a comment why it is needed.

I think you might be confused by rietveld/me. This is a completely new file.
It's the @viewport rule making everything auto that we're actually testing here.

I tried to add a test for "width: 100%" too, but that test was bitten by the
percentage resize problem fixed in https://codereview.chromium.org/22549002/

[aelias_OOO_until_Jul13, 2013-08-29 18:29:35]

https://codereview.chromium.org/23592015/diff/4001/Source/web/WebViewImpl.cpp
File Source/web/WebViewImpl.cpp (right):

https://codereview.chromium.org/23592015/diff/4001/Source/web/WebViewImpl.cpp...
Source/web/WebViewImpl.cpp:4118: bool
WebViewImpl::shouldDisableDesktopWorkarounds()
On 2013/08/29 13:57:53, rune wrote:
> On 2013/08/29 13:35:55, kenneth.r.christiansen wrote:
> > If these are quirks, should we suffix the methods with Quirk?
> > 
> > But I guess the desktop workaround is the actual quirk.
> > 
> > Why not make aapplyDesktopWorkaroundQuirk() or similar.
> 
> You mean negate it and call it shouldApplyDesktopWorkaroundQuirks instead?
> 
> Sounds like a good thing since the result is currently negated the only place
> it's invoked from. It's not a quirk in the standards sense and
workaround-quirk
> sounds excessive to me. I'll see what others think about the name.

It's used to decide whether to trigger Chromium's tap disambiguation (and we
kept the name general in case there would be something else like that in the
future).  I don't think we should use "quirk" since it's nothing to do with
standards quirk.  Maybe "workaround" is a bit confusing too, but I can't come up
with a better name.

[kenneth.r.christiansen, 2013-09-02 15:32:24]

ok lgtm

[rune, 2013-09-03 08:20:46]

@mkwst: could you have a look at this? It needs review by an OWNER.

[Mike West, 2013-09-03 08:28:21]

On 2013/09/03 08:20:46, rune wrote:
> @mkwst: could you have a look at this? It needs review by an OWNER.

LGTM.

[commit-bot: I haz the power, 2013-09-03 08:34:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rune@opera.com/23592015/4001

[commit-bot: I haz the power, 2013-09-03 08:44:14]

Retried try job too often on blink_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=blink_pres...

[rune, 2013-09-03 08:46:17]

Needs a Source/Web owner. Trying abarth.

[abarth-chromium, 2013-09-03 16:16:54]

LGTM

[commit-bot: I haz the power, 2013-09-03 17:02:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rune@opera.com/23592015/4001

[commit-bot: I haz the power, 2013-09-03 17:54:09]

Retried try job too often on linux_blink_rel for step(s) webkit_tests,
webkit_unit_tests, weborigin_unittests, wtf_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blin...

[rune, 2013-09-03 22:39:00]

On 2013/09/03 17:54:09, I haz the power (commit-bot) wrote:
> Retried try job too often on linux_blink_rel for step(s) webkit_tests,
> webkit_unit_tests, weborigin_unittests, wtf_unittests
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blin...

These tests pass for me locally.

[commit-bot: I haz the power, 2013-09-03 23:03:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rune@opera.com/23592015/36001

[commit-bot: I haz the power, 2013-09-04 00:38:17]

Change committed as 157173