Hide heap methods where possible.

Hide heap methods where possible.

Factory is already a friend class of Heap.
We introduce a TestHeap class in cctest.h to access protected methods.

R=mstarzinger@chromium.org

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

[Michael Starzinger, 2014-04-29 13:15:09]

LGTM. Just nits.

https://codereview.chromium.org/247263003/diff/1/src/factory.h
File src/factory.h (right):

https://codereview.chromium.org/247263003/diff/1/src/factory.h#newcode672
src/factory.h:672: inline Handle<Code> NewCodeHelper(int object_size, bool
immovable);
nit: Can we call this "NewCodeRaw", add a comment that says that the object is
not initialized and maybe even move it up a few lines to right below the two
New<> declarations that also return uninitialized objects?

https://codereview.chromium.org/247263003/diff/1/src/heap.h
File src/heap.h (right):

https://codereview.chromium.org/247263003/diff/1/src/heap.h#newcode1545
src/heap.h:1545: // Please note this function does not perform a garbage
collection.
nit: I never was sure of the value of duplicating these two sentences of
comments all across the file. I would be fine with dropping these two sentences
for all protected and private methods and only leave them in place for the
public API.

https://codereview.chromium.org/247263003/diff/1/test/cctest/cctest.h
File test/cctest/cctest.h (right):

https://codereview.chromium.org/247263003/diff/1/test/cctest/cctest.h#newcode84
test/cctest/cctest.h:84: class TestHeap : public i::Heap {
At first this helper class worried me, but the more I think about it, the more I
like it. Also Hannes thinks it's OK. Let's do it. :)

https://codereview.chromium.org/247263003/diff/1/test/cctest/test-heap.cc
File test/cctest/test-heap.cc (right):

https://codereview.chromium.org/247263003/diff/1/test/cctest/test-heap.cc#new...
test/cctest/test-heap.cc:609:
CHECK(String::cast(*b)->IsUtf8EqualTo(CStrVector(string)));
nit: The "String::cast" here is obsolete, "b" is already of that type. Just
using "b->IsUtf8EqualTo()" is easier to read.

[Yang, 2014-04-29 13:55:13]

https://codereview.chromium.org/247263003/diff/1/src/factory.h
File src/factory.h (right):

https://codereview.chromium.org/247263003/diff/1/src/factory.h#newcode672
src/factory.h:672: inline Handle<Code> NewCodeHelper(int object_size, bool
immovable);
On 2014/04/29 13:15:09, Michael Starzinger wrote:
> nit: Can we call this "NewCodeRaw", add a comment that says that the object is
> not initialized and maybe even move it up a few lines to right below the two
> New<> declarations that also return uninitialized objects?

Done.

https://codereview.chromium.org/247263003/diff/1/src/heap.h
File src/heap.h (right):

https://codereview.chromium.org/247263003/diff/1/src/heap.h#newcode1545
src/heap.h:1545: // Please note this function does not perform a garbage
collection.
On 2014/04/29 13:15:09, Michael Starzinger wrote:
> nit: I never was sure of the value of duplicating these two sentences of
> comments all across the file. I would be fine with dropping these two
sentences
> for all protected and private methods and only leave them in place for the
> public API.

Done.

https://codereview.chromium.org/247263003/diff/1/test/cctest/cctest.h
File test/cctest/cctest.h (right):

https://codereview.chromium.org/247263003/diff/1/test/cctest/cctest.h#newcode84
test/cctest/cctest.h:84: class TestHeap : public i::Heap {
On 2014/04/29 13:15:09, Michael Starzinger wrote:
> At first this helper class worried me, but the more I think about it, the more
I
> like it. Also Hannes thinks it's OK. Let's do it. :)

:)
Apparently this is covered by the C++ spec, so I think this should be safe.

[Yang, 2014-04-29 13:59:10]

Committed patchset #2 manually as r21053 (presubmit successful).