Delete several deprecated methods on v8::CpuProfiler

Delete several deprecated methods on v8::CpuProfiler

All methods for accessing collected profiles by index are deprecated. The indexed storage may well be implemented by the embedder should he need it. CpuProfiler's responsibility is just to create CpuProfile object that contains all collected data and whose lifetime can be managed by the embedder.

BUG=chromium:327298
LOG=Y
R=svenpanne@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=18337

[alph, 2013-12-17 16:20:16]

https://codereview.chromium.org/117353002/diff/1/test/cctest/profiler-extensi...
File test/cctest/profiler-extension.cc (right):

https://codereview.chromium.org/117353002/diff/1/test/cctest/profiler-extensi...
test/cctest/profiler-extension.cc:60: v8::String::NewFromUtf8(args.GetIsolate(),
""));
Will v8::String::Empty() work here?

https://codereview.chromium.org/117353002/diff/1/test/cctest/profiler-extensi...
test/cctest/profiler-extension.cc:71: v8::String::NewFromUtf8(args.GetIsolate(),
""));
ditto

https://codereview.chromium.org/117353002/diff/1/test/cctest/test-cpu-profile...
File test/cctest/test-cpu-profiler.cc (right):

https://codereview.chromium.org/117353002/diff/1/test/cctest/test-cpu-profile...
test/cctest/test-cpu-profiler.cc:380: const_cast<v8::CpuProfile*>(p2)->Delete();
What's ProfilerExtension::last_profile at this point?

https://codereview.chromium.org/117353002/diff/1/test/cctest/test-cpu-profile...
test/cctest/test-cpu-profiler.cc:385: CHECK_EQ(0,
iprofiler->GetProfilesCount());
Mind to check here that ProfilerExtension::last_profile is NULL? ;-)

[Sven Panne, 2013-12-18 06:48:20]

https://codereview.chromium.org/117353002/diff/1/test/cctest/profiler-extensi...
File test/cctest/profiler-extension.cc (right):

https://codereview.chromium.org/117353002/diff/1/test/cctest/profiler-extensi...
test/cctest/profiler-extension.cc:60: v8::String::NewFromUtf8(args.GetIsolate(),
""));
On 2013/12/17 16:20:17, alph wrote:
> Will v8::String::Empty() work here?

Heads up: Although this is not explicitly deprecated, we're about to remove
*all* deprecated stuff + some more things. Empty() without an Isolate* parameter
is about to die, too. In general, use an Isolate version if there is one.

[Sven Panne, 2013-12-18 06:57:18]

Thinking about this CL a bit more: As was already mentioned, we plan to remove
all deprecated API entries now plus even a handful of things which were not
deprecated. The rationale behind this is that we really, really need to clean up
our Isolate story, and these entries are road blocks for this. Although we have
a (sloppy) policy to deprecate things for 1 release before they vanish, we think
that one big cleanup/removal is easier to communicate than a constantly moving
API in the foreseeable future, at least if you don't break the policy to often.
Let's have an API without any deprecations for the next release.

What this means for this CL is that I would prefer to remove the API entries
instead of marking them deprecated. If needed, change Chrome first.

[yurys, 2013-12-18 07:26:19]

https://codereview.chromium.org/117353002/diff/1/test/cctest/profiler-extensi...
File test/cctest/profiler-extension.cc (right):

https://codereview.chromium.org/117353002/diff/1/test/cctest/profiler-extensi...
test/cctest/profiler-extension.cc:60: v8::String::NewFromUtf8(args.GetIsolate(),
""));
On 2013/12/17 16:20:17, alph wrote:
> Will v8::String::Empty() work here?

Done.

https://codereview.chromium.org/117353002/diff/1/test/cctest/profiler-extensi...
test/cctest/profiler-extension.cc:60: v8::String::NewFromUtf8(args.GetIsolate(),
""));
On 2013/12/18 06:48:21, Sven Panne wrote:
> On 2013/12/17 16:20:17, alph wrote:
> > Will v8::String::Empty() work here?
> 
> Heads up: Although this is not explicitly deprecated, we're about to remove
> *all* deprecated stuff + some more things. Empty() without an Isolate*
parameter
> is about to die, too. In general, use an Isolate version if there is one.

Switched to the method with Isolate* parameter.

https://codereview.chromium.org/117353002/diff/1/test/cctest/profiler-extensi...
test/cctest/profiler-extension.cc:71: v8::String::NewFromUtf8(args.GetIsolate(),
""));
On 2013/12/17 16:20:17, alph wrote:
> ditto

Done.

https://codereview.chromium.org/117353002/diff/1/test/cctest/test-cpu-profile...
File test/cctest/test-cpu-profiler.cc (right):

https://codereview.chromium.org/117353002/diff/1/test/cctest/test-cpu-profile...
test/cctest/test-cpu-profiler.cc:380: const_cast<v8::CpuProfile*>(p2)->Delete();
On 2013/12/17 16:20:17, alph wrote:
> What's ProfilerExtension::last_profile at this point?

I don't care as ProfilerExtension::last_profile is a way to get result of
ProfilerExtension::stopProfiling method right after it was invoked. It is up to
the test to make sure that it calls it in appropriate moment.

https://codereview.chromium.org/117353002/diff/1/test/cctest/test-cpu-profile...
test/cctest/test-cpu-profiler.cc:385: CHECK_EQ(0,
iprofiler->GetProfilesCount());
On 2013/12/17 16:20:17, alph wrote:
> Mind to check here that ProfilerExtension::last_profile is NULL? ;-)

Why should I? We didn't use ProfilerExtension here and last_profile is a
convenience field for accessing profiles started from the script.

[yurys, 2013-12-18 07:34:18]

On 2013/12/18 06:57:18, Sven Panne wrote:
> Thinking about this CL a bit more: As was already mentioned, we plan to remove
> all deprecated API entries now plus even a handful of things which were not
> deprecated. The rationale behind this is that we really, really need to clean
up
> our Isolate story, and these entries are road blocks for this. Although we
have
> a (sloppy) policy to deprecate things for 1 release before they vanish, we
think
> that one big cleanup/removal is easier to communicate than a constantly moving
> API in the foreseeable future, at least if you don't break the policy to
often.
> Let's have an API without any deprecations for the next release.
> 
> What this means for this CL is that I would prefer to remove the API entries
> instead of marking them deprecated. If needed, change Chrome first.

Chromium side is already prepared for this so I don't mind removing all these
methods without deprecating them first. It sounds a bit weird that we have an
exception to the regular deprecation policy in this case though. Of cause it is
always easier to delete deprecated methods immediately as soon as Chromium is
prepared for this but other embedders may be not be happy with that.

[yurys, 2013-12-18 07:48:26]

Sven, I deleted the methods instead of deprecating them as you suggested. Please
take another look.

[Sven Panne, 2013-12-18 08:30:56]

LGTM if Chrome is happy with the changes.

[yurys, 2013-12-18 08:59:23]

Committed patchset #3 manually as r18337 (presubmit successful).