Cleanup v8 usage in core

Cleanup v8 usage in core

* Explicitly include v8.h where used.
* Do *not* forward declare v8 symbols in headers, per:
http://www.chromium.org/developers/coding-style/cpp-dos-and-donts#TOC-Exceptions

There are 27 files in core (9 .h + 18 .cpp) that use v8.
This corrects usage.

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=175551

[haraken, 2014-06-05 05:52:30]

Unfortunately, not LGTM.

We discussed this before and decided to just include "v8.h". You can see
discussion here: https://codereview.chromium.org/168533002/

[Nils Barth (inactive), 2014-06-05 06:05:31]

On 2014/06/05 05:52:30, haraken wrote:
> Unfortunately, not LGTM.
> 
> We discussed this before and decided to just include "v8.h". You can see
> discussion here: https://codereview.chromium.org/168533002/

Got it, thanks!
1. Updated docs to mention exception
http://www.chromium.org/developers/coding-style/cpp-dos-and-donts#TOC-Exceptions

2. Revised CL so only corrects transitive includes and moves includes to .cpp if
not used in .h
(and replaces 2 existing uses of forward decl of v8 with include)

[haraken, 2014-06-05 06:08:07]

LGTM, thanks!

[Nils Barth (inactive), 2014-06-05 06:11:06]

The CQ bit was checked by nbarth@chromium.org

[commit-bot: I haz the power, 2014-06-05 06:11:56]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/nbarth@chromium.org/318843003/40001

[Nils Barth (inactive), 2014-06-05 07:17:41]

The CQ bit was unchecked by nbarth@chromium.org

[Nils Barth (inactive), 2014-06-05 07:19:13]

On 2014/06/05 07:17:41, Nils Barth wrote:
> The CQ bit was unchecked by mailto:nbarth@chromium.org

Turns out there are build failures if we #include <v8.h> in WebCoreTestSupport.h
instead of using a forward declaration, notably that Skia doesn't build.
To avoid requiring additional include paths/dependencies for Skia, restored this
to using a fwd decl,
and added a comment explaining why.

[Nils Barth (inactive), 2014-06-05 07:19:19]

The CQ bit was checked by nbarth@chromium.org

[commit-bot: I haz the power, 2014-06-05 07:20:11]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/nbarth@chromium.org/318843003/60001

[yurys, 2014-06-05 07:39:17]

Hmm, we used to have Script* abstractions to isolate core/ from VM specific
APIs, has that changed?

At the moment we use both v8 APIs directly in the code under core/ and Script*
abstractions which is odd. We should either keep using Script* abstractions and
move all code working with objects from v8:: namespace under bindings/v8/
(including v8.h should be prohibited under core/) or explicitly allow using v8
API directly from core/. In the latter case we would eliminate some intermediate
layers in the inspector code that just wrap V8 objects.

[Nils Barth (inactive), 2014-06-05 08:00:06]

On 2014/06/05 07:39:17, yurys wrote:
> Hmm, we used to have Script* abstractions to isolate core/ from VM specific
> APIs, has that changed?
> 
> At the moment we use both v8 APIs directly in the code under core/ and Script*
> abstractions which is odd. We should either keep using Script* abstractions
and
> move all code working with objects from v8:: namespace under bindings/v8/
> (including v8.h should be prohibited under core/) or explicitly allow using v8
> API directly from core/. In the latter case we would eliminate some
intermediate
> layers in the inspector code that just wrap V8 objects.

Hi Yury,
Agreed that this should be fixed!
This CL is just making the dependencies explicit, which should help in resolving
them.

I've opened a bug for this:
http://crbug.com/381046

This hasn't been a priority, since componentizing core vs. modules is the first
task,
after which we plan to address core vs. bindings_core (currently intertwined).

Part of this involves moving these V8 calls into an abstraction layer below core
(gin?);
I've commented on this briefly as followup to "componentizing core vs. modules
in bindings":
https://docs.google.com/document/d/1TXth_c0l5hExCwFtZJaS9DFRfHC45zwdzCaAiIF1Q...

[haraken, 2014-06-05 08:09:22]

> Hmm, we used to have Script* abstractions to isolate core/ from VM specific
> APIs, has that changed?

No, we don't want to allow v8:: in core/ and modules/ :)

However, these files already have v8::, so we added v8.h. So this should be
removed with the v8::.

[commit-bot: I haz the power, 2014-06-05 08:32:00]

Change committed as 175551

[yurys, 2014-06-05 08:37:05]

On 2014/06/05 08:00:06, Nils Barth wrote:
> On 2014/06/05 07:39:17, yurys wrote:
> > Hmm, we used to have Script* abstractions to isolate core/ from VM specific
> > APIs, has that changed?
> > 
> > At the moment we use both v8 APIs directly in the code under core/ and
Script*
> > abstractions which is odd. We should either keep using Script* abstractions
> and
> > move all code working with objects from v8:: namespace under bindings/v8/
> > (including v8.h should be prohibited under core/) or explicitly allow using
v8
> > API directly from core/. In the latter case we would eliminate some
> intermediate
> > layers in the inspector code that just wrap V8 objects.
> 
> Hi Yury,
> Agreed that this should be fixed!
> This CL is just making the dependencies explicit, which should help in
resolving
> them.
> 
> I've opened a bug for this:
> http://crbug.com/381046
> 
> This hasn't been a priority, since componentizing core vs. modules is the
first
> task,
> after which we plan to address core vs. bindings_core (currently intertwined).
> 
> Part of this involves moving these V8 calls into an abstraction layer below
core
> (gin?);
> I've commented on this briefly as followup to "componentizing core vs. modules
> in bindings":
>
https://docs.google.com/document/d/1TXth_c0l5hExCwFtZJaS9DFRfHC45zwdzCaAiIF1Q...

Thanks for clarifying! It wasn't obvious from the CL description.