Issue 1549 - initialize v8::V8

[Wladimir Palant, 2014-11-11 20:55:40]

You don't need to address the comment below, LGTM if you decide to keep the code
as is.

http://codereview.adblockplus.org/4978827532959744/diff/5629499534213120/src/...
File src/JsEngine.cpp (right):

http://codereview.adblockplus.org/4978827532959744/diff/5629499534213120/src/...
src/JsEngine.cpp:62: }
Wouldn't it make more sense (meaning more obvious and less code) to have a
static variable initialized at startup instead of the explicit method call? In
case you didn't want to call V8::Initialize() if JsEngine isn't being used at
all - in that scenario the linker should optimize this entire file away and
V8Initializer won't be created anyway.

[sergei, 2014-11-12 10:11:32]

http://codereview.adblockplus.org/4978827532959744/diff/5629499534213120/src/...
File src/JsEngine.cpp (right):

http://codereview.adblockplus.org/4978827532959744/diff/5629499534213120/src/...
src/JsEngine.cpp:62: }
On 2014/11/11 20:55:40, Wladimir Palant wrote:
> Wouldn't it make more sense (meaning more obvious and less code) to have a
> static variable initialized at startup instead of the explicit method call? In
> case you didn't want to call V8::Initialize() if JsEngine isn't being used at
> all - in that scenario the linker should optimize this entire file away and
> V8Initializer won't be created anyway.

Nit: I am not sure whether the linker can optimize it out. I would say it has
quite big side effects and I expect the linker to keep it even if it's not
referenced. I certainly remember regarding such things that linker can drop the
whole static library if nothing is referenced from it.

But what bothers me, the order of initialization of global static is undefined.
I'm afraid that there can be something else static in v8 and I think it's safer
to call `v8::V8::Initialize();` after all that stuff is initialized. Thus v8's
internal static is initialized before `main` and then we can safely call v8's
API.

[Wladimir Palant, 2014-11-12 10:18:35]

http://codereview.adblockplus.org/4978827532959744/diff/5629499534213120/src/...
File src/JsEngine.cpp (right):

http://codereview.adblockplus.org/4978827532959744/diff/5629499534213120/src/...
src/JsEngine.cpp:62: }
Yes, order of initialization is something I wasn't sure about either. So - fine
with me, we can keep this as an explicit call.

[Felix Dahlke, 2014-11-19 10:21:37]

LGTM