[DevTools]: Expose V8 version in /json/version

[DevTools]: Expose V8 version in /json/version

BUG=660240

[eostroukhov, 2016-11-04 00:23:11]

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

[eostroukhov, 2016-11-04 00:23:44]

eostroukhov@chromium.org changed reviewers:
+ dgozman@chromium.org, pfeldman@chromium.org

[eostroukhov, 2016-11-04 00:23:44]

Please take a look.

[commit-bot: I haz the power, 2016-11-04 00:24:17]

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

[dgozman, 2016-11-04 00:25:20]

https://codereview.chromium.org/2472093003/diff/1/content/browser/devtools/de...
File content/browser/devtools/devtools_http_handler.cc (right):

https://codereview.chromium.org/2472093003/diff/1/content/browser/devtools/de...
content/browser/devtools/devtools_http_handler.cc:527:
version.SetString("Js-Engine-Version", js_engine_version_);
Let's call it V8-Version. It's no worse than WebKit-Version.

[eostroukhov, 2016-11-04 00:40:01]

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

[commit-bot: I haz the power, 2016-11-04 00:40:19]

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

[eostroukhov, 2016-11-04 00:41:02]

Thanks for the review. Please consider the updated version.

https://codereview.chromium.org/2472093003/diff/1/content/browser/devtools/de...
File content/browser/devtools/devtools_http_handler.cc (right):

https://codereview.chromium.org/2472093003/diff/1/content/browser/devtools/de...
content/browser/devtools/devtools_http_handler.cc:527:
version.SetString("Js-Engine-Version", js_engine_version_);
On 2016/11/04 00:25:20, dgozman wrote:
> Let's call it V8-Version. It's no worse than WebKit-Version.

Done.

[commit-bot: I haz the power, 2016-11-04 00:53:10]

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

[commit-bot: I haz the power, 2016-11-04 00:53:10]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[dgozman, 2016-11-04 01:06:37]

Thanks, lgtm!

[eostroukhov, 2016-11-04 16:48:54]

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

[commit-bot: I haz the power, 2016-11-04 16:49:22]

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

[eostroukhov, 2016-11-04 16:50:38]

The CQ bit was unchecked by eostroukhov@chromium.org

[eostroukhov, 2016-11-04 16:50:41]

The CQ bit was checked by eostroukhov@chromium.org

[eostroukhov, 2016-11-04 16:50:41]

The patchset sent to the CQ was uploaded after l-g-t-m from dgozman@chromium.org
Link to the patchset: https://codereview.chromium.org/2472093003/#ps40001
(title: "Missed two callsites")

[eostroukhov, 2016-11-04 16:50:45]

The CQ bit was checked by eostroukhov@chromium.org

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

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-11-04 16:59:43]

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

[commit-bot: I haz the power, 2016-11-04 16:59:43]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[eostroukhov, 2016-11-04 17:23:15]

Please take another look, I had to add 2 more files to the CL.

[dgozman, 2016-11-16 05:50:01]

What's the status of this?

[eostroukhov, 2016-11-16 17:28:38]

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

[commit-bot: I haz the power, 2016-11-16 17:29:00]

Dry run: 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-11-16 17:47:23]

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

[commit-bot: I haz the power, 2016-11-16 17:47:24]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[eostroukhov, 2016-11-17 23:12:59]

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

[commit-bot: I haz the power, 2016-11-17 23:14:18]

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

[eostroukhov, 2016-11-17 23:14:26]

eostroukhov@chromium.org changed reviewers:
+ halliwell@chromium.org

[eostroukhov, 2016-11-17 23:14:27]

There was a compiler error in Chromecast so I had to add another file. I also
need OWNER approvals.

[halliwell, 2016-11-17 23:22:26]

https://codereview.chromium.org/2472093003/diff/60001/chromecast/browser/devt...
File chromecast/browser/devtools/remote_debugging_server.cc (right):

https://codereview.chromium.org/2472093003/diff/60001/chromecast/browser/devt...
chromecast/browser/devtools/remote_debugging_server.cc:167: std::string());
Why aren't you passing the actual v8 version here like chrome does?  What is the
implication?

[commit-bot: I haz the power, 2016-11-17 23:52:12]

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

[commit-bot: I haz the power, 2016-11-17 23:52:14]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)

[eostroukhov, 2016-11-17 23:58:58]

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

[commit-bot: I haz the power, 2016-11-18 00:00:07]

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

[eostroukhov, 2016-11-18 00:02:23]

eostroukhov@chromium.org changed reviewers:
+ michaelbai@chromium.org

[eostroukhov, 2016-11-18 00:02:23]

https://codereview.chromium.org/2472093003/diff/60001/chromecast/browser/devt...
File chromecast/browser/devtools/remote_debugging_server.cc (right):

https://codereview.chromium.org/2472093003/diff/60001/chromecast/browser/devt...
chromecast/browser/devtools/remote_debugging_server.cc:167: std::string());
On 2016/11/17 23:22:25, halliwell wrote:
> Why aren't you passing the actual v8 version here like chrome does?  What is
the
> implication?

Linter rules prevent me from including v8.h here, and I am not sure this change
dragging this value through a significant number of files.

This value is for tools frontends to make assumptions about the V8 version, that
is tied to inspector protocol version.

[halliwell, 2016-11-18 00:11:23]

On 2016/11/18 00:02:23, eostroukhov wrote:
>
https://codereview.chromium.org/2472093003/diff/60001/chromecast/browser/devt...
> File chromecast/browser/devtools/remote_debugging_server.cc (right):
> 
>
https://codereview.chromium.org/2472093003/diff/60001/chromecast/browser/devt...
> chromecast/browser/devtools/remote_debugging_server.cc:167: std::string());
> On 2016/11/17 23:22:25, halliwell wrote:
> > Why aren't you passing the actual v8 version here like chrome does?  What is
> the
> > implication?
> 
> Linter rules prevent me from including v8.h here, and I am not sure this
change
> dragging this value through a significant number of files.
> 

What rules.  Are you talking about DEPS?  If it's allowed from
chrome/browser/devtools, it should be ok from chromecast/browser/devtools.

> This value is for tools frontends to make assumptions about the V8 version,
that
> is tied to inspector protocol version.

Right, so what will happen when inspecting with devtools on Chromecast?

[commit-bot: I haz the power, 2016-11-18 00:27:20]

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

[commit-bot: I haz the power, 2016-11-18 00:27:21]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[eostroukhov, 2016-11-18 17:28:34]

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

[eostroukhov, 2016-11-18 17:29:25]

eostroukhov@chromium.org changed reviewers:
+ dtrainor@chromium.org, jochen@chromium.org

[commit-bot: I haz the power, 2016-11-18 17:29:29]

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

[eostroukhov, 2016-11-18 17:30:24]

On 2016/11/18 00:11:23, halliwell wrote:
> On 2016/11/18 00:02:23, eostroukhov wrote:
> >
>
https://codereview.chromium.org/2472093003/diff/60001/chromecast/browser/devt...
> > File chromecast/browser/devtools/remote_debugging_server.cc (right):
> > 
> >
>
https://codereview.chromium.org/2472093003/diff/60001/chromecast/browser/devt...
> > chromecast/browser/devtools/remote_debugging_server.cc:167: std::string());
> > On 2016/11/17 23:22:25, halliwell wrote:
> > > Why aren't you passing the actual v8 version here like chrome does?  What
is
> > the
> > > implication?
> > 
> > Linter rules prevent me from including v8.h here, and I am not sure this
> change
> > dragging this value through a significant number of files.
> > 
> 
> What rules.  Are you talking about DEPS?  If it's allowed from
> chrome/browser/devtools, it should be ok from chromecast/browser/devtools.

I updated the DEPS.

> > This value is for tools frontends to make assumptions about the V8 version,
> that
> > is tied to inspector protocol version.
> 
> Right, so what will happen when inspecting with devtools on Chromecast?

This field is not yet used by the front-ends :)

[commit-bot: I haz the power, 2016-11-18 17:40:57]

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

[commit-bot: I haz the power, 2016-11-18 17:40:58]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[halliwell, 2016-11-18 18:10:18]

On 2016/11/18 17:30:24, eostroukhov wrote:
> On 2016/11/18 00:11:23, halliwell wrote:
> > On 2016/11/18 00:02:23, eostroukhov wrote:
> > >
> >
>
https://codereview.chromium.org/2472093003/diff/60001/chromecast/browser/devt...
> > > File chromecast/browser/devtools/remote_debugging_server.cc (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2472093003/diff/60001/chromecast/browser/devt...
> > > chromecast/browser/devtools/remote_debugging_server.cc:167:
std::string());
> > > On 2016/11/17 23:22:25, halliwell wrote:
> > > > Why aren't you passing the actual v8 version here like chrome does? 
What
> is
> > > the
> > > > implication?
> > > 
> > > Linter rules prevent me from including v8.h here, and I am not sure this
> > change
> > > dragging this value through a significant number of files.
> > > 
> > 
> > What rules.  Are you talking about DEPS?  If it's allowed from
> > chrome/browser/devtools, it should be ok from chromecast/browser/devtools.
> 
> I updated the DEPS.
> 
> > > This value is for tools frontends to make assumptions about the V8
version,
> > that
> > > is tied to inspector protocol version.
> > 
> > Right, so what will happen when inspecting with devtools on Chromecast?
> 
> This field is not yet used by the front-ends :)

lgtm

[eostroukhov, 2016-11-18 19:41:17]

Realized that if I have to change the build (e.g. DEPS) then I can just do
https://codereview.chromium.org/2518623002/.

This CL is abandoned.