Adding an invalid handle in the public API.

Also cleanup of the implementation by removing the unused CoreSingleton
class.

R=bulach@chromium.org,viettrungluu@chromium.org
BUG=

Review URL: https://codereview.chromium.org/260623006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@267516 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 93 insertions, 47 deletions

diff --git a/mojo/android/javatests/src/org/chromium/mojo/system/CoreTest.java b/mojo/android/javatests/src/org/chromium/mojo/system/CoreTest.java
index 97f0912..bf941b0 100644
--- a/mojo/android/javatests/src/org/chromium/mojo/system/CoreTest.java
+++ b/mojo/android/javatests/src/org/chromium/mojo/system/CoreTest.java
@@ -99,7 +99,7 @@ public class CoreTest extends InstrumentationTestCase {
      */
     @SmallTest
     public void testWaitMany() {
-        Core core = CoreSingleton.getInstance();
+        Core core = CoreImpl.getInstance();
         Pair<MessagePipeHandle, MessagePipeHandle> handles = core.createMessagePipe();
 
         try {
@@ -135,7 +135,7 @@ public class CoreTest extends InstrumentationTestCase {
      */
     @SmallTest
     public void testMessagePipeEmpty() {
-        Core core = CoreSingleton.getInstance();
+        Core core = CoreImpl.getInstance();
         Pair<MessagePipeHandle, MessagePipeHandle> handles = core.createMessagePipe();
 
         try {
@@ -189,7 +189,7 @@ public class CoreTest extends InstrumentationTestCase {
      */
     @SmallTest
     public void testMessagePipeSend() {
-        Core core = CoreSingleton.getInstance();
+        Core core = CoreImpl.getInstance();
         Pair<MessagePipeHandle, MessagePipeHandle> handles = core.createMessagePipe();
 
         try {
@@ -207,7 +207,7 @@ public class CoreTest extends InstrumentationTestCase {
     @SmallTest
     public void testMessagePipeReceiveOnSmallBuffer() {
         Random random = new Random();
-        Core core = CoreSingleton.getInstance();
+        Core core = CoreImpl.getInstance();
         Pair<MessagePipeHandle, MessagePipeHandle> handles = core.createMessagePipe();
 
         try {
@@ -236,7 +236,7 @@ public class CoreTest extends InstrumentationTestCase {
      */
     @SmallTest
     public void testMessagePipeSendHandles() {
-        Core core = CoreSingleton.getInstance();
+        Core core = CoreImpl.getInstance();
         Pair<MessagePipeHandle, MessagePipeHandle> handles = core.createMessagePipe();
         Pair<MessagePipeHandle, MessagePipeHandle> handlesToShare = core.createMessagePipe();
 
@@ -266,7 +266,7 @@ public class CoreTest extends InstrumentationTestCase {
     }
 
     private static void createAndCloseDataPipe(DataPipe.CreateOptions options) {
-        Core core = CoreSingleton.getInstance();
+        Core core = CoreImpl.getInstance();
         Pair<DataPipe.ProducerHandle, DataPipe.ConsumerHandle> handles = core.createDataPipe(
                 options);
         handles.first.close();
@@ -297,7 +297,7 @@ public class CoreTest extends InstrumentationTestCase {
      */
     @SmallTest
     public void testDataPipeSend() {
-        Core core = CoreSingleton.getInstance();
+        Core core = CoreImpl.getInstance();
         Random random = new Random();
 
         Pair<DataPipe.ProducerHandle, DataPipe.ConsumerHandle> handles = core.createDataPipe(null);
@@ -337,7 +337,7 @@ public class CoreTest extends InstrumentationTestCase {
     @SmallTest
     public void testDataPipeTwoPhaseSend() {
         Random random = new Random();
-        Core core = CoreSingleton.getInstance();
+        Core core = CoreImpl.getInstance();
         Pair<DataPipe.ProducerHandle, DataPipe.ConsumerHandle> handles = core.createDataPipe(null);
 
         try {
@@ -371,7 +371,7 @@ public class CoreTest extends InstrumentationTestCase {
     @SmallTest
     public void testDataPipeDiscard() {
         Random random = new Random();
-        Core core = CoreSingleton.getInstance();
+        Core core = CoreImpl.getInstance();
         Pair<DataPipe.ProducerHandle, DataPipe.ConsumerHandle> handles = core.createDataPipe(null);
 
         try {
@@ -410,7 +410,7 @@ public class CoreTest extends InstrumentationTestCase {
      */
     @SmallTest
     public void testSharedBufferCreation() {
-        Core core = CoreSingleton.getInstance();
+        Core core = CoreImpl.getInstance();
         // Test creation with empty options.
         core.createSharedBuffer(null, 8).close();
         // Test creation with default options.
@@ -422,7 +422,7 @@ public class CoreTest extends InstrumentationTestCase {
      */
     @SmallTest
     public void testSharedBufferDuplication() {
-        Core core = CoreSingleton.getInstance();
+        Core core = CoreImpl.getInstance();
         SharedBufferHandle handle = core.createSharedBuffer(null, 8);
         try {
             // Test duplication with empty options.
@@ -440,7 +440,7 @@ public class CoreTest extends InstrumentationTestCase {
     @SmallTest
     public void testSharedBufferSending() {
         Random random = new Random();
-        Core core = CoreSingleton.getInstance();
+        Core core = CoreImpl.getInstance();
         SharedBufferHandle handle = core.createSharedBuffer(null, 8);
         SharedBufferHandle newHandle = handle.duplicate(null);
 
@@ -467,5 +467,56 @@ public class CoreTest extends InstrumentationTestCase {
         }
     }
 
+    /**
+     * Testing that invalid handle can be used with this implementation.
+     */
+    @SmallTest
+    public void testInvalidHandle() {
+        Core core = CoreImpl.getInstance();
+        Handle handle = new InvalidHandle();
+
+        // Checking wait.
+        boolean exception = false;
+        try {
+            core.wait(handle, WaitFlags.all(), 0);
+        } catch (MojoException e) {
+            assertEquals(MojoResult.INVALID_ARGUMENT, e.getMojoResult());
+            exception = true;
+        }
+        assertTrue(exception);
+
+        // Checking waitMany.
+        exception = false;
+        try {
+            List<Pair<Handle, WaitFlags>> handles = new ArrayList<Pair<Handle, WaitFlags>>();
+            handles.add(Pair.create(handle, WaitFlags.all()));
+            core.waitMany(handles, 0);
+        } catch (MojoException e) {
+            assertEquals(MojoResult.INVALID_ARGUMENT, e.getMojoResult());
+            exception = true;
+        }
+        assertTrue(exception);
+
+        // Checking sending an invalid handle.
+        // Until the behavior is changed on the C++ side, handle gracefully 2 different use case:
+        // - Receive a INVALID_ARGUMENT exception
+        // - Receive an invalid handle on the other side.
+        Pair<MessagePipeHandle, MessagePipeHandle> handles = core.createMessagePipe();
+        try {
+            handles.first.writeMessage(null,
+                    Collections.<Handle> singletonList(handle),
+                    WriteFlags.none());
+            ReadMessageResult readMessageResult = handles.second.readMessage(null, 1,
+                    ReadFlags.none());
+            assertEquals(1, readMessageResult.getHandlesCount());
+            assertFalse(readMessageResult.getHandles().get(0).isValid());
+        } catch (MojoException e) {
+            assertEquals(MojoResult.INVALID_ARGUMENT, e.getMojoResult());
+        } finally {
+            handles.first.close();
+            handles.second.close();
+        }
+    }
+
     private native void nativeInitApplicationContext(Context context);
 }
diff --git a/mojo/android/system/src/org/chromium/mojo/system/CoreImpl.java b/mojo/android/system/src/org/chromium/mojo/system/CoreImpl.java
index 23f0bf7..cd6cee5 100644
--- a/mojo/android/system/src/org/chromium/mojo/system/CoreImpl.java
+++ b/mojo/android/system/src/org/chromium/mojo/system/CoreImpl.java
@@ -20,7 +20,7 @@ import java.util.List;
  * Implementation of {@link Core}.
  */
 @JNINamespace("mojo::android")
-class CoreImpl implements Core {
+public class CoreImpl implements Core {
 
     /**
      * Discard flag for the |MojoReadData| operation.
@@ -37,8 +37,13 @@ class CoreImpl implements Core {
      */
     private static final int FLAG_SIZE = 4;
 
+    /**
+     * The mojo handle for an invalid handle.
+     */
+    static final int INVALID_HANDLE = 0;
+
     private static class LazyHolder {
-      private static final Core INSTANCE = new CoreImpl();
+        private static final Core INSTANCE = new CoreImpl();
     }
 
     /**
@@ -70,8 +75,7 @@ class CoreImpl implements Core {
         ByteBuffer buffer = allocateDirectBuffer(handles.size() * 8);
         int index = 0;
         for (Pair<Handle, WaitFlags> handle : handles) {
-            HandleImpl realHandler = (HandleImpl) handle.first;
-            buffer.putInt(HANDLE_SIZE * index, realHandler.getMojoHandle());
+            buffer.putInt(HANDLE_SIZE * index, getMojoHandle(handle.first));
             buffer.putInt(HANDLE_SIZE * handles.size() + FLAG_SIZE * index,
                     handle.second.getFlags());
             index++;
@@ -90,7 +94,7 @@ class CoreImpl implements Core {
      */
     @Override
     public int wait(Handle handle, WaitFlags flags, long deadline) {
-        return filterMojoResultForWait(nativeWait(((HandleImpl) handle).getMojoHandle(),
+        return filterMojoResultForWait(nativeWait(getMojoHandle(handle),
                 flags.getFlags(), deadline));
     }
 
@@ -103,7 +107,7 @@ class CoreImpl implements Core {
         if (result.getMojoResult() != MojoResult.OK) {
             throw new MojoException(result.getMojoResult());
         }
-        return Pair.<MessagePipeHandle, MessagePipeHandle>create(
+        return Pair.<MessagePipeHandle, MessagePipeHandle> create(
                 new MessagePipeHandleImpl(this, result.getMojoHandle1()),
                 new MessagePipeHandleImpl(this, result.getMojoHandle2()));
     }
@@ -125,7 +129,7 @@ class CoreImpl implements Core {
         if (result.getMojoResult() != MojoResult.OK) {
             throw new MojoException(result.getMojoResult());
         }
-        return Pair.<ProducerHandle, ConsumerHandle>create(
+        return Pair.<ProducerHandle, ConsumerHandle> create(
                 new DataPipeProducerHandleImpl(this, result.getMojoHandle1()),
                 new DataPipeConsumerHandleImpl(this, result.getMojoHandle2()));
     }
@@ -170,8 +174,7 @@ class CoreImpl implements Core {
         if (handles != null && !handles.isEmpty()) {
             handlesBuffer = allocateDirectBuffer(handles.size() * HANDLE_SIZE);
             for (Handle handle : handles) {
-                HandleImpl realHandle = (HandleImpl) handle;
-                handlesBuffer.putInt(realHandle.getMojoHandle());
+                handlesBuffer.putInt(getMojoHandle(handle));
             }
             handlesBuffer.position(0);
         }
@@ -184,7 +187,9 @@ class CoreImpl implements Core {
         // Success means the handles have been invalidated.
         if (handles != null) {
             for (Handle handle : handles) {
-                ((HandleImpl) handle).invalidateHandle();
+                if (handle.isValid()) {
+                    ((HandleImpl) handle).invalidateHandle();
+                }
             }
         }
     }
@@ -357,6 +362,16 @@ class CoreImpl implements Core {
         }
     }
 
+    /**
+     * @return the mojo handle associated to the given handle, considering invalid handles.
+     */
+    private int getMojoHandle(Handle handle) {
+        if (handle.isValid()) {
+            return ((HandleImpl) handle).getMojoHandle();
+        }
+        return 0;
+    }
+
     private static int filterMojoResultForWait(int code) {
         if (code >= 0) {
             return MojoResult.OK;
diff --git a/mojo/android/system/src/org/chromium/mojo/system/CoreSingleton.java b/mojo/android/system/src/org/chromium/mojo/system/CoreSingleton.java
deleted file mode 100644
index 3cdbf96..0000000
--- a/mojo/android/system/src/org/chromium/mojo/system/CoreSingleton.java
+++ /dev/null
@@ -1,18 +0,0 @@
-// 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.system;
-
-/**
- * Access to the core singleton.
- */
-public class CoreSingleton {
-
-    /**
-     * Access to the {@link Core} singleton.
-     */
-    public static Core getInstance() {
-        return CoreImpl.getInstance();
-    }
-}
diff --git a/mojo/android/system/src/org/chromium/mojo/system/HandleImpl.java b/mojo/android/system/src/org/chromium/mojo/system/HandleImpl.java
index 00cb9cd..5dff103 100644
--- a/mojo/android/system/src/org/chromium/mojo/system/HandleImpl.java
+++ b/mojo/android/system/src/org/chromium/mojo/system/HandleImpl.java
@@ -15,8 +15,6 @@ class HandleImpl implements Handle {
 
     private static final String TAG = "HandleImpl";
 
-    private static final int INVALID_HANDLE = 0;
-
     /**
      * The pointer to the scoped handle owned by this object.
      */
@@ -42,7 +40,7 @@ class HandleImpl implements Handle {
         mCore = other.mCore;
         HandleImpl otherAsHandleImpl = other;
         int mojoHandle = otherAsHandleImpl.mMojoHandle;
-        otherAsHandleImpl.mMojoHandle = INVALID_HANDLE;
+        otherAsHandleImpl.mMojoHandle = CoreImpl.INVALID_HANDLE;
         mMojoHandle = mojoHandle;
     }
 
@@ -51,10 +49,10 @@ class HandleImpl implements Handle {
      */
     @Override
     public void close() {
-        if (mMojoHandle != INVALID_HANDLE) {
+        if (mMojoHandle != CoreImpl.INVALID_HANDLE) {
             // After a close, the handle is invalid whether the close succeed or not.
             int handle = mMojoHandle;
-            mMojoHandle = INVALID_HANDLE;
+            mMojoHandle = CoreImpl.INVALID_HANDLE;
             mCore.close(handle);
         }
     }
@@ -72,7 +70,7 @@ class HandleImpl implements Handle {
      */
     @Override
     public boolean isValid() {
-        return mMojoHandle != INVALID_HANDLE;
+        return mMojoHandle != CoreImpl.INVALID_HANDLE;
     }
 
     /**
@@ -88,7 +86,7 @@ class HandleImpl implements Handle {
      * invalidate the handle. The caller must ensures that the handle does not leak.
      */
     void invalidateHandle() {
-        mMojoHandle = INVALID_HANDLE;
+        mMojoHandle = CoreImpl.INVALID_HANDLE;
     }
 
     /**