[inspector] added reconnect method for tests

[inspector] added reconnect method for tests

This method enables test of agent::restore methods.
Bonus: forbid setCustomObjectFormatterEnabled on disabled agent.

BUG=none
R=dgozman@chromium.org

Review-Url: https://codereview.chromium.org/2713023004
Cr-Commit-Position: refs/heads/master@{#43502}
Committed: https://chromium.googlesource.com/v8/v8/+/9c385f0405bb7d91259632f517c2c75e8dbb485b

[kozy, 2017-02-25 01:58:24]

Dmitry, please take a look!

I'll added tests for another agents in folow up.

[dgozman, 2017-02-27 18:44:57]

https://codereview.chromium.org/2713023004/diff/1/src/inspector/v8-runtime-ag...
File src/inspector/v8-runtime-agent-impl.cc (right):

https://codereview.chromium.org/2713023004/diff/1/src/inspector/v8-runtime-ag...
src/inspector/v8-runtime-agent-impl.cc:739:
m_session->setCustomObjectFormatterEnabled(true);
Looks like we already restore it properly.

https://codereview.chromium.org/2713023004/diff/1/src/inspector/v8-runtime-ag...
src/inspector/v8-runtime-agent-impl.cc:753:
m_session->setCustomObjectFormatterEnabled(m_state->booleanProperty(
And we should not restore anything in enable. It just enables.

https://codereview.chromium.org/2713023004/diff/1/test/inspector/inspector-te...
File test/inspector/inspector-test.cc (right):

https://codereview.chromium.org/2713023004/diff/1/test/inspector/inspector-te...
test/inspector/inspector-test.cc:118: v8::String::NewFromUtf8(isolate,
"reconnect",
I remember you promised to scope all the extensions on some object next time you
add one?

https://codereview.chromium.org/2713023004/diff/1/test/inspector/runtime/runt...
File test/inspector/runtime/runtime-restore-expected.txt (right):

https://codereview.chromium.org/2713023004/diff/1/test/inspector/runtime/runt...
test/inspector/runtime/runtime-restore-expected.txt:69:
bindRemoteObjectFunctionId : {"injectedScriptId":1,"id":3}
Filter out these ids to fight flakiness.

https://codereview.chromium.org/2713023004/diff/1/test/inspector/runtime/runt...
File test/inspector/runtime/runtime-restore.js (right):

https://codereview.chromium.org/2713023004/diff/1/test/inspector/runtime/runt...
test/inspector/runtime/runtime-restore.js:40: .then(() =>
Protocol.Runtime.onExecutionContextDestroyed(null))
Why do you chain sync functions?

[kozy, 2017-02-27 19:56:41]

Description was changed from

==========
[inspector] added reconnect method for tests

This method enables test of agent::restore methods.
Bonus: properly set custom formatter enable on V8RuntimeAgentImpl::enable.

BUG=none
R=dgozman@chromium.org
==========

to

==========
[inspector] added reconnect method for tests

This method enables test of agent::restore methods.
Bonus: forbid setCustomObjectFormatterEnabled on disabled agent.

BUG=none
R=dgozman@chromium.org
==========

[kozy, 2017-02-27 19:57:13]

All done.

Main change in most tests:
print('test description') -> InspectorTest.log(..

https://codereview.chromium.org/2713023004/diff/1/src/inspector/v8-runtime-ag...
File src/inspector/v8-runtime-agent-impl.cc (right):

https://codereview.chromium.org/2713023004/diff/1/src/inspector/v8-runtime-ag...
src/inspector/v8-runtime-agent-impl.cc:753:
m_session->setCustomObjectFormatterEnabled(m_state->booleanProperty(
On 2017/02/27 18:44:56, dgozman wrote:
> And we should not restore anything in enable. It just enables.

Ok, then we should forbid this method on disabled runtime agent - done.

https://codereview.chromium.org/2713023004/diff/1/test/inspector/inspector-te...
File test/inspector/inspector-test.cc (right):

https://codereview.chromium.org/2713023004/diff/1/test/inspector/inspector-te...
test/inspector/inspector-test.cc:118: v8::String::NewFromUtf8(isolate,
"reconnect",
On 2017/02/27 18:44:56, dgozman wrote:
> I remember you promised to scope all the extensions on some object next time
you
> add one?

Done.
I wrapped them in protocol-test.js and continue to use v8::Extensions otherwise
I need to manually create a wrapper object and install it on global object.

https://codereview.chromium.org/2713023004/diff/1/test/inspector/runtime/runt...
File test/inspector/runtime/runtime-restore-expected.txt (right):

https://codereview.chromium.org/2713023004/diff/1/test/inspector/runtime/runt...
test/inspector/runtime/runtime-restore-expected.txt:69:
bindRemoteObjectFunctionId : {"injectedScriptId":1,"id":3}
On 2017/02/27 18:44:56, dgozman wrote:
> Filter out these ids to fight flakiness.

Done.

https://codereview.chromium.org/2713023004/diff/1/test/inspector/runtime/runt...
File test/inspector/runtime/runtime-restore.js (right):

https://codereview.chromium.org/2713023004/diff/1/test/inspector/runtime/runt...
test/inspector/runtime/runtime-restore.js:40: .then(() =>
Protocol.Runtime.onExecutionContextDestroyed(null))
On 2017/02/27 18:44:56, dgozman wrote:
> Why do you chain sync functions?

Done.

[dgozman, 2017-02-28 19:46:40]

lgtm

https://codereview.chromium.org/2713023004/diff/20001/src/inspector/v8-runtim...
File src/inspector/v8-runtime-agent-impl.cc (right):

https://codereview.chromium.org/2713023004/diff/20001/src/inspector/v8-runtim...
src/inspector/v8-runtime-agent-impl.cc:683:
m_session->setCustomObjectFormatterEnabled(false);
Nice one!

https://codereview.chromium.org/2713023004/diff/20001/test/inspector/runtime/...
File test/inspector/runtime/runtime-restore.js (right):

https://codereview.chromium.org/2713023004/diff/20001/test/inspector/runtime/...
test/inspector/runtime/runtime-restore.js:31:
Protocol.Runtime.onExecutionContextsCleared(InspectorTest.logMessage);
Future idea: automatic way to remove all listeners at the end of the test. Maybe
even do that in runTestSuite.

[kozy, 2017-02-28 19:53:29]

The CQ bit was checked by kozyatinskiy@chromium.org

[kozy, 2017-02-28 19:53:30]

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/2713023004/#ps40001
(title: "rebased")

[commit-bot: I haz the power, 2017-02-28 19:53:34]

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, 2017-02-28 20:22:25]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1488311609953640,
"parent_rev": "4e600299ecd9491a2d657eef26ee77e78a0e6abc", "commit_rev":
"9c385f0405bb7d91259632f517c2c75e8dbb485b"}

[commit-bot: I haz the power, 2017-02-28 20:22:31]

Description was changed from

==========
[inspector] added reconnect method for tests

This method enables test of agent::restore methods.
Bonus: forbid setCustomObjectFormatterEnabled on disabled agent.

BUG=none
R=dgozman@chromium.org
==========

to

==========
[inspector] added reconnect method for tests

This method enables test of agent::restore methods.
Bonus: forbid setCustomObjectFormatterEnabled on disabled agent.

BUG=none
R=dgozman@chromium.org

Review-Url: https://codereview.chromium.org/2713023004
Cr-Commit-Position: refs/heads/master@{#43502}
Committed:
https://chromium.googlesource.com/v8/v8/+/9c385f0405bb7d91259632f517c2c75e8db...
==========

[commit-bot: I haz the power, 2017-02-28 20:22:32]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/v8/v8/+/9c385f0405bb7d91259632f517c2c75e8db...