Implementation of MessagePump for Mojo.

Implementation of MessagePump for Mojo.

I've refactored a bunch of the common MessagePump code into
message_pump_test. This allows us to share tests for custom
messagePumps and should allow moving MessagePumpForUI out later
on. Sadly it results in a ginormous macro. On the positive side it's
easy to use.

BUG=none
TEST=none
R=darin@chromium.org

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

[Ben Goodger (Google), 2013-11-08 21:24:34]

https://codereview.chromium.org/66193007/diff/30001/mojo/common/message_pump_...
File mojo/common/message_pump_mojo.cc (right):

https://codereview.chromium.org/66193007/diff/30001/mojo/common/message_pump_...
mojo/common/message_pump_mojo.cc:46: MojoHandle handle,
mojo::Handle, mojo::WriteMessage etc?

[darin (slow to review), 2013-11-09 00:47:49]

LGTM, caveat nits:

https://codereview.chromium.org/66193007/diff/30001/base/message_loop/message...
File base/message_loop/message_loop_test.h (right):

https://codereview.chromium.org/66193007/diff/30001/base/message_loop/message...
base/message_loop/message_loop_test.h:21: void
RunTest_PostTask(MessagePumpFactory* factory);
Do these need to be a pointer to a function pointer? You can just pass
MessagePumpFactory by value, right?

https://codereview.chromium.org/66193007/diff/30001/mojo/common/message_pump_...
File mojo/common/message_pump_mojo.cc (right):

https://codereview.chromium.org/66193007/diff/30001/mojo/common/message_pump_...
mojo/common/message_pump_mojo.cc:129: wait_state.handles.size(),
nit: win64 build will complain here. need
static_cast<uint32_t>(wait_state.handles.size())

https://codereview.chromium.org/66193007/diff/30001/mojo/common/message_pump_...
File mojo/common/message_pump_mojo_handler.h (right):

https://codereview.chromium.org/66193007/diff/30001/mojo/common/message_pump_...
mojo/common/message_pump_mojo_handler.h:20: virtual void
OnHandleInvalid(MojoHandle handle, MojoResult result) = 0;
Perhaps this should be more generic such as OnHandleError? MojoWait could
(should) fail if the other end of the pipe is closed. That isn't the same thing
as someone closing this end of the pipe.

[commit-bot: I haz the power, 2013-11-11 14:18:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sky@chromium.org/66193007/280001

[sky, 2013-11-11 15:14:17]

https://codereview.chromium.org/66193007/diff/30001/base/message_loop/message...
File base/message_loop/message_loop_test.h (right):

https://codereview.chromium.org/66193007/diff/30001/base/message_loop/message...
base/message_loop/message_loop_test.h:21: void
RunTest_PostTask(MessagePumpFactory* factory);
On 2013/11/09 00:47:50, darin wrote:
> Do these need to be a pointer to a function pointer? You can just pass
> MessagePumpFactory by value, right?

You're right.

https://codereview.chromium.org/66193007/diff/30001/mojo/common/message_pump_...
File mojo/common/message_pump_mojo.cc (right):

https://codereview.chromium.org/66193007/diff/30001/mojo/common/message_pump_...
mojo/common/message_pump_mojo.cc:129: wait_state.handles.size(),
On 2013/11/09 00:47:50, darin wrote:
> nit: win64 build will complain here. need
> static_cast<uint32_t>(wait_state.handles.size())

Done.

[commit-bot: I haz the power, 2013-11-11 19:50:10]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...

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

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sky@chromium.org/66193007/280001

[commit-bot: I haz the power, 2013-11-11 21:47:29]

Failed to apply patch for mojo/mojo.gyp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file mojo/mojo.gyp
  Hunk #1 succeeded at 69 (offset 12 lines).
  Hunk #2 FAILED at 77.
  Hunk #3 FAILED at 84.
  2 out of 3 hunks FAILED -- saving rejects to file mojo/mojo.gyp.rej

Patch:       mojo/mojo.gyp
Index: mojo/mojo.gyp
diff --git a/mojo/mojo.gyp b/mojo/mojo.gyp
index
72e6477a521f07a7457cd8365cf9bcd4e12551e2..fc8d8c0ba61091ec9129007b35c79b2aa50360f9
100644
--- a/mojo/mojo.gyp
+++ b/mojo/mojo.gyp
@@ -57,6 +57,9 @@
       'sources': [
         'common/handle_watcher.cc',
         'common/handle_watcher.h',
+        'common/message_pump_mojo.cc',
+        'common/message_pump_mojo.h',
+        'common/message_pump_mojo_handler.h',
         'common/scoped_message_pipe.cc',
         'common/scoped_message_pipe.h',
       ],
@@ -74,6 +77,7 @@
       'type': 'executable',
       'dependencies': [
         '../base/base.gyp:base',
+        '../base/base.gyp:base_message_loop_tests',
         '../base/base.gyp:run_all_unittests',
         '../testing/gtest.gyp:gtest',
         'mojo_common_lib',
@@ -81,6 +85,7 @@
       ],
       'sources': [
         'common/handle_watcher_unittest.cc',
+        'common/message_pump_mojo_unittest.cc',
         'common/test/run_all_unittests.cc',
       ],
       'conditions': [

[commit-bot: I haz the power, 2013-11-11 22:26:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sky@chromium.org/66193007/540001

[commit-bot: I haz the power, 2013-11-12 02:19:28]

Retried try job too often on win7_aura for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...

[sky, 2013-11-12 16:35:06]

Committed patchset #4 manually as r234540 (presubmit successful).