Reland r17877 - Introduce a v8::Platform class that bundles embedder callbacks

Reland r17877 - Introduce a v8::Platform class that bundles embedder callbacks

Over the initial commit, this CL moves the lifetime management of the default
platform to v8.cc from api.cc

R=bmeurer@chromium.org, svenpanne@chromium.org
BUG=v8:3015
LOG=n

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

[jochen (gone - plz use gerrit), 2013-11-20 16:06:17]

I ran cctest in debug mode locally and they worked \o/

[Sven Panne, 2013-11-21 07:43:25]

https://codereview.chromium.org/78453003/diff/1/include/v8-platform.h
File include/v8-platform.h (right):

https://codereview.chromium.org/78453003/diff/1/include/v8-platform.h#newcode42
include/v8-platform.h:42: virtual void Run() = 0;
I think that at some point we need something like C++'s packaged_task / future /
promise. How will it fit into this?

https://codereview.chromium.org/78453003/diff/1/include/v8-platform.h#newcode60
include/v8-platform.h:60: virtual void CallOnBackgroundThread(Task* task, bool
task_is_slow) = 0;
Hmmm, why do we need task_is_slow? And what exactly should it mean to be "slow"?
This seems to have very subjective semantics (= no semantics at all ;-), so I
would like to avoid such wishy-washy concepts in an API.

[Benedikt Meurer, 2013-11-21 07:49:27]

https://codereview.chromium.org/78453003/diff/1/include/v8-platform.h
File include/v8-platform.h (right):

https://codereview.chromium.org/78453003/diff/1/include/v8-platform.h#newcode60
include/v8-platform.h:60: virtual void CallOnBackgroundThread(Task* task, bool
task_is_slow) = 0;
In case we need to keep the "task_is_slow" parameter, can we please turn it into
an enum then?

[jochen (gone - plz use gerrit), 2013-11-21 10:43:41]

ptal

https://codereview.chromium.org/78453003/diff/1/include/v8-platform.h
File include/v8-platform.h (right):

https://codereview.chromium.org/78453003/diff/1/include/v8-platform.h#newcode42
include/v8-platform.h:42: virtual void Run() = 0;
On 2013/11/21 07:43:25, Sven Panne wrote:
> I think that at some point we need something like C++'s packaged_task / future
/
> promise. How will it fit into this?

perfectly it will fit!

The main purpose of this class is to be able to delete whatever thing we end up
using. An implementation would look like this (in chromium).

class ConcreteTask : public Task {
 public:
  explicit ConcreteTask(const base::Closure& closure) : closure_(closure) {}
  virtual ~ConcreteTask() {}

  virtual void Run() { closure_.Run(); }
 private:
  base::Closure closure_;
};

you can then platform->CallOnBackgroundThread(new ConcreteTask(base::Bind(&Foo,
whatever, else)), false)

https://codereview.chromium.org/78453003/diff/1/include/v8-platform.h#newcode60
include/v8-platform.h:60: virtual void CallOnBackgroundThread(Task* task, bool
task_is_slow) = 0;
On 2013/11/21 07:43:25, Sven Panne wrote:
> Hmmm, why do we need task_is_slow? And what exactly should it mean to be
"slow"?
> This seems to have very subjective semantics (= no semantics at all ;-), so I
> would like to avoid such wishy-washy concepts in an API.

It's basically mirroring the windows threading API.

This bool maps to the flag WT_EXECUTELONGFUNCTION

https://codereview.chromium.org/78453003/diff/1/include/v8-platform.h#newcode60
include/v8-platform.h:60: virtual void CallOnBackgroundThread(Task* task, bool
task_is_slow) = 0;
On 2013/11/21 07:49:27, Benedikt Meurer wrote:
> In case we need to keep the "task_is_slow" parameter, can we please turn it
into
> an enum then?

sure

[Sven Panne, 2013-11-21 11:57:14]

lgtm

[Benedikt Meurer, 2013-11-21 12:59:22]

LGTM

[jochen (gone - plz use gerrit), 2013-11-21 14:07:19]

Committed patchset #2 manually as r17970 (presubmit successful).