Rename toWebCoreString*() utility methods to toCoreString*()

Rename toWebCoreString*() utility methods to toCoreString*()

Rename toWebCoreString*() utility methods to toCoreString*() as WebCore no
longer exists.

R=haraken, pfeldman
BUG=327338
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=163759

[Inactive, 2013-12-11 20:52:11]

This is a first step in the direction we discussed on the bug. I am planning to
add the new utility methods we agreed on in a follow-up patch.

[eseidel, 2013-12-11 22:23:50]

I would leave them as "to" since that's the pattern in the rest of core/.  I
find the "as" confusing, and assume it means "static_cast" or something.   These
might malloc, so they shouldn't really just be operators, so I think leaving
them as toCoreString is fine.  I don't think most would expect them to call
"toString" on the JS object.

[Inactive, 2013-12-12 00:11:24]

On 2013/12/11 22:23:50, eseidel wrote:
> I would leave them as "to" since that's the pattern in the rest of core/.  I
> find the "as" confusing, and assume it means "static_cast" or something.  
These
> might malloc, so they shouldn't really just be operators, so I think leaving
> them as toCoreString is fine.  I don't think most would expect them to call
> "toString" on the JS object.

We had toWebCoreString(v8::Handle<v8::Value>) utility methods that we calling
ToString() in the argument. I recently removed them in favor of the
V8TRYCATCH_FOR_V8STRINGRESOURCE() macro. However, pfeldman thought it would be a
good idea to keep such utility methods but have them NOT call ToString() on the
js Object (As this may throw an exception). I thought that if we kept the same
naming as before but changed the behavior to not call ToString(), it would be
confusing. Thus the proposal to use "as" prefix instead of "to".

In this preliminary CL, I was renaming the "new"
toWebCoreString(v8::Handle<v8::String>) utility methods for consistency with the
utility methods that I am about to bring back (the
toWebCoreString(v8::Handle<v8::Value>) ones).

I don't mind keeping the "to" prefix. I am just saying that people might expect
them to call ToString() on the argument if the argument is a v8::Value, since
this is what these methods were doing up until recently, with the same name /
prototype.

[haraken, 2013-12-12 00:18:27]

On 2013/12/12 00:11:24, Chris Dumez wrote:
> On 2013/12/11 22:23:50, eseidel wrote:
> > I would leave them as "to" since that's the pattern in the rest of core/.  I
> > find the "as" confusing, and assume it means "static_cast" or something.  
> These
> > might malloc, so they shouldn't really just be operators, so I think leaving
> > them as toCoreString is fine.  I don't think most would expect them to call
> > "toString" on the JS object.
> 
> We had toWebCoreString(v8::Handle<v8::Value>) utility methods that we calling
> ToString() in the argument. I recently removed them in favor of the
> V8TRYCATCH_FOR_V8STRINGRESOURCE() macro. However, pfeldman thought it would be
a
> good idea to keep such utility methods but have them NOT call ToString() on
the
> js Object (As this may throw an exception). I thought that if we kept the same
> naming as before but changed the behavior to not call ToString(), it would be
> confusing. Thus the proposal to use "as" prefix instead of "to".
> 
> In this preliminary CL, I was renaming the "new"
> toWebCoreString(v8::Handle<v8::String>) utility methods for consistency with
the
> utility methods that I am about to bring back (the
> toWebCoreString(v8::Handle<v8::Value>) ones).
> 
> I don't mind keeping the "to" prefix. I am just saying that people might
expect
> them to call ToString() on the argument if the argument is a v8::Value, since
> this is what these methods were doing up until recently, with the same name /
> prototype.

I'd prefer toCoreString() since other conversion methods in V8Binding.h that can
throw exceptions are called toXXX(). (e.g., toInt32() can throw an exception.)

[Inactive, 2013-12-12 00:23:10]

On 2013/12/12 00:18:27, haraken wrote:
> On 2013/12/12 00:11:24, Chris Dumez wrote:
> > On 2013/12/11 22:23:50, eseidel wrote:
> > > I would leave them as "to" since that's the pattern in the rest of core/. 
I
> > > find the "as" confusing, and assume it means "static_cast" or something.  
> > These
> > > might malloc, so they shouldn't really just be operators, so I think
leaving
> > > them as toCoreString is fine.  I don't think most would expect them to
call
> > > "toString" on the JS object.
> > 
> > We had toWebCoreString(v8::Handle<v8::Value>) utility methods that we
calling
> > ToString() in the argument. I recently removed them in favor of the
> > V8TRYCATCH_FOR_V8STRINGRESOURCE() macro. However, pfeldman thought it would
be
> a
> > good idea to keep such utility methods but have them NOT call ToString() on
> the
> > js Object (As this may throw an exception). I thought that if we kept the
same
> > naming as before but changed the behavior to not call ToString(), it would
be
> > confusing. Thus the proposal to use "as" prefix instead of "to".
> > 
> > In this preliminary CL, I was renaming the "new"
> > toWebCoreString(v8::Handle<v8::String>) utility methods for consistency with
> the
> > utility methods that I am about to bring back (the
> > toWebCoreString(v8::Handle<v8::Value>) ones).
> > 
> > I don't mind keeping the "to" prefix. I am just saying that people might
> expect
> > them to call ToString() on the argument if the argument is a v8::Value,
since
> > this is what these methods were doing up until recently, with the same name
/
> > prototype.
> 
> I'd prefer toCoreString() since other conversion methods in V8Binding.h that
can
> throw exceptions are called toXXX(). (e.g., toInt32() can throw an exception.)

haraken, I don't understand. My point is that they (toWebCoreString*() methods)
*cannot* throw anymore since they don't call ToString(). This is why I am trying
to move away from the toXXX() naming and proposing asXXX().

[haraken, 2013-12-12 00:36:47]

> > I'd prefer toCoreString() since other conversion methods in V8Binding.h that
> can
> > throw exceptions are called toXXX(). (e.g., toInt32() can throw an
exception.)
> 
> haraken, I don't understand. My point is that they (toWebCoreString*()
methods)
> *cannot* throw anymore since they don't call ToString(). This is why I am
trying
> to move away from the toXXX() naming and proposing asXXX().

Sorry, my description was the other way. My point is that toXXX() in V8Binding.h
don't care about whether they throw exceptions or not. toInt32() throws an
exception but toWebCoreDate(), toDOMWindow() etc don't throw exceptions.

Question: Are you planning to have both toCoreString() that throws an exception
and toCoreString() that doesn't throw an exception? Then it would make sense to
distinguish the two methods. Otherwise, I think we can simply call it
toCoreString().

[Inactive, 2013-12-12 00:39:57]

> Sorry, my description was the other way. My point is that toXXX() in
V8Binding.h
> don't care about whether they throw exceptions or not. toInt32() throws an
> exception but toWebCoreDate(), toDOMWindow() etc don't throw exceptions.

Ok, now it makes sense :)

> Question: Are you planning to have both toCoreString() that throws an
exception
> and toCoreString() that doesn't throw an exception? Then it would make sense
to
> distinguish the two methods. Otherwise, I think we can simply call it
> toCoreString().

No, I am not planning to have methods that throw an exceptions since we use the
V8TRYCATCH_FOR_V8STRINGRESOURCE*() macros instead.

Ok then, I will keep the "to" prefix.

[Inactive, 2013-12-12 01:04:42]

On 2013/12/12 00:39:57, Chris Dumez wrote:
> > Sorry, my description was the other way. My point is that toXXX() in
> V8Binding.h
> > don't care about whether they throw exceptions or not. toInt32() throws an
> > exception but toWebCoreDate(), toDOMWindow() etc don't throw exceptions.
> 
> Ok, now it makes sense :)
> 
> > Question: Are you planning to have both toCoreString() that throws an
> exception
> > and toCoreString() that doesn't throw an exception? Then it would make sense
> to
> > distinguish the two methods. Otherwise, I think we can simply call it
> > toCoreString().
> 
> No, I am not planning to have methods that throw an exceptions since we use
the
> V8TRYCATCH_FOR_V8STRINGRESOURCE*() macros instead.
> 
> Ok then, I will keep the "to" prefix.

Done. Please take another look.

[haraken, 2013-12-12 01:10:22]

LGTM, but we should wait for an approval from pfeldman.

[pfeldman, 2013-12-12 06:23:09]

lgtm. haraken, this is a simple rename, right? why wait for approval :-)

[haraken, 2013-12-12 06:24:14]

On 2013/12/12 06:23:09, pfeldman wrote:
> lgtm. haraken, this is a simple rename, right? why wait for approval :-)

Yes, simple rename. I just wanted to make sure you like the new name :)

[commit-bot: I haz the power, 2013-12-12 06:24:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ch.dumez@samsung.com/113213002/20001

[commit-bot: I haz the power, 2013-12-12 07:41:36]

Change committed as 163759