[d8 Workers] Fix bug creating Worker during main thread termination

[d8 Workers] Fix bug creating Worker during main thread termination

When the main thread terminates, it forcibly terminates all Worker threads.
When this happens, the threads objects were only half-created; they had a
JavaScript Worker object, but not a C++ worker object.

This CL fixes that bug, as well as some other fixes:
* Signatures on Worker methods
* Use SetAlignedPointerFromInternalField instead of using an External.
* Remove state_ from Worker. Simplify to atomic bool running_.

BUG=chromium:511880
R=jarin@chromium.org
LOG=n

Committed: https://crrev.com/a87db3dec86e220a402377ca244f879ed0128bfb
Cr-Commit-Position: refs/heads/master@{#29911}

[Jarin, 2015-07-23 07:48:42]

One question. If this is answered, it looks good.

https://codereview.chromium.org/1255563002/diff/20001/src/d8.cc
File src/d8.cc (right):

https://codereview.chromium.org/1255563002/diff/20001/src/d8.cc#newcode1842
src/d8.cc:1842: state_ = TERMINATED;
What if we call quit() from one worker and worker.terminate() from another one?
Cannot there be a data race?

[binji, 2015-07-24 18:52:29]

https://codereview.chromium.org/1255563002/diff/20001/src/d8.cc
File src/d8.cc (right):

https://codereview.chromium.org/1255563002/diff/20001/src/d8.cc#newcode1842
src/d8.cc:1842: state_ = TERMINATED;
On 2015/07/23 at 07:48:41, jarin wrote:
> What if we call quit() from one worker and worker.terminate() from another
one? Cannot there be a data race?

Yes, you're right. I've removed the state_ (because it was mostly for
debugging), and replaced it with a simple atomic bool.

[Jarin, 2015-07-27 05:09:29]

lgtm.

https://codereview.chromium.org/1255563002/diff/40001/src/d8.cc
File src/d8.cc (right):

https://codereview.chromium.org/1255563002/diff/40001/src/d8.cc#newcode780
src/d8.cc:780: }
How about factoring out the extraction of the worker from the args into a helper
function? 

Also, could you DCHECK that InternalFielCount is 1? Just to exclude the
possibility that we deal with some stray object.

[binji, 2015-07-28 22:46:46]

Patchset #4 (id:60001) has been deleted

[binji, 2015-07-28 22:47:38]

The CQ bit was checked by binji@chromium.org

[binji, 2015-07-28 22:47:38]

The patchset sent to the CQ was uploaded after l-g-t-m from jarin@chromium.org
Link to the patchset: https://codereview.chromium.org/1255563002/#ps80001
(title: "typo")

[commit-bot: I haz the power, 2015-07-28 22:47:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1255563002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1255563002/80001

[binji, 2015-07-29 04:37:14]

https://codereview.chromium.org/1255563002/diff/40001/src/d8.cc
File src/d8.cc (right):

https://codereview.chromium.org/1255563002/diff/40001/src/d8.cc#newcode780
src/d8.cc:780: }
On 2015/07/27 at 05:09:28, Jarin (OOO - vacation) wrote:
> How about factoring out the extraction of the worker from the args into a
helper function? 

Done.

> 
> Also, could you DCHECK that InternalFielCount is 1? Just to exclude the
possibility that we deal with some stray object.

DCHECK or runtime check? I'm hesitant to do a DCHECK because cluster-fuzz :)

[commit-bot: I haz the power, 2015-07-30 08:19:32]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2015-07-30 08:19:49]

Patchset 4 (id:??) landed as
https://crrev.com/a87db3dec86e220a402377ca244f879ed0128bfb
Cr-Commit-Position: refs/heads/master@{#29911}