Key pending EarlyTraceEvents by name+tid.

Chrome tracing allows starting similarly named events from different
threads. EarlyTraceEvents throws an exception in this case because it
keys pending events by name only.

This CL changes pending events key to be name+tid, separating events
from different threads.

Bug: 807034
Change-Id: Ia6d4ac740ec028a14c5269d09148db4d1a5d33ce
Reviewed-on: https://chromium-review.googlesource.com/1055941
Commit-Queue: Dmitry Skiba <dskiba@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558829}

base/android/java/src/org/chromium/base/EarlyTraceEvent.java

@@ -106,7 +106,8 @@ public class EarlyTraceEvent {
     @VisibleForTesting static volatile int sState = STATE_DISABLED;
     // Not final as these object are not likely to be used at all.
     @VisibleForTesting static List<Event> sCompletedEvents;
-    @VisibleForTesting static Map<String, Event> sPendingEvents;
+    @VisibleForTesting
+    static Map<String, Event> sPendingEventByKey;
     @VisibleForTesting static List<AsyncEvent> sAsyncEvents;
     @VisibleForTesting static List<String> sPendingAsyncEvents;
 
@@ -138,7 +139,7 @@ public class EarlyTraceEvent {
         synchronized (sLock) {
             if (sState != STATE_DISABLED) return;
             sCompletedEvents = new ArrayList<Event>();
-            sPendingEvents = new HashMap<String, Event>();
+            sPendingEventByKey = new HashMap<String, Event>();
             sAsyncEvents = new ArrayList<AsyncEvent>();
             sPendingAsyncEvents = new ArrayList<String>();
             sState = STATE_ENABLED;
@@ -183,7 +184,7 @@ public class EarlyTraceEvent {
         Event conflictingEvent;
         synchronized (sLock) {
             if (!enabled()) return;
-            conflictingEvent = sPendingEvents.put(name, event);
+            conflictingEvent = sPendingEventByKey.put(makeEventKeyForCurrentThread(name), event);
         }
         if (conflictingEvent != null) {
             throw new IllegalArgumentException(
@@ -196,7 +197,7 @@ public class EarlyTraceEvent {
         if (!isActive()) return;
         synchronized (sLock) {
             if (!isActive()) return;
-            Event event = sPendingEvents.remove(name);
+            Event event = sPendingEventByKey.remove(makeEventKeyForCurrentThread(name));
             if (event == null) return;
             event.end();
             sCompletedEvents.add(event);
@@ -231,7 +232,7 @@ public class EarlyTraceEvent {
     static void resetForTesting() {
         sState = EarlyTraceEvent.STATE_DISABLED;
         sCompletedEvents = null;
-        sPendingEvents = null;
+        sPendingEventByKey = null;
         sAsyncEvents = null;
         sPendingAsyncEvents = null;
     }
@@ -245,9 +246,9 @@ public class EarlyTraceEvent {
             dumpAsyncEvents(sAsyncEvents);
             sAsyncEvents.clear();
         }
-        if (sPendingEvents.isEmpty() && sPendingAsyncEvents.isEmpty()) {
+        if (sPendingEventByKey.isEmpty() && sPendingAsyncEvents.isEmpty()) {
             sState = STATE_FINISHED;
-            sPendingEvents = null;
+            sPendingEventByKey = null;
             sCompletedEvents = null;
             sPendingAsyncEvents = null;
             sAsyncEvents = null;
@@ -279,6 +280,16 @@ public class EarlyTraceEvent {
         return nativeNowNanos - javaNowNanos;
     }
 
+    /**
+     * Returns a key which consists of |name| and the ID of the current thread.
+     * The key is used with pending events making them thread-specific, thus avoiding
+     * an exception when similarly named events are started from multiple threads.
+     */
+    @VisibleForTesting
+    static String makeEventKeyForCurrentThread(String name) {
+        return name + "@" + Process.myTid();
+    }
+
     private static native void nativeRecordEarlyEvent(String name, long beginTimNanos,
             long endTimeNanos, int threadId, long threadDurationMillis);
     private static native void nativeRecordEarlyStartAsyncEvent(

base/android/javatests/src/org/chromium/base/EarlyTraceEventTest.java

@@ -50,7 +50,7 @@ public class EarlyTraceEventTest {
         long afterNanos = Event.elapsedRealtimeNanos();
 
         Assert.assertEquals(1, EarlyTraceEvent.sCompletedEvents.size());
-        Assert.assertTrue(EarlyTraceEvent.sPendingEvents.isEmpty());
+        Assert.assertTrue(EarlyTraceEvent.sPendingEventByKey.isEmpty());
         Event event = EarlyTraceEvent.sCompletedEvents.get(0);
         Assert.assertEquals(EVENT_NAME, event.mName);
         Assert.assertEquals(myThreadId, event.mThreadId);
@@ -71,7 +71,7 @@ public class EarlyTraceEventTest {
         long afterNanos = Event.elapsedRealtimeNanos();
 
         Assert.assertEquals(2, EarlyTraceEvent.sAsyncEvents.size());
-        Assert.assertTrue(EarlyTraceEvent.sPendingEvents.isEmpty());
+        Assert.assertTrue(EarlyTraceEvent.sPendingEventByKey.isEmpty());
         AsyncEvent eventStart = EarlyTraceEvent.sAsyncEvents.get(0);
         AsyncEvent eventEnd = EarlyTraceEvent.sAsyncEvents.get(1);
         Assert.assertEquals(EVENT_NAME, eventStart.mName);
@@ -111,7 +111,7 @@ public class EarlyTraceEventTest {
         long afterNanos = Event.elapsedRealtimeNanos();
 
         Assert.assertEquals(1, EarlyTraceEvent.sCompletedEvents.size());
-        Assert.assertTrue(EarlyTraceEvent.sPendingEvents.isEmpty());
+        Assert.assertTrue(EarlyTraceEvent.sPendingEventByKey.isEmpty());
         Event event = EarlyTraceEvent.sCompletedEvents.get(0);
         Assert.assertEquals(EVENT_NAME, event.mName);
         Assert.assertEquals(myThreadId, event.mThreadId);
@@ -129,15 +129,16 @@ public class EarlyTraceEventTest {
         EarlyTraceEvent.begin(EVENT_NAME);
 
         Assert.assertTrue(EarlyTraceEvent.sCompletedEvents.isEmpty());
-        Assert.assertEquals(1, EarlyTraceEvent.sPendingEvents.size());
-        EarlyTraceEvent.Event event = EarlyTraceEvent.sPendingEvents.get(EVENT_NAME);
+        Assert.assertEquals(1, EarlyTraceEvent.sPendingEventByKey.size());
+        EarlyTraceEvent.Event event = EarlyTraceEvent.sPendingEventByKey.get(
+                EarlyTraceEvent.makeEventKeyForCurrentThread(EVENT_NAME));
         Assert.assertEquals(EVENT_NAME, event.mName);
     }
 
     @Test
     @SmallTest
     @Feature({"Android-AppBase"})
-    public void testNoDuplicatePendingEvents() {
+    public void testNoDuplicatePendingEventsFromSameThread() {
         EarlyTraceEvent.enable();
         EarlyTraceEvent.begin(EVENT_NAME);
         try {
@@ -149,6 +150,21 @@ public class EarlyTraceEventTest {
         Assert.fail();
     }
 
+    @Test
+    @SmallTest
+    @Feature({"Android-AppBase"})
+    public void testDuplicatePendingEventsFromDifferentThreads() throws Exception {
+        EarlyTraceEvent.enable();
+
+        Thread otherThread = new Thread(() -> { EarlyTraceEvent.begin(EVENT_NAME); });
+        otherThread.start();
+        otherThread.join();
+
+        // At this point we have a pending event with EVENT_NAME name. But events are per
+        // thread, so we should be able to start EVENT_NAME event in a different thread.
+        EarlyTraceEvent.begin(EVENT_NAME);
+    }
+
     @Test
     @SmallTest
     @Feature({"Android-AppBase"})
@@ -182,7 +198,7 @@ public class EarlyTraceEventTest {
         EarlyTraceEvent.begin(EVENT_NAME2);
         EarlyTraceEvent.end(EVENT_NAME2);
 
-        Assert.assertEquals(1, EarlyTraceEvent.sPendingEvents.size());
+        Assert.assertEquals(1, EarlyTraceEvent.sPendingEventByKey.size());
         Assert.assertTrue(EarlyTraceEvent.sCompletedEvents.isEmpty());
     }