[inspector] better isArrayLike for injected-script-source.js

[inspector] better isArrayLike for injected-script-source.js

We should never check existing of the property by typeof obj.name check.

BUG=chromium:693338
R=dgozman@chromium.org,luoe@chromium.org

Review-Url: https://codereview.chromium.org/2767323002
Cr-Commit-Position: refs/heads/master@{#44113}
Committed: https://chromium.googlesource.com/v8/v8/+/6c2215325e34d6a311df5d7d65abb43e0a1edf05

[kozy, 2017-03-22 23:17:05]

Dmitry and Erik, please take a look.
This CL is example of using new method from
https://codereview.chromium.org/2770823003/.

[dgozman, 2017-03-23 21:10:20]

https://codereview.chromium.org/2767323002/diff/20001/src/inspector/injected_...
File src/inspector/injected_script_externs.js (right):

https://codereview.chromium.org/2767323002/diff/20001/src/inspector/injected_...
src/inspector/injected_script_externs.js:20:
InjectedScriptHostClass.prototype.get = function(obj, name) {}
getProperty

https://codereview.chromium.org/2767323002/diff/20001/src/inspector/v8-inject...
File src/inspector/v8-injected-script-host.cc (right):

https://codereview.chromium.org/2767323002/diff/20001/src/inspector/v8-inject...
src/inspector/v8-injected-script-host.cc:109:
info.GetReturnValue().Set(property);
So, do we return undefined in case of JS execution or exception?

https://codereview.chromium.org/2767323002/diff/20001/test/inspector/protocol...
File test/inspector/protocol-test.js (right):

https://codereview.chromium.org/2767323002/diff/20001/test/inspector/protocol...
test/inspector/protocol-test.js:347: InspectorTest.log('WARNING: you can
additionally comment \'input_text = rjsmin.jsmin(input_text)\' line in xxd.py to
get unminified injected-script-source.js.');
Let's not be too specific (or it will be outdated):
... comment rjsmin in xxd.py ...

https://codereview.chromium.org/2767323002/diff/20001/test/inspector/runtime/...
File test/inspector/runtime/evaluate-with-generate-preview-expected.txt (right):

https://codereview.chromium.org/2767323002/diff/20001/test/inspector/runtime/...
test/inspector/runtime/evaluate-with-generate-preview-expected.txt:837: type :
object
So this was an array before or not? What changed?

https://codereview.chromium.org/2767323002/diff/20001/test/inspector/runtime/...
File test/inspector/runtime/evaluate-with-generate-preview.js (right):

https://codereview.chromium.org/2767323002/diff/20001/test/inspector/runtime/...
test/inspector/runtime/evaluate-with-generate-preview.js:68:
InspectorTest.makeContextDirty();
Let's rename this to setupInjectedScriptEnvironment

[kozy, 2017-03-24 00:40:37]

Addressed all comments, please take another look.

https://codereview.chromium.org/2767323002/diff/20001/src/inspector/injected_...
File src/inspector/injected_script_externs.js (right):

https://codereview.chromium.org/2767323002/diff/20001/src/inspector/injected_...
src/inspector/injected_script_externs.js:20:
InjectedScriptHostClass.prototype.get = function(obj, name) {}
On 2017/03/23 21:10:20, dgozman wrote:
> getProperty

Done.

https://codereview.chromium.org/2767323002/diff/20001/src/inspector/v8-inject...
File src/inspector/v8-injected-script-host.cc (right):

https://codereview.chromium.org/2767323002/diff/20001/src/inspector/v8-inject...
src/inspector/v8-injected-script-host.cc:109:
info.GetReturnValue().Set(property);
On 2017/03/23 21:10:20, dgozman wrote:
> So, do we return undefined in case of JS execution or exception?

yes, we do, in current use cases it looks like expected behavior.

https://codereview.chromium.org/2767323002/diff/20001/test/inspector/protocol...
File test/inspector/protocol-test.js (right):

https://codereview.chromium.org/2767323002/diff/20001/test/inspector/protocol...
test/inspector/protocol-test.js:347: InspectorTest.log('WARNING: you can
additionally comment \'input_text = rjsmin.jsmin(input_text)\' line in xxd.py to
get unminified injected-script-source.js.');
On 2017/03/23 21:10:20, dgozman wrote:
> Let's not be too specific (or it will be outdated):
> ... comment rjsmin in xxd.py ...

Done.

https://codereview.chromium.org/2767323002/diff/20001/test/inspector/runtime/...
File test/inspector/runtime/evaluate-with-generate-preview-expected.txt (right):

https://codereview.chromium.org/2767323002/diff/20001/test/inspector/runtime/...
test/inspector/runtime/evaluate-with-generate-preview-expected.txt:837: type :
object
On 2017/03/23 21:10:20, dgozman wrote:
> So this was an array before or not? What changed?

It was the same before (this test I copied from layout_tests). It's for case
when we have array as prototype: Object.create([1,2]) - in this case we have
splice and length but all values are not own but we assume on frontend side that
values are own. In this case we'll show [undefined, undefined] if pretend this
as real array.

https://codereview.chromium.org/2767323002/diff/20001/test/inspector/runtime/...
File test/inspector/runtime/evaluate-with-generate-preview.js (right):

https://codereview.chromium.org/2767323002/diff/20001/test/inspector/runtime/...
test/inspector/runtime/evaluate-with-generate-preview.js:68:
InspectorTest.makeContextDirty();
On 2017/03/23 21:10:20, dgozman wrote:
> Let's rename this to setupInjectedScriptEnvironment

Done in previous CL.

[kozy, 2017-03-24 01:55:26]

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

[commit-bot: I haz the power, 2017-03-24 01:55:35]

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, 2017-03-24 02:29:33]

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

[commit-bot: I haz the power, 2017-03-24 02:29:34]

Dry run: This issue passed the CQ dry run.

[dgozman, 2017-03-24 16:50:48]

lgtm

[kozy, 2017-03-24 16:53:26]

The CQ bit was checked by kozyatinskiy@chromium.org

[commit-bot: I haz the power, 2017-03-24 16:53:32]

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-03-24 17:38:12]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1490374406702320,
"parent_rev": "dae6e43d38c0f66a49f66cfd7eb2f7113f054e68", "commit_rev":
"1a8fa5fbc5fdca69524b75903dbf6c2e2c5905cd"}

[commit-bot: I haz the power, 2017-03-24 17:38:21]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1490374406702320,
"parent_rev": "09050c8a967f5f2956305e5d016b304d7bf5e669", "commit_rev":
"6c2215325e34d6a311df5d7d65abb43e0a1edf05"}

[commit-bot: I haz the power, 2017-03-24 17:38:25]

Description was changed from

==========
[inspector] better isArrayLike for injected-script-source.js

We should never check existing of the property by typeof obj.name check.

BUG=chromium:693338
R=dgozman@chromium.org,luoe@chromium.org
==========

to

==========
[inspector] better isArrayLike for injected-script-source.js

We should never check existing of the property by typeof obj.name check.

BUG=chromium:693338
R=dgozman@chromium.org,luoe@chromium.org

Review-Url: https://codereview.chromium.org/2767323002
Cr-Commit-Position: refs/heads/master@{#44113}
Committed:
https://chromium.googlesource.com/v8/v8/+/6c2215325e34d6a311df5d7d65abb43e0a1...
==========

[commit-bot: I haz the power, 2017-03-24 17:38:26]

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