Allow enabling "CSSViewport" runtime feature outside Blink

Introduce WebRuntimeFeatures::setCSSViewportEnabled() so that it can be called from content::SetRuntimeFeaturesDefaultsAndUpdateFromArgs()
if the "--enable-viewport" command line option is present.

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

[Mikhail, 2014-10-06 15:00:15]

mikhail.pozdnyakov@intel.com changed reviewers:
+ kenneth.r.christiansen@intel.com, mkwst@chromium.org

[Mikhail, 2014-10-06 15:04:59]

PTAL. 
(I'm not 100% sure this is the proper fix: do we need to have both
RuntimeEnabledFeatures::isCSSViewportEnabled and
WebSettingsImpl::viewportEnabled? maybe one of them can be omitted?)

[Mike West, 2014-10-06 15:41:06]

On 2014/10/06 15:04:59, Mikhail wrote:
> PTAL. 
> (I'm not 100% sure this is the proper fix: do we need to have both
> RuntimeEnabledFeatures::isCSSViewportEnabled and
> WebSettingsImpl::viewportEnabled? maybe one of them can be omitted?)

The many failing tests make me suspect that this isn't the right thing to do.

[Mikhail, 2014-10-07 14:22:46]

On 2014/10/06 15:41:06, Mike West wrote:
> On 2014/10/06 15:04:59, Mikhail wrote:
> > PTAL. 
> > (I'm not 100% sure this is the proper fix: do we need to have both
> > RuntimeEnabledFeatures::isCSSViewportEnabled and
> > WebSettingsImpl::viewportEnabled? maybe one of them can be omitted?)
> 
> The many failing tests make me suspect that this isn't the right thing to do.

Right, though the situation with tests is a bit strange because
WebSettingsImpl::viewportEnabled is unset(!) when viewport layout tests are
running.
The new fix consists of two parts: exporting "CSSViewport" feature from Blink
(this patch) and setting it from chromium
(https://codereview.chromium.org/633943002).

[Mike West, 2014-10-08 12:39:10]

On 2014/10/07 14:22:46, Mikhail wrote:
> Right, though the situation with tests is a bit strange because
> WebSettingsImpl::viewportEnabled is unset(!) when viewport layout tests are
> running.
> The new fix consists of two parts: exporting "CSSViewport" feature from Blink
> (this patch) and setting it from chromium
> (https://codereview.chromium.org/633943002).

Do you just need this as a separate flag? 'CSSViewport' should already be
enabled in tests and via the "Experimental Web Platform Features" flag, as it's
listed as "status=experimental" in RuntimeEnabledFeatures.in.

[Mikhail, 2014-10-08 13:10:37]

On 2014/10/08 12:39:10, Mike West wrote:
> On 2014/10/07 14:22:46, Mikhail wrote:
> > Right, though the situation with tests is a bit strange because
> > WebSettingsImpl::viewportEnabled is unset(!) when viewport layout tests are
> > running.
> > The new fix consists of two parts: exporting "CSSViewport" feature from
Blink
> > (this patch) and setting it from chromium
> > (https://codereview.chromium.org/633943002).
> 
> Do you just need this as a separate flag? 'CSSViewport' should already be
> enabled in tests and via the "Experimental Web Platform Features" flag, as
it's
> listed as "status=experimental" in RuntimeEnabledFeatures.in.

Agree, but when you run  "content_shell --enable-viewport" the 'CSSViewport'
does not get enabled and non-working @viewport is quite confusing (and you might
not always want to enable all the experimental features at the same time just to
make this work).

[Mike West, 2014-10-08 13:17:53]

On 2014/10/08 13:10:37, Mikhail wrote:
> Agree, but when you run  "content_shell --enable-viewport" the 'CSSViewport'
> does not get enabled and non-working @viewport is quite confusing (and you
might
> not always want to enable all the experimental features at the same time just
to
> make this work).

I see. We skip some steps on the Chromium-side in
RenderViewImpl::ProcessViewLayoutFlags if the flag is set, but the blink-side
doesn't get updated. Ok, LGTM. You'll still need someone to sign off on the
public/ changes. Perhaps dglazkov@ or jochen@?

[Mikhail, 2014-10-13 07:48:33]

mikhail.pozdnyakov@intel.com changed reviewers:
+ dglazkov@chromium.org, jochen@chromium.org

[Mikhail, 2014-10-13 07:49:27]

On 2014/10/08 13:17:53, Mike West wrote:
> On 2014/10/08 13:10:37, Mikhail wrote:
> > Agree, but when you run  "content_shell --enable-viewport" the 'CSSViewport'
> > does not get enabled and non-working @viewport is quite confusing (and you
> might
> > not always want to enable all the experimental features at the same time
just
> to
> > make this work).
> 
> I see. We skip some steps on the Chromium-side in
> RenderViewImpl::ProcessViewLayoutFlags if the flag is set, but the blink-side
> doesn't get updated. Ok, LGTM. You'll still need someone to sign off on the
> public/ changes. Perhaps dglazkov@ or jochen@?

Thanks! dglazkov@, jochen@ could you pls have a look?

[Mike West, 2014-10-13 07:56:31]

On 2014/10/13 07:49:27, Mikhail wrote:
> On 2014/10/08 13:17:53, Mike West wrote:
> > On 2014/10/08 13:10:37, Mikhail wrote:
> > > Agree, but when you run  "content_shell --enable-viewport" the
'CSSViewport'
> > > does not get enabled and non-working @viewport is quite confusing (and you
> > might
> > > not always want to enable all the experimental features at the same time
> just
> > to
> > > make this work).
> > 
> > I see. We skip some steps on the Chromium-side in
> > RenderViewImpl::ProcessViewLayoutFlags if the flag is set, but the
blink-side
> > doesn't get updated. Ok, LGTM. You'll still need someone to sign off on the
> > public/ changes. Perhaps dglazkov@ or jochen@?
> 
> Thanks! dglazkov@, jochen@ could you pls have a look?

(In the meantime, I snuck into OWNERS files. You should be able to CQ this.)

[Mikhail, 2014-10-13 08:03:46]

The CQ bit was checked by mikhail.pozdnyakov@intel.com

[commit-bot: I haz the power, 2014-10-13 08:04:19]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/630903002/20001

[commit-bot: I haz the power, 2014-10-13 09:11:42]

Committed patchset #2 (id:20001) as 183593

[Noel Gordon, 2014-10-15 06:22:30]

noel@chromium.org changed reviewers:
+ noel@chromium.org

[Noel Gordon, 2014-10-15 06:22:32]

https://codereview.chromium.org/630903002/diff/20001/Source/web/WebRuntimeFea...
File Source/web/WebRuntimeFeatures.cpp (right):

https://codereview.chromium.org/630903002/diff/20001/Source/web/WebRuntimeFea...
Source/web/WebRuntimeFeatures.cpp:311: void
WebRuntimeFeatures::setCSSViewportEnabled(bool enable)
Naming: should this be called enableCSSViewport(bool enable) like all the other
WebRuntimeFeatures ?