Remove usage of deprecated APIs from test-api.cc.

Remove usage of deprecated APIs from test-api.cc.

No more deprecation warnings up to line 11k.

BUG=

Committed: https://crrev.com/fd73cb191c5b7c8a49002d11e0cecd5556df0c58
Cr-Commit-Position: refs/heads/master@{#32334}

[vogelheim, 2015-11-23 18:20:30]

Description was changed from

==========
Remove usage of deprecated APIs from test-api.cc.

[Ca 1/2 done. Will reformat before review.]

BUG=
==========

to

==========
Remove usage of deprecated APIs from test-api.cc.

[Ca 1/2 done.]

BUG=
==========

[vogelheim, 2015-11-23 18:22:14]

vogelheim@chromium.org changed reviewers:
+ jochen@chromium.org

[vogelheim, 2015-11-23 18:22:15]

Please have a look.

Have achieved 50% or so; more to come, but I'd like to split this into multiple
CLs.

Also, please have look at the two comments. I'm not really sure what the right
thing to do there is.

https://codereview.chromium.org/1427643005/diff/40001/test/cctest/test-api.cc
File test/cctest/test-api.cc (right):

https://codereview.chromium.org/1427643005/diff/40001/test/cctest/test-api.cc...
test/cctest/test-api.cc:4890: context->Global()->ForceSet(context.local(), prop,
v8_num(7),
I can't figure out how to replace ForceSet with non-None PropertyAttributes.
Does this test just go away?

https://codereview.chromium.org/1427643005/diff/40001/test/cctest/test-api.cc...
test/cctest/test-api.cc:8936: TEST(SecurityHandler) {
Here & below: I was having a really hard time with the security-related tests.
Mainly, they're testing cross-context accesses, which the new API is supposed to
make impossible by having explicit contexts.

For the API deprecation, this opens two failure modes:
- I incorrectly inserted the wrong contexts. In this case, the test no longer
tests anything security related.
- I correctly inserted the context, but now the error condition can no longer
happen. In that case, the security test no longer tests anything.

Could you please have an extra-careful eye on these?

[jochen (gone - plz use gerrit), 2015-11-24 12:42:25]

https://codereview.chromium.org/1427643005/diff/40001/test/cctest/test-api.cc
File test/cctest/test-api.cc (right):

https://codereview.chromium.org/1427643005/diff/40001/test/cctest/test-api.cc...
test/cctest/test-api.cc:4890: context->Global()->ForceSet(context.local(), prop,
v8_num(7),
On 2015/11/23 at 18:22:15, vogelheim wrote:
> I can't figure out how to replace ForceSet with non-None PropertyAttributes.
Does this test just go away?

does context->Global()->DefineOwnProperty(context.local(), prop, v8_num(7),
v8::ReadOnly).FromJust() work?

https://codereview.chromium.org/1427643005/diff/40001/test/cctest/test-api.cc...
test/cctest/test-api.cc:8971: global0->Get(context0,
v8_str("foo")).ToLocalChecked();
in line 8005/8960 context1 is entered, so we should use context1 here and below

the security check should pass nevertheless because we just forced it to pass in
line 8013/8968

https://codereview.chromium.org/1427643005/diff/60001/test/cctest/test-api.cc
File test/cctest/test-api.cc (right):

https://codereview.chromium.org/1427643005/diff/60001/test/cctest/test-api.cc...
test/cctest/test-api.cc:9167: CHECK(env1->Global()->Set(env2, v8_str("prop"),
v8_num(3)).FromJust());
should be env1 here

https://codereview.chromium.org/1427643005/diff/60001/test/cctest/test-api.cc...
test/cctest/test-api.cc:9478: // SHOULD BE env2 ???
env1 is ok here

[vogelheim, 2015-11-26 10:57:32]

https://codereview.chromium.org/1427643005/diff/40001/test/cctest/test-api.cc
File test/cctest/test-api.cc (right):

https://codereview.chromium.org/1427643005/diff/40001/test/cctest/test-api.cc...
test/cctest/test-api.cc:4890: context->Global()->ForceSet(context.local(), prop,
v8_num(7),
On 2015/11/24 12:42:25, jochen wrote:
> On 2015/11/23 at 18:22:15, vogelheim wrote:
> > I can't figure out how to replace ForceSet with non-None PropertyAttributes.
> Does this test just go away?
> 
> does context->Global()->DefineOwnProperty(context.local(), prop, v8_num(7),
> v8::ReadOnly).FromJust() work?

It does. Done.

https://codereview.chromium.org/1427643005/diff/40001/test/cctest/test-api.cc...
test/cctest/test-api.cc:8971: global0->Get(context0,
v8_str("foo")).ToLocalChecked();
On 2015/11/24 12:42:25, jochen wrote:
> in line 8005/8960 context1 is entered, so we should use context1 here and
below
> 
> the security check should pass nevertheless because we just forced it to pass
in
> line 8013/8968

Done.

https://codereview.chromium.org/1427643005/diff/60001/test/cctest/test-api.cc
File test/cctest/test-api.cc (right):

https://codereview.chromium.org/1427643005/diff/60001/test/cctest/test-api.cc...
test/cctest/test-api.cc:9167: CHECK(env1->Global()->Set(env2, v8_str("prop"),
v8_num(3)).FromJust());
On 2015/11/24 12:42:25, jochen wrote:
> should be env1 here 

Done.

https://codereview.chromium.org/1427643005/diff/60001/test/cctest/test-api.cc...
test/cctest/test-api.cc:9478: // SHOULD BE env2 ???
On 2015/11/24 12:42:25, jochen wrote:
> env1 is ok here

Done.

[jochen (gone - plz use gerrit), 2015-11-26 10:58:33]

lgtm

[vogelheim, 2015-11-26 10:58:59]

Description was changed from

==========
Remove usage of deprecated APIs from test-api.cc.

[Ca 1/2 done.]

BUG=
==========

to

==========
Remove usage of deprecated APIs from test-api.cc.

No more deprecation warnings up to line 11k.

BUG=
==========

[vogelheim, 2015-11-26 10:59:24]

The CQ bit was checked by vogelheim@chromium.org

[commit-bot: I haz the power, 2015-11-26 10:59:31]

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

[commit-bot: I haz the power, 2015-11-26 11:13:45]

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

[commit-bot: I haz the power, 2015-11-26 11:13:46]

Try jobs failed on following builders:
  v8_linux_nodcheck_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel/build...)

[vogelheim, 2015-11-26 12:41:27]

The CQ bit was checked by vogelheim@chromium.org

[vogelheim, 2015-11-26 12:41:28]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org
Link to the patchset: https://codereview.chromium.org/1427643005/#ps120001
(title: "Rebase + resolve merge conflicts.")

[commit-bot: I haz the power, 2015-11-26 12:41:34]

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

[commit-bot: I haz the power, 2015-11-26 12:44:14]

Description was changed from

==========
Remove usage of deprecated APIs from test-api.cc.

No more deprecation warnings up to line 11k.

BUG=
==========

to

==========
Remove usage of deprecated APIs from test-api.cc.

No more deprecation warnings up to line 11k.

BUG=
==========

[commit-bot: I haz the power, 2015-11-26 12:44:15]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-11-26 12:44:47]

Description was changed from

==========
Remove usage of deprecated APIs from test-api.cc.

No more deprecation warnings up to line 11k.

BUG=
==========

to

==========
Remove usage of deprecated APIs from test-api.cc.

No more deprecation warnings up to line 11k.

BUG=

Committed: https://crrev.com/fd73cb191c5b7c8a49002d11e0cecd5556df0c58
Cr-Commit-Position: refs/heads/master@{#32334}
==========

[commit-bot: I haz the power, 2015-11-26 12:44:48]

Patchset 7 (id:??) landed as
https://crrev.com/fd73cb191c5b7c8a49002d11e0cecd5556df0c58
Cr-Commit-Position: refs/heads/master@{#32334}