Remove 'V8' prefix from generated callback function classes

Remove 'V8' prefix from generated callback function classes

It wouldn't make much sense to have 'V8' prefix since V8 specific
conversions are encapsulated in the generated code.

BUG=569301
Committed: https://crrev.com/7395f42e6c47b2e75ea28b0c4db644fd7a25b07a
Cr-Commit-Position: refs/heads/master@{#422646}

[bashi, 2016-09-30 06:58:28]

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

[commit-bot: I haz the power, 2016-09-30 06:58:44]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[bashi, 2016-09-30 07:06:09]

bashi@chromium.org changed reviewers:
+ haraken@chromium.org, lkawai@google.com, peria@chromium.org,
yukishiino@chromium.org

[bashi, 2016-09-30 07:06:10]

PTAL

[peria, 2016-09-30 07:31:45]

It seems reasonable to me to have 'V8' prefix to show those classes handle
v8::Local variables.
Beside it, I also agree to drop the verbose prefix.

IMHO, we should introduce more namespaces than we're discussing in
platform-architecture-dev@...
https://groups.google.com/a/chromium.org/forum/#!topic/platform-architecture-...


Anyway, I don't object to renaming them.
LGTM with a nit.

https://codereview.chromium.org/2383613003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/timing/PerformanceObserver.h (right):

https://codereview.chromium.org/2383613003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/timing/PerformanceObserver.h:21: class
PerformanceObserverCallback;
alphabetical order

[bashi, 2016-09-30 07:53:14]

On 2016/09/30 07:31:45, peria wrote:
> It seems reasonable to me to have 'V8' prefix to show those classes handle
> v8::Local variables.
> Beside it, I also agree to drop the verbose prefix.
> 
> IMHO, we should introduce more namespaces than we're discussing in
> platform-architecture-dev@...
>
https://groups.google.com/a/chromium.org/forum/#!topic/platform-architecture-...

I understand your point but I propose this renaming from the caller's
perspective. For example, a typical usage would look like:

// Foo is a ScriptWrappable
void Foo::method() {
  Vector<String> strings = getStringList();
  m_callback->call(m_scriptState.get(), this, exceptionState, strings);
}

In most cases there is no V8 things when invoking a callback, and the callback
will be passed by the binding layer. In general impl classes aren't necessary
aware of V8.

> 
> 
> Anyway, I don't object to renaming them.
> LGTM with a nit.
> 
>
https://codereview.chromium.org/2383613003/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/timing/PerformanceObserver.h (right):
> 
>
https://codereview.chromium.org/2383613003/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/timing/PerformanceObserver.h:21: class
> PerformanceObserverCallback;
> alphabetical order

[bashi, 2016-09-30 07:53:57]

https://codereview.chromium.org/2383613003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/timing/PerformanceObserver.h (right):

https://codereview.chromium.org/2383613003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/timing/PerformanceObserver.h:21: class
PerformanceObserverCallback;
On 2016/09/30 07:31:44, peria wrote:
> alphabetical order

Done.

[lkawai, 2016-09-30 08:03:39]

LGTM

[Yuki, 2016-09-30 08:06:24]

lgtm

[peria, 2016-09-30 08:08:04]

On 2016/09/30 07:53:14, bashi1 wrote:
> On 2016/09/30 07:31:45, peria wrote:
> > It seems reasonable to me to have 'V8' prefix to show those classes handle
> > v8::Local variables.
> > Beside it, I also agree to drop the verbose prefix.
> > 
> > IMHO, we should introduce more namespaces than we're discussing in
> > platform-architecture-dev@...
> >
>
https://groups.google.com/a/chromium.org/forum/#!topic/platform-architecture-...
> 
> I understand your point but I propose this renaming from the caller's
> perspective. For example, a typical usage would look like:
> 
> // Foo is a ScriptWrappable
> void Foo::method() {
>   Vector<String> strings = getStringList();
>   m_callback->call(m_scriptState.get(), this, exceptionState, strings);
> }
> 
> In most cases there is no V8 things when invoking a callback, and the callback
> will be passed by the binding layer. In general impl classes aren't necessary
> aware of V8.

In that example, ScriptWrappable and ExceptionState are actually V8 things,
even though they don't have the prefix in type names.

[bashi, 2016-09-30 09:30:09]

On 2016/09/30 08:08:04, peria wrote:
> On 2016/09/30 07:53:14, bashi1 wrote:
> > On 2016/09/30 07:31:45, peria wrote:
> > > It seems reasonable to me to have 'V8' prefix to show those classes handle
> > > v8::Local variables.
> > > Beside it, I also agree to drop the verbose prefix.
> > > 
> > > IMHO, we should introduce more namespaces than we're discussing in
> > > platform-architecture-dev@...
> > >
> >
>
https://groups.google.com/a/chromium.org/forum/#!topic/platform-architecture-...
> > 
> > I understand your point but I propose this renaming from the caller's
> > perspective. For example, a typical usage would look like:
> > 
> > // Foo is a ScriptWrappable
> > void Foo::method() {
> >   Vector<String> strings = getStringList();
> >   m_callback->call(m_scriptState.get(), this, exceptionState, strings);
> > }
> > 
> > In most cases there is no V8 things when invoking a callback, and the
callback
> > will be passed by the binding layer. In general impl classes aren't
necessary
> > aware of V8.
> 
> In that example, ScriptWrappable and ExceptionState are actually V8 things,
> even though they don't have the prefix in type names.

Yes, that's my point :)

[haraken, 2016-09-30 13:11:46]

LGTM

[bashi, 2016-10-03 23:21:29]

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

[commit-bot: I haz the power, 2016-10-03 23:21:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-04 00:59:01]

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

[commit-bot: I haz the power, 2016-10-04 00:59:02]

Dry run: This issue passed the CQ dry run.

[bashi, 2016-10-04 00:59:50]

The CQ bit was checked by bashi@chromium.org

[bashi, 2016-10-04 00:59:51]

The patchset sent to the CQ was uploaded after l-g-t-m from peria@chromium.org,
haraken@chromium.org, lkawai@google.com, yukishiino@chromium.org
Link to the patchset: https://codereview.chromium.org/2383613003/#ps40001
(title: "rebase")

[commit-bot: I haz the power, 2016-10-04 01:00:18]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-04 01:05:34]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-10-04 01:07:27]

Description was changed from

==========
Remove 'V8' prefix from generated callback function classes

It wouldn't make much sense to have 'V8' prefix since V8 specific
conversions are encapsulated in the generated code.

BUG=569301
==========

to

==========
Remove 'V8' prefix from generated callback function classes

It wouldn't make much sense to have 'V8' prefix since V8 specific
conversions are encapsulated in the generated code.

BUG=569301

Committed: https://crrev.com/7395f42e6c47b2e75ea28b0c4db644fd7a25b07a
Cr-Commit-Position: refs/heads/master@{#422646}
==========

[commit-bot: I haz the power, 2016-10-04 01:07:28]

Patchset 3 (id:??) landed as
https://crrev.com/7395f42e6c47b2e75ea28b0c4db644fd7a25b07a
Cr-Commit-Position: refs/heads/master@{#422646}