Simplify ownership of a ThreadState's interruptors.

Simplify ownership of a ThreadState's interruptors.

Have ThreadState assume ownership of interruptors registered with
addInterruptor(). That simplifies the handling on the caller side,
allowing them to register-and-forget rather than each having to
manually unregister & destruct their interruptor before shutting down.
(Not that this last step wasn't needed previously either, as ThreadState
would have destructed the interruptors upon its destruction.)

(ThreadState::removeInterruptor() is retained should it prove useful, but
currently unused.)

R=haraken
BUG=420515
Committed: https://crrev.com/4800e03bbc37af803129ee4f56d0d1b3153e13b3
git-svn-id: svn://svn.chromium.org/blink/trunk@199487 bbb929c8-8fbe-4397-9dbb-9b2b20218538

[sof, 2015-07-24 18:40:19]

sigbjornf@opera.com changed reviewers:
+ oilpan-reviews@chromium.org

[sof, 2015-07-24 18:40:19]

please take a look.

[haraken, 2015-07-24 18:58:32]

haraken@chromium.org changed reviewers:
+ haraken@chromium.org

[haraken, 2015-07-24 18:58:33]

Maybe can we simplify things a bit more by making each ThreadState own an
Interruptor by default?

class ThreadState {
  OwnPtr<Interruptor> m_interruptor; // This is created in ThreadState::attach()
and destructed in ThreadState::detach().
};

https://codereview.chromium.org/1257723002/diff/1/Source/platform/heap/Thread...
File Source/platform/heap/ThreadState.cpp (right):

https://codereview.chromium.org/1257723002/diff/1/Source/platform/heap/Thread...
Source/platform/heap/ThreadState.cpp:1283: void
ThreadState::addInterruptor(Interruptor* interruptor)

Shall we make addInterruptor take PassOwnPtr<Interruptor> so that the caller
side can pass adoptPtr(new Interruptor)? It will make the ownership a bit
clearer.

[sof, 2015-07-25 15:00:40]

Why would making ThreadState have an interruptor be a worthwhile improvement?
Both the main thread and the compositor worker manager's WebThread keep two
(isolate + message loop), so it's not as if we can remove the need for
addInterruptor().

https://codereview.chromium.org/1257723002/diff/1/Source/platform/heap/Thread...
File Source/platform/heap/ThreadState.cpp (right):

https://codereview.chromium.org/1257723002/diff/1/Source/platform/heap/Thread...
Source/platform/heap/ThreadState.cpp:1283: void
ThreadState::addInterruptor(Interruptor* interruptor)
On 2015/07/24 18:58:33, haraken wrote:
> 
> Shall we make addInterruptor take PassOwnPtr<Interruptor> so that the caller
> side can pass adoptPtr(new Interruptor)? It will make the ownership a bit
> clearer.
> 
> 

Yes, preferable to encode ownership transfer in the argument type.

[haraken, 2015-07-26 17:32:29]

On 2015/07/25 15:00:40, sof wrote:
> Why would making ThreadState have an interruptor be a worthwhile improvement?
> Both the main thread and the compositor worker manager's WebThread keep two
> (isolate + message loop), so it's not as if we can remove the need for
> addInterruptor().

Makes sense.

LGTM

[sof, 2015-07-27 05:12:30]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2015-07-27 05:12:39]

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

[commit-bot: I haz the power, 2015-07-27 07:04:18]

Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=199487

[commit-bot: I haz the power, 2015-09-23 11:47:43]

Patchset 2 (id:??) landed as
https://crrev.com/4800e03bbc37af803129ee4f56d0d1b3153e13b3