[Android WebView] Update viewport size and scale calculation to match WebView Classic

[Android WebView] Update viewport size and scale calculation to match WebView Classic

Tweaked initial scaling and layout width assignment behavior to
align better with the legacy WebView. Some of the tweaks are only
applied when support for target-densityDpi is turned on.

Assume viewport and fixed layout are always enabled. WebView will no 
more toggle 'setViewportEnabled' and 'fixedLayoutMode' flags,
they will be always enabled. This is to avoid skipping "meta viewport"
tag processing.

For controlling wide viewport-related behavior, added a corresponding
WebSetting.

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

[mnaganov (inactive), 2013-04-12 11:53:39]

Hello Alexandre, PTAL!

As we have discussed some time ago, to retain compatibility with the legacy
WebView behavior, we should never turn off viewport and fixed layout, but
instead adjust layout width according to 'UseWideViewport' setting.

I have compared behavior of legacy and Chromium WebViews with respect to the
initial scale and layout width, and made minor corrections to our algorithms. I
think, these changes are indivisible from wide viewport, so they are in a single
patch. The amount of changes isn't big anyway.

[mnaganov (inactive), 2013-04-12 11:59:50]

This is the corresponding Chromium change:
https://codereview.chromium.org/14234002

[aelias_OOO_until_Jul13, 2013-04-15 03:56:00]

lgtm, nits below.  Note you'll need review from a Blink OWNER, adding abarth@.

- Please change the CL title to mention the behavior changes as that's most of
this patch.

- Please merge the two if (m_initialPageScaleFactorOverride != -1) blocks into
one.

[mnaganov (inactive), 2013-04-15 16:29:44]

On 2013/04/15 03:56:00, aelias wrote:
> lgtm, nits below.  Note you'll need review from a Blink OWNER, adding abarth@.
> 
> - Please change the CL title to mention the behavior changes as that's most of
> this patch.
> 

Done.

> - Please merge the two if (m_initialPageScaleFactorOverride != -1) blocks into
> one.

Done.

Adam, please take a look!

[abarth-chromium, 2013-04-15 23:33:57]

I mean, I don't really like this CL.  It's make this code very complicated.  I
don't understand why it needs to be so complicated.  This code seems poorly
factored between Blink and WebView.  The complexity arising from supporting all
these crazy WebView-specific viewports should be in the WebView rather than in
Blink.

Sorry for being grumpy about your CL.  I think it's ok to land (so LGTM), but
please consider if there's a better way to separate these concerns so we can
avoid this nest of if statements and boolean variables.

https://codereview.chromium.org/13880019/diff/11001/Source/WebKit/chromium/sr...
File Source/WebKit/chromium/src/ChromeClientImpl.cpp (right):

https://codereview.chromium.org/13880019/diff/11001/Source/WebKit/chromium/sr...
Source/WebKit/chromium/src/ChromeClientImpl.cpp:667: bool
applyTargetDensityDpiToLayoutSize = true;
Dpi -> DPI

https://codereview.chromium.org/13880019/diff/11001/Source/WebKit/chromium/sr...
Source/WebKit/chromium/src/ChromeClientImpl.cpp:680: }
Why does this need to be so complicated?  Can we structure this code a bit more
(maybe using inline helper functions) to make it more clear what's going on?

[mnaganov (inactive), 2013-04-16 14:41:25]

Thanks, Adam and Alexandre!

https://codereview.chromium.org/13880019/diff/11001/Source/WebKit/chromium/sr...
File Source/WebKit/chromium/src/ChromeClientImpl.cpp (right):

https://codereview.chromium.org/13880019/diff/11001/Source/WebKit/chromium/sr...
Source/WebKit/chromium/src/ChromeClientImpl.cpp:667: bool
applyTargetDensityDpiToLayoutSize = true;
On 2013/04/15 23:33:57, abarth wrote:
> Dpi -> DPI

Done.

https://codereview.chromium.org/13880019/diff/11001/Source/WebKit/chromium/sr...
Source/WebKit/chromium/src/ChromeClientImpl.cpp:680: }
On 2013/04/15 23:33:57, abarth wrote:
> Why does this need to be so complicated?  Can we structure this code a bit
more
> (maybe using inline helper functions) to make it more clear what's going on?

I compared the resulting scales and widths of old and new WebViews in different
combinations of settings and meta viewport arguments, and this was required to
align them.

I brushed this logic a bit, getting rid of this boolean var, and extracting one
case into an inline function. The result is shorter and looks a bit simpler.

I would like to somehow bring this logic on WebView implementation side, but I
have no idea how to do this yet.

[abarth-chromium, 2013-04-16 16:57:18]

Thanks.  I appreciate your iterating on this patch.

[mnaganov (inactive), 2013-04-16 17:32:04]

A note for sheriffs / gardeners. If any of Android Webview tests fail after
rolling this change into Chromium, please disable the tests, don't roll out the
patch.

This change is paired with a Chromium change:
https://codereview.chromium.org/14234002/. I tried the two changes together, and
all tests seem to pass. I also tried uploading the Blink change alone to Android
try bots, but they were not handling it properly. I tested the Blink change
alone locally, but this doesn't guarantee it from failing on Chromium bots.

[mnaganov (inactive), 2013-04-16 17:37:42]

Committed patchset #4 manually as r148480 (presubmit successful).