Experimental viewport meta tag support for desktop

Experimental viewport meta tag support for desktop

Added flag to allow desktop builds to experimentally turn
on support for the viewport meta tag. The --enable-viewport
flag now turns on only support for @viewport.
--enable-viewport-meta turns on viewport meta and @viewport
support.

On Android, the viewport gets rescaled when the view is
resized, i.e. phone is rotated between landscape and
portrait. On desktop, enabling the viewport causes the page
to zoom out when the window is shrunk, rather than adding
scrollbars. To support viewport on both devices, I've added
a --main-frame-resizes-are-orientation-changes flag that should be set on
Android and not desktop. This prevents the viewport scaling
code in WebViewImpl::resize().

Enabling viewport on desktop means the page scale can
change. This causes a known issue with scrollbars where
mouse events are scaled with the page, making mouse
interaction with scrollbars broken.

This is the Chromium side of a 2-side patch
(Blink-side at https://codereview.chromium.org/40423003/)

BUG=232102
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=234617

[bokan, 2013-10-24 16:11:40]

The flags I added would need to be set on Clank by default (replace
--enable-viewport with --enable-viewport-meta and --scale-viewport-on-resize).
If that's a problem I can change the default and negate the flags so we set them
on Chrome instead.

[aelias_OOO_until_Jul13, 2013-10-25 04:13:27]

https://codereview.chromium.org/38793007/diff/1/content/browser/web_contents/...
File content/browser/web_contents/web_contents_impl.cc (right):

https://codereview.chromium.org/38793007/diff/1/content/browser/web_contents/...
content/browser/web_contents/web_contents_impl.cc:671:
prefs.viewport_meta_enabled;
This effectively turns on @viewport support for Android which we are not ready
to ship yet.  Please remove the || and keep the flags entirely separate.

[kenneth.r.christiansen, 2013-10-25 06:27:01]

https://codereview.chromium.org/38793007/diff/1/chrome/browser/chromeos/login...
File chrome/browser/chromeos/login/chrome_restart_request.cc (right):

https://codereview.chromium.org/38793007/diff/1/chrome/browser/chromeos/login...
chrome/browser/chromeos/login/chrome_restart_request.cc:110:
::switches::kScaleViewportOnResize,
It cannot always scale and will overflow. Not sure if we can find a better
wording.

Why can't this be used on desktop? I mean is it a problem when you make the view
bigger or smaller?

I think what you really want here is whether to apply the aspect-ratio scale
difference (if not restricted) between orientation changes. 

That is only related to some specific kind of resize.

kApplyViewportAspectRatioScaleDifference ?

https://codereview.chromium.org/38793007/diff/1/content/browser/web_contents/...
File content/browser/web_contents/web_contents_impl.cc (right):

https://codereview.chromium.org/38793007/diff/1/content/browser/web_contents/...
content/browser/web_contents/web_contents_impl.cc:671:
prefs.viewport_meta_enabled;
On 2013/10/25 04:13:27, aelias wrote:
> This effectively turns on @viewport support for Android which we are not ready
> to ship yet.  Please remove the || and keep the flags entirely separate.

There is a separate runtime flag for that only enabled with
--enable-experimental-web-platform-features

[bokan, 2013-10-25 17:19:33]

https://codereview.chromium.org/38793007/diff/1/chrome/browser/chromeos/login...
File chrome/browser/chromeos/login/chrome_restart_request.cc (right):

https://codereview.chromium.org/38793007/diff/1/chrome/browser/chromeos/login...
chrome/browser/chromeos/login/chrome_restart_request.cc:110:
::switches::kScaleViewportOnResize,
On 2013/10/25 06:27:01, kenneth.r.christiansen wrote:
> It cannot always scale and will overflow. Not sure if we can find a better
> wording.
> 
> Why can't this be used on desktop? I mean is it a problem when you make the
view
> bigger or smaller?
> 
> I think what you really want here is whether to apply the aspect-ratio scale
> difference (if not restricted) between orientation changes. 
> 
> That is only related to some specific kind of resize.
> 
> kApplyViewportAspectRatioScaleDifference ?

On desktop, if viewport is enabled and we have this flag (it's currently
implicitly on), shrinking the window will resize the page so that we don't get
horizontal overflow since the page thinks we're changing orientation. This
happens even on pages without a viewport tag.

You're correct in that we're really after is to distinguish between these two
kinds of resizes. Alex suggested MainFrameResizesAreOrientationChanges. I think
it's more clear.

https://codereview.chromium.org/38793007/diff/1/content/browser/web_contents/...
File content/browser/web_contents/web_contents_impl.cc (right):

https://codereview.chromium.org/38793007/diff/1/content/browser/web_contents/...
content/browser/web_contents/web_contents_impl.cc:671:
prefs.viewport_meta_enabled;
On 2013/10/25 04:13:27, aelias wrote:
> This effectively turns on @viewport support for Android which we are not ready
> to ship yet.  Please remove the || and keep the flags entirely separate.

--viewport-enabled is already on in Android and I didn't change how it behaves
except that it needs --viewport-meta-enabled to process any legacy viewport
tags. As Kenneth mentions, you need an additional flag to enable @viewport. The
idea here is that eventually we'd be able to turn on @viewport (using
--enable-viewport) without turning on viewport meta. Android would then add
--enable-viewport-meta to support both.

https://codereview.chromium.org/38793007/diff/1/content/browser/web_contents/...
content/browser/web_contents/web_contents_impl.cc:671:
prefs.viewport_meta_enabled;
On 2013/10/25 06:27:01, kenneth.r.christiansen wrote:
> On 2013/10/25 04:13:27, aelias wrote:
> > This effectively turns on @viewport support for Android which we are not
ready
> > to ship yet.  Please remove the || and keep the flags entirely separate.
> 
> There is a separate runtime flag for that only enabled with
> --enable-experimental-web-platform-features
I was actually unaware that --enable-viewport doesn't turn on @viewport. Does
the experimental flag only apply to author stylesheets? The UA stylesheet has an
@viewport rule in it that seems to be respected (in WebFrame tests at least)
without the additional flag. Is this correct?

[aelias_OOO_until_Jul13, 2013-10-31 20:54:26]

lgtm.  Adding darin@ for OWNERS (this patch just adds new flags and plumbs them
to Blink)

https://codereview.chromium.org/38793007/diff/1/content/browser/web_contents/...
File content/browser/web_contents/web_contents_impl.cc (right):

https://codereview.chromium.org/38793007/diff/1/content/browser/web_contents/...
content/browser/web_contents/web_contents_impl.cc:671:
prefs.viewport_meta_enabled;
On 2013/10/25 17:19:33, bokan wrote:
> I was actually unaware that --enable-viewport doesn't turn on @viewport. Does
> the experimental flag only apply to author stylesheets? The UA stylesheet has
an
> @viewport rule in it that seems to be respected (in WebFrame tests at least)
> without the additional flag. Is this correct?

Yes, as discussed on the Blink-side patch, it's always respected for UA
stylesheets.

[bokan, 2013-10-31 21:02:19]

On 2013/10/31 20:54:26, aelias wrote:
> lgtm.  Adding darin@ for OWNERS (this patch just adds new flags and plumbs
them
> to Blink)
> 
>
https://codereview.chromium.org/38793007/diff/1/content/browser/web_contents/...
> File content/browser/web_contents/web_contents_impl.cc (right):
> 
>
https://codereview.chromium.org/38793007/diff/1/content/browser/web_contents/...
> content/browser/web_contents/web_contents_impl.cc:671:
> prefs.viewport_meta_enabled;
> On 2013/10/25 17:19:33, bokan wrote:
> > I was actually unaware that --enable-viewport doesn't turn on @viewport.
Does
> > the experimental flag only apply to author stylesheets? The UA stylesheet
has
> an
> > @viewport rule in it that seems to be respected (in WebFrame tests at least)
> > without the additional flag. Is this correct?
> 
> Yes, as discussed on the Blink-side patch, it's always respected for UA
> stylesheets.

I'm assuming I should land the Blink CL first to avoid breaking the build?
Should I change the values for the enableViewportMeta and
resizesAreOrientationChange in that CL to depend on --viewport-enabled until
this lands so that we can land these independently without issue?

[aelias_OOO_until_Jul13, 2013-11-01 00:05:57]

On 2013/10/31 21:02:19, bokan wrote:
> I'm assuming I should land the Blink CL first to avoid breaking the build?
> Should I change the values for the enableViewportMeta and
> resizesAreOrientationChange in that CL to depend on --viewport-enabled until
> this lands so that we can land these independently without issue?

Yes, that sounds fine.

[bokan, 2013-11-06 23:30:25]

Blink side has landed. Darin, please take a look.

Thanks.

[kenneth.r.christiansen, 2013-11-07 09:06:26]

https://codereview.chromium.org/38793007/diff/150001/content/public/common/co...
File content/public/common/content_switches.h (right):

https://codereview.chromium.org/38793007/diff/150001/content/public/common/co...
content/public/common/content_switches.h:170: extern const char
kEnableViewportMeta[];
What is the rule for CONTENT_EXPORTing these or not? I have had to redeclare
some of these for using them in our other project based on Chromium.

[bokan, 2013-11-12 00:51:53]

On 2013/11/07 09:06:26, kenneth.r.christiansen wrote:
>
https://codereview.chromium.org/38793007/diff/150001/content/public/common/co...
> File content/public/common/content_switches.h (right):
> 
>
https://codereview.chromium.org/38793007/diff/150001/content/public/common/co...
> content/public/common/content_switches.h:170: extern const char
> kEnableViewportMeta[];
> What is the rule for CONTENT_EXPORTing these or not? I have had to redeclare
> some of these for using them in our other project based on Chromium.

I'm actually not sure, someone else will have to answer that.

[darin (slow to review), 2013-11-12 05:12:28]

LGTM

[commit-bot: I haz the power, 2013-11-12 14:49:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bokan@chromium.org/38793007/230001

[commit-bot: I haz the power, 2013-11-12 18:43:25]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2013-11-12 19:29:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bokan@chromium.org/38793007/230001

[commit-bot: I haz the power, 2013-11-12 20:15:30]

Change committed as 234617