[Android WebView] Add a legacy quirk for 'user-scalable=no' case

[Android WebView] Add a legacy quirk for 'user-scalable=no' case

In legacy Android WebView implementation, having 'user-scalable=no'
in the viewport meta tag ignores scale set via 'initial-scale' and
also affects wide viewport quirk behavior.

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

[mnaganov (inactive), 2013-10-30 18:27:04]

Hello, Alexandre!

Please take a look! This only affects the legacy compatibility mode.

[mnaganov (inactive), 2013-11-01 07:56:42]

aelias@: Ping!

[aelias_OOO_until_Jul13, 2013-11-01 20:12:36]

See my comment on the bug.

[aelias_OOO_until_Jul13, 2013-11-01 23:58:00]

https://codereview.chromium.org/53053002/diff/40001/Source/web/PageScaleConst...
File Source/web/PageScaleConstraintsSet.cpp (right):

https://codereview.chromium.org/53053002/diff/40001/Source/web/PageScaleConst...
Source/web/PageScaleConstraintsSet.cpp:176: 
The logic inside wideViewportQuirkEnabled is too tangled.  Could you delete all
the changes above and just have a block at this point that says:

if (nonUserScalableQuirkEnabled) {
    if (!description.userZoom) {
        m_pageDefinedConstraints.initialScale = targetDensityDPIFactor;
        if (description.maxWidth.isAuto() || description.maxWidth.type() ==
ExtendToZoom || !description.maxWidth.isFixed())))
            adjustedLayoutSizeWidth = viewSize.width() / targetDensityDPIFactor;

    }
}

if (!description.userZoom) {
    m_pageDefinedConstraints.minimumScale =
m_pageDefinedConstraints.initialScale;
    m_pageDefinedConstraints.maximumScale =
m_pageDefinedConstraints.initialScale;
}

I think that's equivalent (except that it actually disallows scaling in all
cases with "user-scalable:no" which is a bugfix anyway).

[mnaganov (inactive), 2013-11-05 11:01:37]

Thanks, Alexandre!

https://codereview.chromium.org/53053002/diff/40001/Source/web/PageScaleConst...
File Source/web/PageScaleConstraintsSet.cpp (right):

https://codereview.chromium.org/53053002/diff/40001/Source/web/PageScaleConst...
Source/web/PageScaleConstraintsSet.cpp:176: 
On 2013/11/01 23:58:00, aelias wrote:
> The logic inside wideViewportQuirkEnabled is too tangled.  Could you delete
all
> the changes above and just have a block at this point that says:
> 
> if (nonUserScalableQuirkEnabled) {
>     if (!description.userZoom) {
>         m_pageDefinedConstraints.initialScale = targetDensityDPIFactor;
>         if (description.maxWidth.isAuto() || description.maxWidth.type() ==
> ExtendToZoom || !description.maxWidth.isFixed())))
>             adjustedLayoutSizeWidth = viewSize.width() /
targetDensityDPIFactor;
> 
>     }
> }
> 
> if (!description.userZoom) {
>     m_pageDefinedConstraints.minimumScale =
> m_pageDefinedConstraints.initialScale;
>     m_pageDefinedConstraints.maximumScale =
> m_pageDefinedConstraints.initialScale;
> }
> 
> I think that's equivalent (except that it actually disallows scaling in all
> cases with "user-scalable:no" which is a bugfix anyway).

OK, thanks for the suggestion! Rewritten.

[aelias_OOO_until_Jul13, 2013-11-05 19:39:09]

lgtm.  Adding abarth@ for new setting in public/web/

https://codereview.chromium.org/53053002/diff/110001/Source/web/PageScaleCons...
File Source/web/PageScaleConstraintsSet.cpp (right):

https://codereview.chromium.org/53053002/diff/110001/Source/web/PageScaleCons...
Source/web/PageScaleConstraintsSet.cpp:183: if (!description.userZoom) {
On second thought, let's also add "&& m_pageDefinedConstraints.initialScale !=
oldInitialScale" here to make it clear this only has an effect if the above code
changes it.

[eseidel, 2013-11-05 20:09:53]

lgtm

https://codereview.chromium.org/53053002/diff/110001/Source/web/PageScaleCons...
File Source/web/PageScaleConstraintsSet.h (right):

https://codereview.chromium.org/53053002/diff/110001/Source/web/PageScaleCons...
Source/web/PageScaleConstraintsSet.h:52: void
adjustForAndroidWebViewQuirks(const WebCore::ViewportDescription&,
WebCore::IntSize viewSize, int layoutFallbackWidth, float deviceScaleFactor,
bool supportTargetDensityDPI, bool wideViewportQuirkEnabled, bool
useWideViewport, bool loadWithOverviewMode, bool nonUserScalableQuirkEnabled);
This is almost ready to use a struct to hold its args. :)

https://codereview.chromium.org/53053002/diff/110001/Source/web/WebSettingsIm...
File Source/web/WebSettingsImpl.h (right):

https://codereview.chromium.org/53053002/diff/110001/Source/web/WebSettingsIm...
Source/web/WebSettingsImpl.h:214: // This quirk is to maintain compatibility
with Android apps built on
Thank you!

[mnaganov (inactive), 2013-11-06 09:53:06]

Thanks Alexandre and Eric!

https://codereview.chromium.org/53053002/diff/110001/Source/web/PageScaleCons...
File Source/web/PageScaleConstraintsSet.cpp (right):

https://codereview.chromium.org/53053002/diff/110001/Source/web/PageScaleCons...
Source/web/PageScaleConstraintsSet.cpp:183: if (!description.userZoom) {
On 2013/11/05 19:39:09, aelias wrote:
> On second thought, let's also add "&& m_pageDefinedConstraints.initialScale !=
> oldInitialScale" here to make it clear this only has an effect if the above
code
> changes it.

Done.

[commit-bot: I haz the power, 2013-11-06 09:54:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mnaganov@chromium.org/53053002/210001

[commit-bot: I haz the power, 2013-11-06 11:00:46]

Change committed as 161448

[aelias_OOO_until_Jul13, 2013-11-06 20:45:25]

https://codereview.chromium.org/53053002/diff/210001/Source/web/PageScaleCons...
File Source/web/PageScaleConstraintsSet.cpp (right):

https://codereview.chromium.org/53053002/diff/210001/Source/web/PageScaleCons...
Source/web/PageScaleConstraintsSet.cpp:189:
m_pageDefinedConstraints.minimumScale = m_pageDefinedConstraints.initialScale;
Hmm, I don't think this should've been moved here in the last patchset.  Now if
the wideViewportQuirk block changed initial-scale, user-scalable=no doesn't work
properly again.  It's a pre-existing bug but it would've been nice to fix.

[mnaganov (inactive), 2013-11-06 20:54:20]

On 2013/11/06 20:45:25, aelias wrote:
>
https://codereview.chromium.org/53053002/diff/210001/Source/web/PageScaleCons...
> File Source/web/PageScaleConstraintsSet.cpp (right):
> 
>
https://codereview.chromium.org/53053002/diff/210001/Source/web/PageScaleCons...
> Source/web/PageScaleConstraintsSet.cpp:189:
> m_pageDefinedConstraints.minimumScale = m_pageDefinedConstraints.initialScale;
> Hmm, I don't think this should've been moved here in the last patchset.  Now
if
> the wideViewportQuirk block changed initial-scale, user-scalable=no doesn't
work
> properly again.  It's a pre-existing bug but it would've been nice to fix.

Right. I've misread your proposal. Will fix tomorrow.

[aelias_OOO_until_Jul13, 2013-11-06 20:55:10]

OK, thanks!  Not that big a deal, but maybe it could be folded into another
related patch.