Enable runtime features with status=test before WebKit

Enable runtime features with status=test before WebKit

There was a sligh difference in how status=test and status=experimental were
handled. Content turned on experimental features before initializing webkit and
the content shell enabled test features *after* webkit was started. This is a
problem when the feature alters webkit initialization.

This cl moves enabling test features to the place where experimental features
are enabled.

BUG=268240
LOG=no

Committed: https://crrev.com/852ffcf3023df225ba0888efcf1e0b60bec40a14
Cr-Commit-Position: refs/heads/master@{#419192}

[Marcel Hlopko, 2016-09-16 11:16:20]

hlopko@chromium.org changed reviewers:
+ clamy@chromium.org, haraken@chromium.org, jochen@chromium.org

[Marcel Hlopko, 2016-09-16 11:16:28]

The CQ bit was checked by hlopko@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-16 11:16:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Marcel Hlopko, 2016-09-16 11:17:46]

Ptal

[jochen (gone - plz use gerrit), 2016-09-16 11:18:40]

I already did that (and cc'd you on the CL :) )

[clamy, 2016-09-16 12:00:13]

Thanks! A few questions below.

https://codereview.chromium.org/2345233002/diff/1/content/public/renderer/con...
File content/public/renderer/content_renderer_client.h (right):

https://codereview.chromium.org/2345233002/diff/1/content/public/renderer/con...
content/public/renderer/content_renderer_client.h:343: virtual void
SetRuntimeFeatures() {}
Maybe the function name could make it more explicit that this is intended to run
before the initialization of Webkit? While it is precised in the comment, this
is not clear from the function name.

https://codereview.chromium.org/2345233002/diff/1/content/renderer/render_thr...
File content/renderer/render_thread_impl.cc (right):

https://codereview.chromium.org/2345233002/diff/1/content/renderer/render_thr...
content/renderer/render_thread_impl.cc:1167:
GetContentClient()->renderer()->SetRuntimeFeatures();
Quick sanity check: we're sure the ContentRendererClient will have been properly
initialized at that time? (I'm not familiar with the order of initialization in
the renderer).

[jochen (gone - plz use gerrit), 2016-09-16 12:25:08]

btw, I closed my CL. This one lgtm from myside, but please also address clamy@'s
comments

https://codereview.chromium.org/2345233002/diff/1/content/renderer/render_thr...
File content/renderer/render_thread_impl.cc (right):

https://codereview.chromium.org/2345233002/diff/1/content/renderer/render_thr...
content/renderer/render_thread_impl.cc:1167:
GetContentClient()->renderer()->SetRuntimeFeatures();
On 2016/09/16 at 12:00:13, clamy wrote:
> Quick sanity check: we're sure the ContentRendererClient will have been
properly initialized at that time? (I'm not familiar with the order of
initialization in the renderer).

yeah, that's guaranteed

[commit-bot: I haz the power, 2016-09-16 12:29:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-16 12:29:29]

Dry run: This issue passed the CQ dry run.

[haraken, 2016-09-16 12:42:59]

LGTM 2.

[Marcel Hlopko, 2016-09-16 14:57:23]

On 2016/09/16 at 11:18:40, jochen wrote:
> I already did that (and cc'd you on the CL :) )

Only if I checked my chromium inbox folder :) Sorry about that.

[Marcel Hlopko, 2016-09-16 15:09:00]

The CQ bit was checked by hlopko@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-16 15:09:18]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Marcel Hlopko, 2016-09-16 15:11:44]

The CQ bit was checked by hlopko@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-16 15:12:28]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Marcel Hlopko, 2016-09-16 15:13:37]

I addressed comments and moved other related code over to the callback (as
jochen did in his cl). Ptal, thanks :)

https://codereview.chromium.org/2345233002/diff/1/content/public/renderer/con...
File content/public/renderer/content_renderer_client.h (right):

https://codereview.chromium.org/2345233002/diff/1/content/public/renderer/con...
content/public/renderer/content_renderer_client.h:343: virtual void
SetRuntimeFeatures() {}
On 2016/09/16 at 12:00:12, clamy wrote:
> Maybe the function name could make it more explicit that this is intended to
run before the initialization of Webkit? While it is precised in the comment,
this is not clear from the function name.

Done.

[clamy, 2016-09-16 15:20:03]

Thanks! Lgtm with one comment.

https://codereview.chromium.org/2345233002/diff/20001/content/public/renderer...
File content/public/renderer/content_renderer_client.h (right):

https://codereview.chromium.org/2345233002/diff/20001/content/public/renderer...
content/public/renderer/content_renderer_client.h:343: virtual void
SetRuntimeFeaturesDefaultsBeforeWebKitInitialization() {}
s/WebKit/Blink

[jochen (gone - plz use gerrit), 2016-09-16 15:32:45]

https://codereview.chromium.org/2345233002/diff/40001/content/shell/renderer/...
File content/shell/renderer/layout_test/layout_test_content_renderer_client.cc
(right):

https://codereview.chromium.org/2345233002/diff/40001/content/shell/renderer/...
content/shell/renderer/layout_test/layout_test_content_renderer_client.cc:255:
EnableRendererLayoutTestMode();
This will crash windows

[Marcel Hlopko, 2016-09-16 15:37:20]

The CQ bit was checked by hlopko@chromium.org

[Marcel Hlopko, 2016-09-16 15:37:21]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, jochen@chromium.org, clamy@chromium.org
Link to the patchset: https://codereview.chromium.org/2345233002/#ps60001
(title: "Not crashing windows")

[commit-bot: I haz the power, 2016-09-16 15:37:49]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-16 16:51:22]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-09-16 16:54:20]

Description was changed from

==========
Enable runtime features with status=test before WebKit

There was a sligh difference in how status=test and status=experimental were
handled. Content turned on experimental features before initializing webkit and
the content shell enabled test features *after* webkit was started. This is a
problem when the feature alters webkit initialization.

This cl moves enabling test features to the place where experimental features
are enabled.

BUG=268240
LOG=no
==========

to

==========
Enable runtime features with status=test before WebKit

There was a sligh difference in how status=test and status=experimental were
handled. Content turned on experimental features before initializing webkit and
the content shell enabled test features *after* webkit was started. This is a
problem when the feature alters webkit initialization.

This cl moves enabling test features to the place where experimental features
are enabled.

BUG=268240
LOG=no

Committed: https://crrev.com/852ffcf3023df225ba0888efcf1e0b60bec40a14
Cr-Commit-Position: refs/heads/master@{#419192}
==========

[commit-bot: I haz the power, 2016-09-16 16:54:21]

Patchset 4 (id:??) landed as
https://crrev.com/852ffcf3023df225ba0888efcf1e0b60bec40a14
Cr-Commit-Position: refs/heads/master@{#419192}