Refactor string internalization.

Refactor string internalization.

Remove duplicate code in StringTable and Heap. Instead of having many InternalizeXXXString methods, add a general InternalizeStringWithKey that takes a string table key.

BUG=
R=dcarney@chromium.org, svenpanne@chromium.org

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

[ulan, 2014-01-16 16:07:28]

PTAL

[dcarney, 2014-01-16 16:17:21]

lgtm

[marja, 2014-01-16 16:18:18]

https://codereview.chromium.org/140953002/diff/30001/src/objects-inl.h
File src/objects-inl.h (right):

https://codereview.chromium.org/140953002/diff/30001/src/objects-inl.h#newcod...
src/objects-inl.h:6681: uint32_t Hash() {
Drive-by coding style nit: don't you need to mark virtual funcs virtual in the
subclasses too?

[ulan, 2014-01-16 16:23:11]

https://codereview.chromium.org/140953002/diff/30001/src/objects-inl.h
File src/objects-inl.h (right):

https://codereview.chromium.org/140953002/diff/30001/src/objects-inl.h#newcod...
src/objects-inl.h:6681: uint32_t Hash() {
On 2014/01/16 16:18:18, marja wrote:
> Drive-by coding style nit: don't you need to mark virtual funcs virtual in the
> subclasses too?
I guess we don't need to mark them as virtual. I saw several other places in the
code where they are not marked.

[Sven Panne, 2014-01-17 08:43:24]

On 2014/01/16 16:23:11, ulan wrote:
> https://codereview.chromium.org/140953002/diff/30001/src/objects-inl.h
> File src/objects-inl.h (right):
> 
>
https://codereview.chromium.org/140953002/diff/30001/src/objects-inl.h#newcod...
> src/objects-inl.h:6681: uint32_t Hash() {
> On 2014/01/16 16:18:18, marja wrote:
> > Drive-by coding style nit: don't you need to mark virtual funcs virtual in
the
> > subclasses too?
> I guess we don't need to mark them as virtual. I saw several other places in
the
> code where they are not marked.

Technically, "virtual" is inherited, but for clarity we should add it. In the
same vein, I would prefer a '// static' comment before the implementation of
static methods. IIRC, both things are required by the Google C++ style guide,
but even if not, they are nice to have. Remember: We write programs primarily
for human consumption, not for the compiler. :-)

[ulan, 2014-01-17 09:44:49]

On 2014/01/17 08:43:24, Sven Panne wrote:
> Technically, "virtual" is inherited, but for clarity we should add it. In the
> same vein, I would prefer a '// static' comment before the implementation of
> static methods. IIRC, both things are required by the Google C++ style guide,
> but even if not, they are nice to have. Remember: We write programs primarily
> for human consumption, not for the compiler. :-)
I added "virtual" to the virtual method, and moved string-key classes up where
the string related methods are. Sven, could please take another look?

[Sven Panne, 2014-01-17 09:51:50]

lgtm

[ulan, 2014-01-17 10:28:07]

Committed patchset #4 manually as r18657 (presubmit successful).