Relax check for message loop in SequencedWorkerPool

Relax check for message loop in SequencedWorkerPool

Assert that the constructor message loop is non-NULL only when
the SequencedWorkerPool is about to be destroyed.

Revert now-unneccessary test changes that adds MessagePools for
SequenedWorkerPool.

BUG=117940
TEST=


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=127335

[akalin, 2012-03-16 10:00:30]

+joth for review
+brettw for OWNERS

[joth, 2012-03-16 12:04:04]

LGTM

This looks fine to me, a potential alternative did also occur which I thought
I'd throw out there...

https://chromiumcodereview.appspot.com/9699115/diff/1/base/threading/sequence...
File base/threading/sequenced_worker_pool.h (right):

https://chromiumcodereview.appspot.com/9699115/diff/1/base/threading/sequence...
base/threading/sequenced_worker_pool.h:132: // leak it.
FWIW reading this another approach occurred to me: we could make it explicit in
the API when an instance is intended to be leaked by requiring an extra
SequencedWorkerPool::LEAKY enum param to be passed, the absence of which would
invoke the current DCHECK for message loop (or even, allow the user to pass in
the desired destruct message loop).

Then in OnDestroy if the destruction_message_loop_ is NULL it can just bail out
(i.e. implementing the 'leak' semantics, even if the caller releases all
ref-counts onto the pool). (Optionally it could go into a half-dead state, where
it would DCHECK if any new tasks were posted).

[brettw, 2012-03-16 19:27:25]

lgtm

[commit-bot: I haz the power, 2012-03-16 19:30:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/akalin@chromium.org/9699115/1

[akalin, 2012-03-16 19:31:49]

On 2012/03/16 12:04:04, joth wrote:
>
https://chromiumcodereview.appspot.com/9699115/diff/1/base/threading/sequence...
> base/threading/sequenced_worker_pool.h:132: // leak it.
> FWIW reading this another approach occurred to me: we could make it explicit
in
> the API when an instance is intended to be leaked by requiring an extra
> SequencedWorkerPool::LEAKY enum param to be passed, the absence of which would
> invoke the current DCHECK for message loop (or even, allow the user to pass in
> the desired destruct message loop).
> 
> Then in OnDestroy if the destruction_message_loop_ is NULL it can just bail
out
> (i.e. implementing the 'leak' semantics, even if the caller releases all
> ref-counts onto the pool). (Optionally it could go into a half-dead state,
where
> it would DCHECK if any new tasks were posted).

Yeah, that's possible, although I don't know if it's worth it.  I'll stick with
the approach here for now; if later it becomes confusing, I can switch to your
suggestion.

[commit-bot: I haz the power, 2012-03-16 22:18:41]

Try job failure for 9699115-1 (retry) on win_rel for step "browser_tests".
It's a second try, previously, steps "ui_tests, browser_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2012-03-16 22:22:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/akalin@chromium.org/9699115/1

[commit-bot: I haz the power, 2012-03-17 00:54:12]

Try job failure for 9699115-1 (retry) on win_rel for step "browser_tests".
It's a second try, previously, step "browser_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2012-03-17 01:01:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/akalin@chromium.org/9699115/1

[commit-bot: I haz the power, 2012-03-17 03:27:55]

Change committed as 127335