Move SetJavaScriptFlags() from webkit to content.

Remove SetJavaScriptFlags() wrapper function from webkit.

This function is just an helper/wrapper function around v8::V8::SetFlagsFromString() and used only by content.

John preferred to just remove the wrapper and call the v8 API directly in content.

Also removing it from webkit_glue.h helps with the removal of src/webkit/ directory.

BUG=265753
TEST=content_shell and content_unittests
R=jochen@chromium.org, jam@chromium.org
TBR=darin

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

[jochen (gone - plz use gerrit), 2014-01-27 15:08:28]

you'll probably need to make content_child depend on v8.gyp:v8 , no?

https://codereview.chromium.org/147973002/diff/1/content/shell/renderer/DEPS
File content/shell/renderer/DEPS (right):

https://codereview.chromium.org/147973002/diff/1/content/shell/renderer/DEPS#...
content/shell/renderer/DEPS:2: "+content/child/child_thread.h",
content shell can only include public apis

https://codereview.chromium.org/147973002/diff/1/content/shell/renderer/shell...
File content/shell/renderer/shell_render_process_observer.cc (right):

https://codereview.chromium.org/147973002/diff/1/content/shell/renderer/shell...
content/shell/renderer/shell_render_process_observer.cc:69:
ChildThread::SetJavaScriptFlags(" --expose-gc");
instead of calling this here, move this to EnableRendererLayoutTestMode()

[tfarina, 2014-01-27 23:35:47]

On 2014/01/27 15:08:28, jochen wrote:
> you'll probably need to make content_child depend on v8.gyp:v8 , no?
> 
Done.

https://codereview.chromium.org/147973002/diff/1/content/shell/renderer/DEPS
File content/shell/renderer/DEPS (right):

https://codereview.chromium.org/147973002/diff/1/content/shell/renderer/DEPS#...
content/shell/renderer/DEPS:2: "+content/child/child_thread.h",
On 2014/01/27 15:08:29, jochen wrote:
> content shell can only include public apis

Done.

https://codereview.chromium.org/147973002/diff/1/content/shell/renderer/shell...
File content/shell/renderer/shell_render_process_observer.cc (right):

https://codereview.chromium.org/147973002/diff/1/content/shell/renderer/shell...
content/shell/renderer/shell_render_process_observer.cc:69:
ChildThread::SetJavaScriptFlags(" --expose-gc");
On 2014/01/27 15:08:29, jochen wrote:
> instead of calling this here, move this to EnableRendererLayoutTestMode()

Done.

[tfarina, 2014-01-27 23:35:52]

On 2014/01/27 15:08:28, jochen wrote:
> you'll probably need to make content_child depend on v8.gyp:v8 , no?
> 
Done.

[jam, 2014-01-28 00:16:28]

this doesn't really seem to belong on ChildThread. it seems simple to call
v8::V8::SetFlagsFromString everywhere. I saw your email about that being awkward
API, but you can just use sizeof() in the places where you have a constant, and
in other places a local string

[tfarina, 2014-01-28 02:31:09]

John, I made the changes you suggested. Please, take another look.

Thanks!

[jam, 2014-01-28 07:01:26]

https://codereview.chromium.org/147973002/diff/230001/content/public/test/ren...
File content/public/test/render_view_test.cc (right):

https://codereview.chromium.org/147973002/diff/230001/content/public/test/ren...
content/public/test/render_view_test.cc:153: static const char expose_gc_flags[]
= "--expose-gc";
nit: seems this would be clearer with just
std::string flags("--expose-gc");
v8::V8::SetFlagsFromString(flags.c_str(), str.size());

also in other places.

https://codereview.chromium.org/147973002/diff/230001/content/worker/worker_t...
File content/worker/worker_thread.cc (right):

https://codereview.chromium.org/147973002/diff/230001/content/worker/worker_t...
content/worker/worker_thread.cc:46:
static_cast<int>(strlen(switches::kJavaScriptFlags)));
nit: just sizeof(switches::...)?

[tfarina, 2014-01-28 12:20:30]

https://codereview.chromium.org/147973002/diff/230001/content/public/test/ren...
File content/public/test/render_view_test.cc (right):

https://codereview.chromium.org/147973002/diff/230001/content/public/test/ren...
content/public/test/render_view_test.cc:153: static const char expose_gc_flags[]
= "--expose-gc";
On 2014/01/28 07:01:26, jam wrote:
> nit: seems this would be clearer with just
> std::string flags("--expose-gc");
> v8::V8::SetFlagsFromString(flags.c_str(), str.size());
> 
> also in other places.

Done.

https://codereview.chromium.org/147973002/diff/230001/content/worker/worker_t...
File content/worker/worker_thread.cc (right):

https://codereview.chromium.org/147973002/diff/230001/content/worker/worker_t...
content/worker/worker_thread.cc:46:
static_cast<int>(strlen(switches::kJavaScriptFlags)));
On 2014/01/28 07:01:26, jam wrote:
> nit: just sizeof(switches::...)?
when I use sizeof here this happens:

../../content/worker/worker_thread.cc:46:15: error: invalid application of
'sizeof' to an incomplete type 'const char []'
        sizeof(switches::kJavaScriptFlags));

[jam, 2014-01-28 15:43:45]

lgtm

https://codereview.chromium.org/147973002/diff/230001/content/worker/worker_t...
File content/worker/worker_thread.cc (right):

https://codereview.chromium.org/147973002/diff/230001/content/worker/worker_t...
content/worker/worker_thread.cc:46:
static_cast<int>(strlen(switches::kJavaScriptFlags)));
On 2014/01/28 12:20:31, tfarina wrote:
> On 2014/01/28 07:01:26, jam wrote:
> > nit: just sizeof(switches::...)?
> when I use sizeof here this happens:
> 
> ../../content/worker/worker_thread.cc:46:15: error: invalid application of
> 'sizeof' to an incomplete type 'const char []'
>         sizeof(switches::kJavaScriptFlags));

oops, sorry yeah that won't work when the constant is in another cc file.

[tfarina, 2014-01-28 16:50:01]

Darin, I'm just TBRing you for webkit/, because I'm just removing an wrapper
function from webkit_glue.h and Jochen and John already reviewed this.

Best,

[tfarina, 2014-01-28 16:54:26]

https://codereview.chromium.org/147973002/diff/250001/content/shell/renderer/...
File content/shell/renderer/shell_render_process_observer.cc (left):

https://codereview.chromium.org/147973002/diff/250001/content/shell/renderer/...
content/shell/renderer/shell_render_process_observer.cc:69:
webkit_glue::SetJavaScriptFlags(" --expose-gc");
looks like removing this from here broke content_browsertests

https://codereview.chromium.org/147973002/diff/250001/content/test/layouttest...
File content/test/layouttest_support.cc (right):

https://codereview.chromium.org/147973002/diff/250001/content/test/layouttest...
content/test/layouttest_support.cc:102: void EnableBrowserLayoutTestMode() {
but adding --expose-gc here too, might fix content_browsertests.

I'll try later tonight.

[jochen (gone - plz use gerrit), 2014-01-28 17:02:00]

Since you're now calling v8 directly, you could just leave the call in
content shell

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[tfarina, 2014-01-29 00:40:14]

On 2014/01/28 17:02:00, jochen wrote:
> Since you're now calling v8 directly, you could just leave the call in
> content shell
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[tfarina, 2014-01-29 00:40:22]

On 2014/01/29 00:40:14, tfarina wrote:
> On 2014/01/28 17:02:00, jochen wrote:
> > Since you're now calling v8 directly, you could just leave the call in
> > content shell
> > 
> > To unsubscribe from this group and stop receiving emails from it, send an
> email
> > to mailto:chromium-reviews+unsubscribe@chromium.org.

[tfarina, 2014-01-29 00:41:08]

On 2014/01/28 17:02:00, jochen wrote:
> Since you're now calling v8 directly, you could just leave the call in
> content shell
> 
My broken mouse is acting weird :( Doing double clicks all the time, sorry for
the email spam!

Done. Test fixed!

[commit-bot: I haz the power, 2014-01-29 00:43:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tfarina@chromium.org/147973002/230002

[commit-bot: I haz the power, 2014-01-29 01:54:58]

Retried try job too often on mac_rel for step(s) remoting_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2014-01-29 02:01:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tfarina@chromium.org/147973002/230002

[commit-bot: I haz the power, 2014-01-29 05:22:15]

Change committed as 247626