Continuing removing deprecated function from cctest

Continuing removing deprecated function from cctest

Removes deprecated functions from the following files:

test/cctest/compiler/function-tester.h
test/cctest/test-thread-termination.cc
test/cctest/test-threads.cc
test/cctest/test-transitions.cc
test/cctest/test-typedarrays.cc
test/cctest/test-types.cc
test/cctest/test-typing-reset.cc
test/cctest/test-unbound-queue.cc
test/cctest/test-unboxed-doubles.cc

BUG=v8:4134
LOG=n

Committed: https://crrev.com/41111e3dc318f18fcde690701191f3f28b3d3712
Cr-Commit-Position: refs/heads/master@{#30846}

[mythria, 2015-09-14 15:42:24]

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

[commit-bot: I haz the power, 2015-09-14 15:42:32]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1344583002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1344583002/1

[commit-bot: I haz the power, 2015-09-14 15:42:33]

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

[commit-bot: I haz the power, 2015-09-14 15:42:33]

Dry run: No L-G-T-M from a valid reviewer yet. Only full committers are
accepted.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[mythria, 2015-09-14 15:42:35]

The CQ bit was unchecked by mythria@chromium.org

[mythria, 2015-09-14 15:43:09]

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

[commit-bot: I haz the power, 2015-09-14 15:43:22]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1344583002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1344583002/1

[commit-bot: I haz the power, 2015-09-14 15:43:23]

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

[commit-bot: I haz the power, 2015-09-14 15:43:24]

Dry run: No L-G-T-M from a valid reviewer yet. Only full committers are
accepted.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[mythria, 2015-09-14 15:43:59]

mythria@chromium.org changed reviewers:
+ rmcilroy@chromium.org

[mythria, 2015-09-14 15:43:59]

Could you please review my changes and provide your feedback.

Thanks and Regards,
Mythri

https://codereview.chromium.org/1344583002/diff/1/test/cctest/test-thread-ter...
File test/cctest/test-thread-termination.cc (right):

https://codereview.chromium.org/1344583002/diff/1/test/cctest/test-thread-ter...
test/cctest/test-thread-termination.cc:95: CHECK(result.IsEmpty());
Added a check to see if the result is empty here i.e. the run was not
successful. I am not sure if this is the intended behavior. All the tests pass,
so I guess that is correct but I am not sure. This happens at several other
places in this file.

[rmcilroy, 2015-09-15 10:29:47]

https://codereview.chromium.org/1344583002/diff/1/test/cctest/test-thread-ter...
File test/cctest/test-thread-termination.cc (right):

https://codereview.chromium.org/1344583002/diff/1/test/cctest/test-thread-ter...
test/cctest/test-thread-termination.cc:95: CHECK(result.IsEmpty());
On 2015/09/14 15:43:59, mythria wrote:
> Added a check to see if the result is empty here i.e. the run was not
> successful. I am not sure if this is the intended behavior. All the tests
pass,
> so I guess that is correct but I am not sure. This happens at several other
> places in this file. 

This seems correct to me. The test seems like it is intending to fail part-way
through the scripts execution, so it seems like it should not return a result.

https://codereview.chromium.org/1344583002/diff/1/test/cctest/test-thread-ter...
test/cctest/test-thread-termination.cc:177:
->Run(CcTest::isolate()->GetCurrentContext());
nit - how about having a helper CompileAndRun function locally which does: 

v8::MaybeLocal<v8::Value> CompileAndRun(v8::Local<v8::String> source) {
  v8::Script::Compile(CcTest::isolate()->GetCurrentContext(), source)
                .ToLocalChecked()
                ->Run(CcTest::isolate()->GetCurrentContext());
}

This would avoid a lot of repetitive boilerplate.

https://codereview.chromium.org/1344583002/diff/1/test/cctest/test-thread-ter...
test/cctest/test-thread-termination.cc:319: .ToLocalChecked(),
nit - you could also have a helper for:

v8::Local<v8::String> NewString(Isolate* isolate, const char* string) {
  return v8::String::NewFromUtf8(isolate, string, v8::NewStringType::kNormal)
                    .ToLocalChecked());

This would reduce a bunch of other boilerplate you had to introduce due to the
API changes.

https://codereview.chromium.org/1344583002/diff/1/test/cctest/test-typedarray...
File test/cctest/test-typedarrays.cc (right):

https://codereview.chromium.org/1344583002/diff/1/test/cctest/test-typedarray...
test/cctest/test-typedarrays.cc:23: ->GetCurrentContext();
Can you not just do: v8::Isolate::GetCurrent()->GetCurrentContext() here?

https://codereview.chromium.org/1344583002/diff/1/test/cctest/test-unboxed-do...
File test/cctest/test-unboxed-doubles.cc (right):

https://codereview.chromium.org/1344583002/diff/1/test/cctest/test-unboxed-do...
test/cctest/test-unboxed-doubles.cc:59: ->GetCurrentContext();
ditto

[mythria, 2015-09-17 11:21:48]

I addressed the review comments. Could you review and suggest if any further
changes are required.

https://codereview.chromium.org/1344583002/diff/1/test/cctest/test-thread-ter...
File test/cctest/test-thread-termination.cc (right):

https://codereview.chromium.org/1344583002/diff/1/test/cctest/test-thread-ter...
test/cctest/test-thread-termination.cc:177:
->Run(CcTest::isolate()->GetCurrentContext());
On 2015/09/15 10:29:47, rmcilroy wrote:
> nit - how about having a helper CompileAndRun function locally which does: 
> 
> v8::MaybeLocal<v8::Value> CompileAndRun(v8::Local<v8::String> source) {
>   v8::Script::Compile(CcTest::isolate()->GetCurrentContext(), source)
>                 .ToLocalChecked()
>                 ->Run(CcTest::isolate()->GetCurrentContext());
> }
> 
> This would avoid a lot of repetitive boilerplate.

Done.

https://codereview.chromium.org/1344583002/diff/1/test/cctest/test-thread-ter...
test/cctest/test-thread-termination.cc:319: .ToLocalChecked(),
On 2015/09/15 10:29:47, rmcilroy wrote:
> nit - you could also have a helper for:
> 
> v8::Local<v8::String> NewString(Isolate* isolate, const char* string) {
>   return v8::String::NewFromUtf8(isolate, string, v8::NewStringType::kNormal)
>                     .ToLocalChecked());
> 
> This would reduce a bunch of other boilerplate you had to introduce due to the
> API changes.

Done.

https://codereview.chromium.org/1344583002/diff/1/test/cctest/test-thread-ter...
test/cctest/test-thread-termination.cc:319: .ToLocalChecked(),
On 2015/09/15 10:29:47, rmcilroy wrote:
> nit - you could also have a helper for:
> 
> v8::Local<v8::String> NewString(Isolate* isolate, const char* string) {
>   return v8::String::NewFromUtf8(isolate, string, v8::NewStringType::kNormal)
>                     .ToLocalChecked());
> 
> This would reduce a bunch of other boilerplate you had to introduce due to the
> API changes.

There was already a helper function. I did not notice it earlier. The same for
the string as well.

https://codereview.chromium.org/1344583002/diff/1/test/cctest/test-typedarray...
File test/cctest/test-typedarrays.cc (right):

https://codereview.chromium.org/1344583002/diff/1/test/cctest/test-typedarray...
test/cctest/test-typedarrays.cc:23: ->GetCurrentContext();
On 2015/09/15 10:29:47, rmcilroy wrote:
> Can you not just do: v8::Isolate::GetCurrent()->GetCurrentContext() here?

Done.

https://codereview.chromium.org/1344583002/diff/1/test/cctest/test-unboxed-do...
File test/cctest/test-unboxed-doubles.cc (right):

https://codereview.chromium.org/1344583002/diff/1/test/cctest/test-unboxed-do...
test/cctest/test-unboxed-doubles.cc:59: ->GetCurrentContext();
On 2015/09/15 10:29:47, rmcilroy wrote:
> ditto

Done.

[rmcilroy, 2015-09-17 11:33:20]

One more nit, but otherwise lgtm, thanks!

https://codereview.chromium.org/1344583002/diff/20001/test/cctest/test-thread...
File test/cctest/test-thread-termination.cc (right):

https://codereview.chromium.org/1344583002/diff/20001/test/cctest/test-thread...
test/cctest/test-thread-termination.cc:379: .FromJust());
nit - I think this is a bit too much in the CHECK, how about:

v8::Local<v8::Value> result =  CompileRun(...)
              .ToLocalChecked();
CHECK(result->Equals(...).FromJust());

https://codereview.chromium.org/1344583002/diff/20001/test/cctest/test-thread...
test/cctest/test-thread-termination.cc:537: .FromJust());
ditto here.

https://codereview.chromium.org/1344583002/diff/20001/test/cctest/test-thread...
test/cctest/test-thread-termination.cc:561: ->IsFunction());
ditto here.

https://codereview.chromium.org/1344583002/diff/20001/test/cctest/test-thread...
test/cctest/test-thread-termination.cc:570: .FromJust());
ditto here.

[mythria, 2015-09-21 10:08:39]

Thanks for the review. I fixed all the nits.

https://codereview.chromium.org/1344583002/diff/20001/test/cctest/test-thread...
File test/cctest/test-thread-termination.cc (right):

https://codereview.chromium.org/1344583002/diff/20001/test/cctest/test-thread...
test/cctest/test-thread-termination.cc:379: .FromJust());
On 2015/09/17 11:33:20, rmcilroy wrote:
> nit - I think this is a bit too much in the CHECK, how about:
> 
> v8::Local<v8::Value> result =  CompileRun(...)
>               .ToLocalChecked();
> CHECK(result->Equals(...).FromJust());

Done.

https://codereview.chromium.org/1344583002/diff/20001/test/cctest/test-thread...
test/cctest/test-thread-termination.cc:537: .FromJust());
On 2015/09/17 11:33:20, rmcilroy wrote:
> ditto here.

Done.

https://codereview.chromium.org/1344583002/diff/20001/test/cctest/test-thread...
test/cctest/test-thread-termination.cc:561: ->IsFunction());
On 2015/09/17 11:33:20, rmcilroy wrote:
> ditto here.

Done.

https://codereview.chromium.org/1344583002/diff/20001/test/cctest/test-thread...
test/cctest/test-thread-termination.cc:570: .FromJust());
On 2015/09/17 11:33:20, rmcilroy wrote:
> ditto here.

Done.

[mythria, 2015-09-21 10:08:52]

The CQ bit was checked by mythria@chromium.org

[mythria, 2015-09-21 10:08:52]

The patchset sent to the CQ was uploaded after l-g-t-m from
rmcilroy@chromium.org
Link to the patchset: https://codereview.chromium.org/1344583002/#ps60001
(title: "Fixed few more nits I forgot in my last patch")

[commit-bot: I haz the power, 2015-09-21 10:08:55]

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

[commit-bot: I haz the power, 2015-09-21 10:34:49]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-09-21 10:35:09]

Patchset 4 (id:??) landed as
https://crrev.com/41111e3dc318f18fcde690701191f3f28b3d3712
Cr-Commit-Position: refs/heads/master@{#30846}