Kick the pump when allowing nestable tasks on a message loop

Kick the pump when allowing nestable tasks on a message loop

BUG=318998
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=236405

[jamesr, 2013-11-20 01:29:23]

My windows box is offline right now so I can't easily confirm this, but I'm
working on a test that I'll post whenever the network comes up.

[cpu_(ooo_6.6-7.5), 2013-11-20 01:55:46]

lgtm

[jamesr, 2013-11-20 21:00:54]

Check it out, a unit test!

https://codereview.chromium.org/65173003/diff/100001/base/message_loop/messag...
File base/message_loop/message_loop_unittest.cc (right):

https://codereview.chromium.org/65173003/diff/100001/base/message_loop/messag...
base/message_loop/message_loop_unittest.cc:1129: EXPECT_TRUE(did_run);
this check fails without the message_loop.cc patch since we don't end up running
the task before the timer fires

[cpu_(ooo_6.6-7.5), 2013-11-20 21:31:24]

https://codereview.chromium.org/65173003/diff/100001/base/message_loop/messag...
File base/message_loop/message_loop_unittest.cc (right):

https://codereview.chromium.org/65173003/diff/100001/base/message_loop/messag...
base/message_loop/message_loop_unittest.cc:1109: }
check message

https://codereview.chromium.org/65173003/diff/100001/base/message_loop/messag...
base/message_loop/message_loop_unittest.cc:1120: // get to run any posted tasks.
comment is stale?

https://codereview.chromium.org/65173003/diff/100001/base/message_loop/messag...
base/message_loop/message_loop_unittest.cc:1139: string16 class_name =
L"MessageLoopTest_HWND";
unneeded class_name

https://codereview.chromium.org/65173003/diff/100001/base/message_loop/messag...
base/message_loop/message_loop_unittest.cc:1146: atom = RegisterClassEx(&wc);
ATOM atom =

https://codereview.chromium.org/65173003/diff/100001/base/message_loop/messag...
base/message_loop/message_loop_unittest.cc:1158:
ASSERT_TRUE(DestroyWindow(message_hwnd));
UnregisterClass(atom)

[jamesr, 2013-11-21 00:30:40]

OK!  Carlos and Ananta helped me get the test up and going.  It's set up now
where it passes very quickly without races and times out when it fails, which is
fine since it should always pass.

jar@ - could you do the honors and give this an OWNERS review?  Kicking the pump
here in SetNestableTasksAllowed(true) is definitely a hack and doesn't cover all
cases but covers the most obvious cases and has very little performance risk, so
I'd like to land this first and merge it to m32 while we consider more holistic
fixes.

[jar (doing other things), 2013-11-21 01:20:44]

lgtm

This seems like a minimalist change, as the SetNestableTasksAllowed() is one of
my most-discouraged-to-call methods.  I'm hopeful it won't break tests... or
induce a perf regression... so be sure to watch the perf bots (it is *amazing*
how many "minor" changes in message loop can cause impressively large impact).

[jamesr, 2013-11-21 01:26:07]

On 2013/11/21 01:20:44, jar wrote:
> This seems like a minimalist change, as the SetNestableTasksAllowed() is one
of
> my most-discouraged-to-call methods.  I'm hopeful it won't break tests... or
> induce a perf regression... so be sure to watch the perf bots (it is *amazing*
> how many "minor" changes in message loop can cause impressively large impact).

Thanks.  I'll definitely keep a close eye on the perf bots.

[commit-bot: I haz the power, 2013-11-21 01:46:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/65173003/210001

[commit-bot: I haz the power, 2013-11-21 06:11:03]

Change committed as 236405