Adding a router class to handle messages that expect responses.

mojo/bindings/java/src/org/chromium/mojo/bindings/AutoCloseableRouter.java

Index: mojo/bindings/java/src/org/chromium/mojo/bindings/AutoCloseableRouter.java
diff --git a/mojo/bindings/java/src/org/chromium/mojo/bindings/AutoCloseableRouter.java b/mojo/bindings/java/src/org/chromium/mojo/bindings/AutoCloseableRouter.java
new file mode 100644
index 0000000000000000000000000000000000000000..aacfc0b97268ec58fc43a377c75a81468fe7a4e6
--- /dev/null
+++ b/mojo/bindings/java/src/org/chromium/mojo/bindings/AutoCloseableRouter.java
@@ -0,0 +1,111 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.mojo.bindings;
+
+import org.chromium.mojo.system.Core;
+import org.chromium.mojo.system.MessagePipeHandle;
+
+import java.util.concurrent.Executor;
+
+/**
+ * Wrapper around {@link Router} that will close the connection when not referenced anymore.
+ */
+class AutoCloseableRouter implements Router {
+
+    /**
+     * The underlying router.
+     */
+    private final Router mRouter;
+
+    /**
+     * The executor to close the underlying router.
+     */
+    private final Executor mExecutor;
+
+    /**
+     * Constructor.
+     */
+    public AutoCloseableRouter(Core core, Router router) {
+        mRouter = router;
+        mExecutor = core == null ? null : ExecutorFactory.getExecutorForCurrentThread(core);

[rmcilroy, 2014/07/10 19:03:57]

I don't think you need the 'core == null' check given that the class will not be
useful if null is passed as core, since it will immediately close the object.  I
would prefer a NPE here if it is used incorrectly rather than a harder to debug
error later when the router is unexpectedly closed.

[qsr, 2014/07/11 11:42:07]

Done.

+        if (mExecutor == null) {
+            mRouter.close();
+        }
+    }
+
+    /**
+     * @see Router#setIncomingMessageReceiver(MessageReceiverWithResponder)
+     */
+    @Override
+    public void setIncomingMessageReceiver(MessageReceiverWithResponder incomingMessageReceiver) {
+        mRouter.setIncomingMessageReceiver(incomingMessageReceiver);
+    }
+
+    /**
+     * @see HandleOwner#passHandle()
+     */
+    @Override
+    public MessagePipeHandle passHandle() {
+        return mRouter.passHandle();
+    }
+
+    /**
+     * @see MessageReceiver#accept(Message)
+     */
+    @Override
+    public boolean accept(Message message) {
+        return mRouter.accept(message);
+    }
+
+    /**
+     * @see MessageReceiverWithResponder#acceptWithResponder(Message, MessageReceiver)
+     */
+    @Override
+    public boolean acceptWithResponder(Message message, MessageReceiver receiver) {
+        return mRouter.acceptWithResponder(message, receiver);
+
+    }
+
+    /**
+     * @see Router#start()
+     */
+    @Override
+    public void start() {
+        mRouter.start();
+    }
+
+    /**
+     * @see Router#setErrorHandler(ConnectionErrorHandler)
+     */
+    @Override
+    public void setErrorHandler(ConnectionErrorHandler errorHandler) {
+        mRouter.setErrorHandler(errorHandler);
+    }
+
+    /**
+     * @see java.io.Closeable#close()
+     */
+    @Override
+    public void close() {
+        mRouter.close();
+    }
+
+    /**
+     * @see Object#finalize()
+     */
+    @Override
+    protected void finalize() throws Throwable {
+        if (mExecutor != null) {

[rmcilroy, 2014/07/10 19:03:57]

Keep track of whether the router is already closed and only close in finalizer
if not already closed.  Also log a great big noisy warning message here (and I
would also advise to throw an exception in debug builds) because if this is ever
reached then the user of the class is doing something wrong. 

[qsr, 2014/07/11 11:42:07]

 I cannot log anything, as there is no standard logging mechanism. I can throw
unconditionally, as finalizer just log the exception and do not do anything.
That will be enough to see the issues, but that will not break anything.

[rmcilroy, 2014/07/14 14:44:56]

Could you use the C MojoLogger?  (I'm fine with this being a followup CL given
you've added the exception)

[qsr, 2014/07/15 12:03:15]

 I cannot do this in this module, that is pure java and platform independant. If
we want to be able to use the C MojoLogger, we need to add an API for it in
Core, which mean designing a new (yet again) java interface for logging. Do you
want this? Given that I expect java application to use their own native logging
API, and that this is currently the only place where we need logging, I'm not
sure we really want this.

[rmcilroy, 2014/07/15 18:11:57]

For this case it isn't necessary.  I'm a bit worried that there is no unified
way to do logging for the Java Mojo bindings though.

+            mExecutor.execute(new Runnable() {
+
+                @Override
+                public void run() {
+                    close();
+                }
+            });
+        }
+        super.finalize();
+    }
+}