[DevTools] Move part of CommandLineAPI to native

[DevTools] Move part of CommandLineAPI to native

- ScopeExtension is stored by v8::Private instead v8::Symbol;
- part of CommandLineAPI methods are moved to native;

The benefits are:
- scopeExtension object can not be received in JS and stored somewhere;
- console related methods (dir, dirxml, ...) can't be overridden;
- removed InspectorConsoleAgent::clearAllMessages method;
- removed one more InjectedScriptSource.js part;

BUG=606055, 595206
R=dgozman@chromium.org

Committed: https://crrev.com/2cab1fca0201ab6d6b059bb0057e63973ebbb3c4
Cr-Commit-Position: refs/heads/master@{#389628}

[kozy, 2016-04-25 06:17:42]

Description was changed from

==========
[DevTools] Move part of CommandLineAPI to native

BUG=606055,595206
==========

to

==========
[DevTools] Move part of CommandLineAPI to native

- ScopeExtension is stored by v8::Private instead v8::Symbol;
- part of CommandLineAPI methods are moved to native;

The benefits are:
- scopeExtension object can be received in JS and stored somewhere;
- console related methods (dir, dirxml, ...) can't be overridden;
- removed InspectorConsoleAgent::clearAllMessages method;
- removed one more InjectedScriptSource.js part; 

BUG=606055,595206
R=dgozman@chromium.org
==========

[kozy, 2016-04-25 06:17:42]

kozyatinskiy@chromium.org changed reviewers:
+ dgozman@chromium.org

[kozy, 2016-04-25 06:52:47]

Description was changed from

==========
[DevTools] Move part of CommandLineAPI to native

- ScopeExtension is stored by v8::Private instead v8::Symbol;
- part of CommandLineAPI methods are moved to native;

The benefits are:
- scopeExtension object can be received in JS and stored somewhere;
- console related methods (dir, dirxml, ...) can't be overridden;
- removed InspectorConsoleAgent::clearAllMessages method;
- removed one more InjectedScriptSource.js part; 

BUG=606055,595206
R=dgozman@chromium.org
==========

to

==========
[DevTools] Move part of CommandLineAPI to native

- ScopeExtension is stored by v8::Private instead v8::Symbol;
- part of CommandLineAPI methods are moved to native;

The benefits are:
- scopeExtension object can be received in JS and stored somewhere;
- console related methods (dir, dirxml, ...) can't be overridden;
- removed InspectorConsoleAgent::clearAllMessages method;
- removed one more InjectedScriptSource.js part; 

With this CL Command Line API clear method calls console.clear directly. Without
it calls InspectorConsoleAgent::clearAllMessages, this method in case of
PageConsoleAgent additionally clears node that was pushed to frontend.

BUG=606055,595206
R=dgozman@chromium.org
==========

[kozy, 2016-04-25 15:31:23]

Description was changed from

==========
[DevTools] Move part of CommandLineAPI to native

- ScopeExtension is stored by v8::Private instead v8::Symbol;
- part of CommandLineAPI methods are moved to native;

The benefits are:
- scopeExtension object can be received in JS and stored somewhere;
- console related methods (dir, dirxml, ...) can't be overridden;
- removed InspectorConsoleAgent::clearAllMessages method;
- removed one more InjectedScriptSource.js part; 

With this CL Command Line API clear method calls console.clear directly. Without
it calls InspectorConsoleAgent::clearAllMessages, this method in case of
PageConsoleAgent additionally clears node that was pushed to frontend.

BUG=606055,595206
R=dgozman@chromium.org
==========

to

==========
[DevTools] Move part of CommandLineAPI to native

- ScopeExtension is stored by v8::Private instead v8::Symbol;
- part of CommandLineAPI methods are moved to native;

The benefits are:
- scopeExtension object can not be received in JS and stored somewhere;
- console related methods (dir, dirxml, ...) can't be overridden;
- removed InspectorConsoleAgent::clearAllMessages method;
- removed one more InjectedScriptSource.js part; 

With this CL Command Line API clear method calls console.clear directly. Without
it calls InspectorConsoleAgent::clearAllMessages, this method in case of
PageConsoleAgent additionally clears node that was pushed to frontend.

BUG=606055,595206
R=dgozman@chromium.org
==========

[kozy, 2016-04-25 15:36:05]

Dmitry, please take a look.

[dgozman, 2016-04-25 18:09:50]

Looking good.

https://codereview.chromium.org/1917733002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/v8_inspector/InjectedScript.cpp (right):

https://codereview.chromium.org/1917733002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/InjectedScript.cpp:414:
function.appendArgument(V8Console::createCommandLineAPI(m_context,
m_injectedScript->context()));
InspectedContext is enough to pass.

https://codereview.chromium.org/1917733002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/v8_inspector/InjectedScriptSource.js
(right):

https://codereview.chromium.org/1917733002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/InjectedScriptSource.js:620:
function customToStringMethod(name)
Let's have a big map {method -> toString} for all methods including native ones.

https://codereview.chromium.org/1917733002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/v8_inspector/V8Console.cpp (right):

https://codereview.chromium.org/1917733002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/V8Console.cpp:511: if
(V8DebuggerAgentImpl* debuggerAgent = helper.debuggerAgent()) {
nit: early bailout

https://codereview.chromium.org/1917733002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/V8Console.cpp:550:
v8::Local<v8::Function> function = v8::Local<v8::Function>::Cast(info[0]);
You already got this at line 545.

https://codereview.chromium.org/1917733002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/V8Console.cpp:660: for (size_t i
= 0; i < sizeof(members) / sizeof(const char*); ++i)
Not yours, but let's change to WTF_ARRAY_LENGTH.

https://codereview.chromium.org/1917733002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/V8Console.cpp:671: for (size_t i
= 0; i < sizeof(members) / sizeof(const char*); ++i)
ditto

https://codereview.chromium.org/1917733002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/v8_inspector/V8Console.h (right):

https://codereview.chromium.org/1917733002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/V8Console.h:19: static
v8::Local<v8::Object> createCommandLineAPI(v8::Local<v8::Context>,
InspectedContext*);
Can we create it only once lazily and reuse?

[kozy, 2016-04-25 19:25:43]

Patchset #4 (id:60001) has been deleted

[kozy, 2016-04-25 19:30:17]

Patchset #4 (id:80001) has been deleted

[kozy, 2016-04-25 19:30:27]

Patchset #4 (id:100001) has been deleted

[kozy, 2016-04-25 19:31:07]

All done.
Please take a look.

https://codereview.chromium.org/1917733002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/v8_inspector/InjectedScript.cpp (right):

https://codereview.chromium.org/1917733002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/InjectedScript.cpp:414:
function.appendArgument(V8Console::createCommandLineAPI(m_context,
m_injectedScript->context()));
On 2016/04/25 18:09:50, dgozman wrote:
> InspectedContext is enough to pass.

Done.

https://codereview.chromium.org/1917733002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/v8_inspector/InjectedScriptSource.js
(right):

https://codereview.chromium.org/1917733002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/InjectedScriptSource.js:620:
function customToStringMethod(name)
On 2016/04/25 18:09:50, dgozman wrote:
> Let's have a big map {method -> toString} for all methods including native
ones.

Done.

https://codereview.chromium.org/1917733002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/v8_inspector/V8Console.cpp (right):

https://codereview.chromium.org/1917733002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/V8Console.cpp:511: if
(V8DebuggerAgentImpl* debuggerAgent = helper.debuggerAgent()) {
On 2016/04/25 18:09:50, dgozman wrote:
> nit: early bailout

Done.

https://codereview.chromium.org/1917733002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/V8Console.cpp:550:
v8::Local<v8::Function> function = v8::Local<v8::Function>::Cast(info[0]);
On 2016/04/25 18:09:50, dgozman wrote:
> You already got this at line 545.

Done.

https://codereview.chromium.org/1917733002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/V8Console.cpp:660: for (size_t i
= 0; i < sizeof(members) / sizeof(const char*); ++i)
On 2016/04/25 18:09:50, dgozman wrote:
> Not yours, but let's change to WTF_ARRAY_LENGTH.

Done.

https://codereview.chromium.org/1917733002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/V8Console.cpp:671: for (size_t i
= 0; i < sizeof(members) / sizeof(const char*); ++i)
On 2016/04/25 18:09:50, dgozman wrote:
> ditto

Done.

https://codereview.chromium.org/1917733002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/v8_inspector/V8Console.h (right):

https://codereview.chromium.org/1917733002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/V8Console.h:19: static
v8::Local<v8::Object> createCommandLineAPI(v8::Local<v8::Context>,
InspectedContext*);
On 2016/04/25 18:09:50, dgozman wrote:
> Can we create it only once lazily and reuse?

Done.

[dgozman, 2016-04-25 20:55:08]

https://codereview.chromium.org/1917733002/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/inspector-protocol/console/console-let-const-with-api-expected.txt
(right):

https://codereview.chromium.org/1917733002/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/inspector-protocol/console/console-let-const-with-api-expected.txt:12:
function profile(value) { [Command Line API] }
[title] was better

https://codereview.chromium.org/1917733002/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/inspector-protocol/console/console-let-const-with-api-expected.txt:13:
function profileEnd(value) { [Command Line API] }
ditto

https://codereview.chromium.org/1917733002/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/inspector-protocol/console/console-let-const-with-api-expected.txt:24:
function table(value) { [Command Line API] }
data, [columns]

https://codereview.chromium.org/1917733002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/v8_inspector/InjectedScript.cpp (right):

https://codereview.chromium.org/1917733002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/v8_inspector/InjectedScript.cpp:390:
*errorString = "Internal error";
nit: hasInternalError

https://codereview.chromium.org/1917733002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/v8_inspector/InjectedScriptSource.js
(right):

https://codereview.chromium.org/1917733002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/v8_inspector/InjectedScriptSource.js:624:
nativeCommandLineAPI["$"].toString = function() { return "function $(selector,
[startNode]) { [Command Line API] }"};
Let's make it an explicit map.

https://codereview.chromium.org/1917733002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/v8_inspector/InjectedScriptSource.js:1175: //
NOTE: This list contains only not native Command Line API methods. For full
list: V8InjectedScriptHost.
V8InjectedScriptHost -> V8Console

https://codereview.chromium.org/1917733002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/v8_inspector/InjectedScriptSource.js:1181:
CommandLineAPIImpl.members_ = [
nit: inline this, but keep the comment.

[kozy, 2016-04-25 21:33:46]

Description was changed from

==========
[DevTools] Move part of CommandLineAPI to native

- ScopeExtension is stored by v8::Private instead v8::Symbol;
- part of CommandLineAPI methods are moved to native;

The benefits are:
- scopeExtension object can not be received in JS and stored somewhere;
- console related methods (dir, dirxml, ...) can't be overridden;
- removed InspectorConsoleAgent::clearAllMessages method;
- removed one more InjectedScriptSource.js part; 

With this CL Command Line API clear method calls console.clear directly. Without
it calls InspectorConsoleAgent::clearAllMessages, this method in case of
PageConsoleAgent additionally clears node that was pushed to frontend.

BUG=606055,595206
R=dgozman@chromium.org
==========

to

==========
[DevTools] Move part of CommandLineAPI to native

- ScopeExtension is stored by v8::Private instead v8::Symbol;
- part of CommandLineAPI methods are moved to native;

The benefits are:
- scopeExtension object can not be received in JS and stored somewhere;
- console related methods (dir, dirxml, ...) can't be overridden;
- removed InspectorConsoleAgent::clearAllMessages method;
- removed one more InjectedScriptSource.js part; 

BUG=606055,595206
R=dgozman@chromium.org
==========

[kozy, 2016-04-25 21:33:59]

Please take a look. All done.

https://codereview.chromium.org/1917733002/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/inspector-protocol/console/console-let-const-with-api-expected.txt
(right):

https://codereview.chromium.org/1917733002/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/inspector-protocol/console/console-let-const-with-api-expected.txt:12:
function profile(value) { [Command Line API] }
On 2016/04/25 20:55:08, dgozman wrote:
> [title] was better

Done.

https://codereview.chromium.org/1917733002/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/inspector-protocol/console/console-let-const-with-api-expected.txt:13:
function profileEnd(value) { [Command Line API] }
On 2016/04/25 20:55:08, dgozman wrote:
> ditto

Done.

https://codereview.chromium.org/1917733002/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/inspector-protocol/console/console-let-const-with-api-expected.txt:24:
function table(value) { [Command Line API] }
On 2016/04/25 20:55:08, dgozman wrote:
> data, [columns]

Done.

https://codereview.chromium.org/1917733002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/v8_inspector/InjectedScript.cpp (right):

https://codereview.chromium.org/1917733002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/v8_inspector/InjectedScript.cpp:390:
*errorString = "Internal error";
On 2016/04/25 20:55:08, dgozman wrote:
> nit: hasInternalError

Done.

https://codereview.chromium.org/1917733002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/v8_inspector/InjectedScriptSource.js
(right):

https://codereview.chromium.org/1917733002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/v8_inspector/InjectedScriptSource.js:624:
nativeCommandLineAPI["$"].toString = function() { return "function $(selector,
[startNode]) { [Command Line API] }"};
On 2016/04/25 20:55:08, dgozman wrote:
> Let's make it an explicit map.

Done.

https://codereview.chromium.org/1917733002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/v8_inspector/InjectedScriptSource.js:1175: //
NOTE: This list contains only not native Command Line API methods. For full
list: V8InjectedScriptHost.
On 2016/04/25 20:55:08, dgozman wrote:
> V8InjectedScriptHost -> V8Console

Done.

https://codereview.chromium.org/1917733002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/v8_inspector/InjectedScriptSource.js:1181:
CommandLineAPIImpl.members_ = [
On 2016/04/25 20:55:08, dgozman wrote:
> nit: inline this, but keep the comment.

Done.

[dgozman, 2016-04-25 21:52:12]

lgtm

[kozy, 2016-04-25 23:05:09]

The CQ bit was checked by kozyatinskiy@chromium.org

[kozy, 2016-04-25 23:05:26]

kozyatinskiy@chromium.org changed reviewers:
+ pfeldman@chromium.org

[kozy, 2016-04-25 23:05:51]

Pavel, ptal, I need owner lgtm for
third_party/WebKit/Source/bindings/core/v8/custom/V8WindowCustom.cpp

[commit-bot: I haz the power, 2016-04-25 23:05:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1917733002/130001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1917733002/130001

[commit-bot: I haz the power, 2016-04-25 23:15:06]

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

[commit-bot: I haz the power, 2016-04-25 23:15:08]

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

[pfeldman, 2016-04-25 23:31:37]

lgtm

[kozy, 2016-04-25 23:33:28]

The CQ bit was checked by kozyatinskiy@chromium.org

[commit-bot: I haz the power, 2016-04-25 23:34:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1917733002/130001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1917733002/130001

[commit-bot: I haz the power, 2016-04-26 00:12:39]

Description was changed from

==========
[DevTools] Move part of CommandLineAPI to native

- ScopeExtension is stored by v8::Private instead v8::Symbol;
- part of CommandLineAPI methods are moved to native;

The benefits are:
- scopeExtension object can not be received in JS and stored somewhere;
- console related methods (dir, dirxml, ...) can't be overridden;
- removed InspectorConsoleAgent::clearAllMessages method;
- removed one more InjectedScriptSource.js part; 

BUG=606055,595206
R=dgozman@chromium.org
==========

to

==========
[DevTools] Move part of CommandLineAPI to native

- ScopeExtension is stored by v8::Private instead v8::Symbol;
- part of CommandLineAPI methods are moved to native;

The benefits are:
- scopeExtension object can not be received in JS and stored somewhere;
- console related methods (dir, dirxml, ...) can't be overridden;
- removed InspectorConsoleAgent::clearAllMessages method;
- removed one more InjectedScriptSource.js part; 

BUG=606055,595206
R=dgozman@chromium.org
==========

[commit-bot: I haz the power, 2016-04-26 00:12:40]

Committed patchset #5 (id:130001)

[commit-bot: I haz the power, 2016-04-26 00:14:54]

Description was changed from

==========
[DevTools] Move part of CommandLineAPI to native

- ScopeExtension is stored by v8::Private instead v8::Symbol;
- part of CommandLineAPI methods are moved to native;

The benefits are:
- scopeExtension object can not be received in JS and stored somewhere;
- console related methods (dir, dirxml, ...) can't be overridden;
- removed InspectorConsoleAgent::clearAllMessages method;
- removed one more InjectedScriptSource.js part; 

BUG=606055,595206
R=dgozman@chromium.org
==========

to

==========
[DevTools] Move part of CommandLineAPI to native

- ScopeExtension is stored by v8::Private instead v8::Symbol;
- part of CommandLineAPI methods are moved to native;

The benefits are:
- scopeExtension object can not be received in JS and stored somewhere;
- console related methods (dir, dirxml, ...) can't be overridden;
- removed InspectorConsoleAgent::clearAllMessages method;
- removed one more InjectedScriptSource.js part;

BUG=606055,595206
R=dgozman@chromium.org

Committed: https://crrev.com/2cab1fca0201ab6d6b059bb0057e63973ebbb3c4
Cr-Commit-Position: refs/heads/master@{#389628}
==========

[commit-bot: I haz the power, 2016-04-26 00:14:56]

Patchset 5 (id:??) landed as
https://crrev.com/2cab1fca0201ab6d6b059bb0057e63973ebbb3c4
Cr-Commit-Position: refs/heads/master@{#389628}