Add remaining DDMS messages into DdmPublishChunk

This ensures that one can get notified of every DDMS chunk that would
be published.

Add a test to ensure that appropriate ddms events are sent.

We also refactor the underlying code so that the only way these events
are sent is through the DdmPublishChunk callback.

Test: ./test.py --host -j50
Bug: 62821960
Change-Id: I0c4a60b75615c206a27aff17df853b53cf5e8c96

6 files changed, 157 insertions, 73 deletions

diff --git a/runtime/debugger.cc b/runtime/debugger.cc
index 613e4fe7c7..3784212ef0 100644
--- a/runtime/debugger.cc
+++ b/runtime/debugger.cc
@@ -345,7 +345,14 @@ Dbg::DbgThreadLifecycleCallback Dbg::thread_lifecycle_callback_;
 Dbg::DbgClassLoadCallback Dbg::class_load_callback_;
 
 void DebuggerDdmCallback::DdmPublishChunk(uint32_t type, const ArrayRef<const uint8_t>& data) {
-  Dbg::DdmSendChunk(type, data);
+  if (gJdwpState == nullptr) {
+    VLOG(jdwp) << "Debugger thread not active, ignoring DDM send: " << type;
+  } else {
+    iovec vec[1];
+    vec[0].iov_base = reinterpret_cast<void*>(const_cast<uint8_t*>(data.data()));
+    vec[0].iov_len = data.size();
+    gJdwpState->DdmSendChunkV(type, vec, 1);
+  }
 }
 
 bool DebuggerActiveMethodInspectionCallback::IsMethodBeingInspected(ArtMethod* m ATTRIBUTE_UNUSED) {
@@ -4458,10 +4465,11 @@ void Dbg::DdmSendThreadNotification(Thread* t, uint32_t type) {
     return;
   }
 
+  RuntimeCallbacks* cb = Runtime::Current()->GetRuntimeCallbacks();
   if (type == CHUNK_TYPE("THDE")) {
     uint8_t buf[4];
     JDWP::Set4BE(&buf[0], t->GetThreadId());
-    Dbg::DdmSendChunk(CHUNK_TYPE("THDE"), 4, buf);
+    cb->DdmPublishChunk(CHUNK_TYPE("THDE"), ArrayRef<const uint8_t>(buf));
   } else {
     CHECK(type == CHUNK_TYPE("THCR") || type == CHUNK_TYPE("THNM")) << type;
     ScopedObjectAccessUnchecked soa(Thread::Current());
@@ -4480,7 +4488,7 @@ void Dbg::DdmSendThreadNotification(Thread* t, uint32_t type) {
       JDWP::AppendUtf16BE(bytes, chars, char_count);
     }
     CHECK_EQ(bytes.size(), char_count*2 + sizeof(uint32_t)*2);
-    Dbg::DdmSendChunk(type, bytes);
+    cb->DdmPublishChunk(type, ArrayRef<const uint8_t>(bytes));
   }
 }
 
@@ -4523,30 +4531,6 @@ void Dbg::PostThreadDeath(Thread* t) {
   Dbg::PostThreadStartOrStop(t, CHUNK_TYPE("THDE"));
 }
 
-void Dbg::DdmSendChunk(uint32_t type, const ArrayRef<const uint8_t>& data) {
-  DdmSendChunk(type, data.size(), data.data());
-}
-
-void Dbg::DdmSendChunk(uint32_t type, size_t byte_count, const uint8_t* buf) {
-  CHECK(buf != nullptr);
-  iovec vec[1];
-  vec[0].iov_base = reinterpret_cast<void*>(const_cast<uint8_t*>(buf));
-  vec[0].iov_len = byte_count;
-  Dbg::DdmSendChunkV(type, vec, 1);
-}
-
-void Dbg::DdmSendChunk(uint32_t type, const std::vector<uint8_t>& bytes) {
-  DdmSendChunk(type, bytes.size(), &bytes[0]);
-}
-
-void Dbg::DdmSendChunkV(uint32_t type, const iovec* iov, int iov_count) {
-  if (gJdwpState == nullptr) {
-    VLOG(jdwp) << "Debugger thread not active, ignoring DDM send: " << type;
-  } else {
-    gJdwpState->DdmSendChunkV(type, iov, iov_count);
-  }
-}
-
 JDWP::JdwpState* Dbg::GetJdwpState() {
   return gJdwpState;
 }
@@ -4624,7 +4608,8 @@ void Dbg::DdmSendHeapInfo(HpifWhen reason) {
   JDWP::Append4BE(bytes, heap->GetBytesAllocated());
   JDWP::Append4BE(bytes, heap->GetObjectsAllocated());
   CHECK_EQ(bytes.size(), 4U + (heap_count * (4 + 8 + 1 + 4 + 4 + 4 + 4)));
-  Dbg::DdmSendChunk(CHUNK_TYPE("HPIF"), bytes);
+  Runtime::Current()->GetRuntimeCallbacks()->DdmPublishChunk(CHUNK_TYPE("HPIF"),
+                                                             ArrayRef<const uint8_t>(bytes));
 }
 
 enum HpsgSolidity {
@@ -4710,7 +4695,8 @@ class HeapChunkContext {
     CHECK_LE(pieceLenField_, p_);
     JDWP::Set4BE(pieceLenField_, totalAllocationUnits_);
 
-    Dbg::DdmSendChunk(type_, p_ - &buf_[0], &buf_[0]);
+    ArrayRef<const uint8_t> out(&buf_[0], p_ - &buf_[0]);
+    Runtime::Current()->GetRuntimeCallbacks()->DdmPublishChunk(type_, out);
     Reset();
   }
 
@@ -4892,6 +4878,7 @@ void Dbg::DdmSendHeapSegments(bool native) {
   if (when == HPSG_WHEN_NEVER) {
     return;
   }
+  RuntimeCallbacks* cb = Runtime::Current()->GetRuntimeCallbacks();
   // Figure out what kind of chunks we'll be sending.
   CHECK(what == HPSG_WHAT_MERGED_OBJECTS || what == HPSG_WHAT_DISTINCT_OBJECTS)
       << static_cast<int>(what);
@@ -4899,7 +4886,8 @@ void Dbg::DdmSendHeapSegments(bool native) {
   // First, send a heap start chunk.
   uint8_t heap_id[4];
   JDWP::Set4BE(&heap_id[0], 1);  // Heap id (bogus; we only have one heap).
-  Dbg::DdmSendChunk(native ? CHUNK_TYPE("NHST") : CHUNK_TYPE("HPST"), sizeof(heap_id), heap_id);
+  cb->DdmPublishChunk(native ? CHUNK_TYPE("NHST") : CHUNK_TYPE("HPST"),
+                      ArrayRef<const uint8_t>(heap_id));
   Thread* self = Thread::Current();
   Locks::mutator_lock_->AssertSharedHeld(self);
 
@@ -4958,7 +4946,8 @@ void Dbg::DdmSendHeapSegments(bool native) {
   }
 
   // Finally, send a heap end chunk.
-  Dbg::DdmSendChunk(native ? CHUNK_TYPE("NHEN") : CHUNK_TYPE("HPEN"), sizeof(heap_id), heap_id);
+  cb->DdmPublishChunk(native ? CHUNK_TYPE("NHEN") : CHUNK_TYPE("HPEN"),
+                      ArrayRef<const uint8_t>(heap_id));
 }
 
 void Dbg::SetAllocTrackingEnabled(bool enable) {
diff --git a/runtime/debugger.h b/runtime/debugger.h
index c3184e8374..d5bad8dc67 100644
--- a/runtime/debugger.h
+++ b/runtime/debugger.h
@@ -662,14 +662,6 @@ class Dbg {
   static bool DdmHandlePacket(JDWP::Request* request, uint8_t** pReplyBuf, int* pReplyLen);
   static void DdmConnected() REQUIRES_SHARED(Locks::mutator_lock_);
   static void DdmDisconnected() REQUIRES_SHARED(Locks::mutator_lock_);
-  static void DdmSendChunk(uint32_t type, const ArrayRef<const uint8_t>& bytes)
-      REQUIRES_SHARED(Locks::mutator_lock_);
-  static void DdmSendChunk(uint32_t type, const std::vector<uint8_t>& bytes)
-      REQUIRES_SHARED(Locks::mutator_lock_);
-  static void DdmSendChunk(uint32_t type, size_t len, const uint8_t* buf)
-      REQUIRES_SHARED(Locks::mutator_lock_);
-  static void DdmSendChunkV(uint32_t type, const iovec* iov, int iov_count)
-      REQUIRES_SHARED(Locks::mutator_lock_);
 
   // Visit breakpoint roots, used to prevent unloading of methods with breakpoints.
   static void VisitRoots(RootVisitor* visitor)
diff --git a/runtime/trace.cc b/runtime/trace.cc
index 4b5a7610a3..a113ab5cc8 100644
--- a/runtime/trace.cc
+++ b/runtime/trace.cc
@@ -413,42 +413,40 @@ void Trace::StopTracing(bool finish_tracing, bool flush_file) {
     sampling_pthread_ = 0U;
   }
 
-  {
+  if (the_trace != nullptr) {
+    stop_alloc_counting = (the_trace->flags_ & Trace::kTraceCountAllocs) != 0;
+    if (finish_tracing) {
+      the_trace->FinishTracing();
+    }
     gc::ScopedGCCriticalSection gcs(self,
                                     gc::kGcCauseInstrumentation,
                                     gc::kCollectorTypeInstrumentation);
     ScopedSuspendAll ssa(__FUNCTION__);
-    if (the_trace != nullptr) {
-      stop_alloc_counting = (the_trace->flags_ & Trace::kTraceCountAllocs) != 0;
-      if (finish_tracing) {
-        the_trace->FinishTracing();
-      }
 
-      if (the_trace->trace_mode_ == TraceMode::kSampling) {
-        MutexLock mu(self, *Locks::thread_list_lock_);
-        runtime->GetThreadList()->ForEach(ClearThreadStackTraceAndClockBase, nullptr);
+    if (the_trace->trace_mode_ == TraceMode::kSampling) {
+      MutexLock mu(self, *Locks::thread_list_lock_);
+      runtime->GetThreadList()->ForEach(ClearThreadStackTraceAndClockBase, nullptr);
+    } else {
+      runtime->GetInstrumentation()->DisableMethodTracing(kTracerInstrumentationKey);
+      runtime->GetInstrumentation()->RemoveListener(
+          the_trace, instrumentation::Instrumentation::kMethodEntered |
+          instrumentation::Instrumentation::kMethodExited |
+          instrumentation::Instrumentation::kMethodUnwind);
+    }
+    if (the_trace->trace_file_.get() != nullptr) {
+      // Do not try to erase, so flush and close explicitly.
+      if (flush_file) {
+        if (the_trace->trace_file_->Flush() != 0) {
+          PLOG(WARNING) << "Could not flush trace file.";
+        }
       } else {
-        runtime->GetInstrumentation()->DisableMethodTracing(kTracerInstrumentationKey);
-        runtime->GetInstrumentation()->RemoveListener(
-            the_trace, instrumentation::Instrumentation::kMethodEntered |
-            instrumentation::Instrumentation::kMethodExited |
-            instrumentation::Instrumentation::kMethodUnwind);
+        the_trace->trace_file_->MarkUnchecked();  // Do not trigger guard.
       }
-      if (the_trace->trace_file_.get() != nullptr) {
-        // Do not try to erase, so flush and close explicitly.
-        if (flush_file) {
-          if (the_trace->trace_file_->Flush() != 0) {
-            PLOG(WARNING) << "Could not flush trace file.";
-          }
-        } else {
-          the_trace->trace_file_->MarkUnchecked();  // Do not trigger guard.
-        }
-        if (the_trace->trace_file_->Close() != 0) {
-          PLOG(ERROR) << "Could not close trace file.";
-        }
+      if (the_trace->trace_file_->Close() != 0) {
+        PLOG(ERROR) << "Could not close trace file.";
       }
-      delete the_trace;
     }
+    delete the_trace;
   }
   if (stop_alloc_counting) {
     // Can be racy since SetStatsEnabled is not guarded by any locks.
@@ -717,12 +715,12 @@ void Trace::FinishTracing() {
     FlushBuf();
   } else {
     if (trace_file_.get() == nullptr) {
-      iovec iov[2];
-      iov[0].iov_base = reinterpret_cast<void*>(const_cast<char*>(header.c_str()));
-      iov[0].iov_len = header.length();
-      iov[1].iov_base = buf_.get();
-      iov[1].iov_len = final_offset;
-      Dbg::DdmSendChunkV(CHUNK_TYPE("MPSE"), iov, 2);
+      std::vector<uint8_t> data;
+      data.resize(header.length() + final_offset);
+      memcpy(data.data(), header.c_str(), header.length());
+      memcpy(data.data() + header.length(), buf_.get(), final_offset);
+      Runtime::Current()->GetRuntimeCallbacks()->DdmPublishChunk(CHUNK_TYPE("MPSE"),
+                                                                 ArrayRef<const uint8_t>(data));
       const bool kDumpTraceInfo = false;
       if (kDumpTraceInfo) {
         LOG(INFO) << "Trace sent:\n" << header;
diff --git a/test/1940-ddms-ext/expected.txt b/test/1940-ddms-ext/expected.txt
index cf4ad50e90..62d3b7bd4c 100644
--- a/test/1940-ddms-ext/expected.txt
+++ b/test/1940-ddms-ext/expected.txt
@@ -5,3 +5,6 @@ MyDdmHandler: Chunk returned: Chunk(Type: 0xFADE7357, Len: 8, data: [0, 0, 0, 0,
 JVMTI returned chunk: Chunk(Type: 0xFADE7357, Len: 8, data: [0, 0, 0, 0, 0, -128, 0, 37])
 Sending chunk: Chunk(Type: 0xDEADBEEF, Len: 8, data: [9, 10, 11, 12, 13, 14, 15, 16])
 Chunk published: Chunk(Type: 0xDEADBEEF, Len: 8, data: [9, 10, 11, 12, 13, 14, 15, 16])
+Saw expected thread events.
+Expected chunk type published: 1213221190
+Expected chunk type published: 1297109829
diff --git a/test/1940-ddms-ext/src-art/art/Test1940.java b/test/1940-ddms-ext/src-art/art/Test1940.java
index f0ee7102a9..9f79eaebba 100644
--- a/test/1940-ddms-ext/src-art/art/Test1940.java
+++ b/test/1940-ddms-ext/src-art/art/Test1940.java
@@ -17,6 +17,7 @@
 package art;
 
 import org.apache.harmony.dalvik.ddmc.*;
+import dalvik.system.VMDebug;
 
 import java.lang.reflect.Method;
 import java.util.Arrays;
@@ -30,6 +31,12 @@ public class Test1940 {
   public static final int MY_DDMS_TYPE = 0xDEADBEEF;
   public static final int MY_DDMS_RESPONSE_TYPE = 0xFADE7357;
 
+  public static final boolean PRINT_ALL_CHUNKS = false;
+
+  public static interface DdmHandler {
+    public void HandleChunk(int type, byte[] data);
+  }
+
   public static final class TestError extends Error {
     public TestError(String s) { super(s); }
   }
@@ -69,11 +76,38 @@ public class Test1940 {
 
   public static final ChunkHandler SINGLE_HANDLER = new MyDdmHandler();
 
+  public static DdmHandler CURRENT_HANDLER;
+
   public static void HandlePublish(int type, byte[] data) {
-    System.out.println("Chunk published: " + printChunk(new Chunk(type, data, 0, data.length)));
+    if (PRINT_ALL_CHUNKS) {
+      System.out.println(
+          "Unknown Chunk published: " + printChunk(new Chunk(type, data, 0, data.length)));
+    }
+    CURRENT_HANDLER.HandleChunk(type, data);
+  }
+
+  // TYPE Thread Create
+  public static final int TYPE_THCR = 0x54484352;
+  // Type Thread name
+  public static final int TYPE_THNM = 0x54484E4D;
+  // Type Thread death.
+  public static final int TYPE_THDE = 0x54484445;
+  // Type Heap info
+  public static final int TYPE_HPIF = 0x48504946;
+  // Type Trace Results
+  public static final int TYPE_MPSE = 0x4D505345;
+
+  public static boolean IsFromThread(Thread t, byte[] data) {
+    // DDMS always puts the thread-id as the first 4 bytes.
+    ByteBuffer b = ByteBuffer.wrap(data);
+    b.order(ByteOrder.BIG_ENDIAN);
+    return b.getInt() == (int) t.getId();
   }
 
   public static void run() throws Exception {
+    CURRENT_HANDLER = (type, data) -> {
+      System.out.println("Chunk published: " + printChunk(new Chunk(type, data, 0, data.length)));
+    };
     initializeTest(
         Test1940.class,
         Test1940.class.getDeclaredMethod("HandlePublish", Integer.TYPE, new byte[0].getClass()));
@@ -90,8 +124,70 @@ public class Test1940 {
         MY_DDMS_TYPE, new byte[] { 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10 }, 0, 8);
     System.out.println("Sending chunk: " + printChunk(c));
     DdmServer.sendChunk(c);
+
+    // Test thread chunks are sent.
+    final boolean[] types_seen = new boolean[] { false, false, false };
+    CURRENT_HANDLER = (type, cdata) -> {
+      switch (type) {
+        case TYPE_THCR:
+          types_seen[0] = true;
+          break;
+        case TYPE_THNM:
+          types_seen[1] = true;
+          break;
+        case TYPE_THDE:
+          types_seen[2] = true;
+          break;
+        default:
+          // We don't want to print other types.
+          break;
+      }
+    };
+    DdmVmInternal.threadNotify(true);
+    final Thread thr = new Thread(() -> { return; }, "THREAD");
+    thr.start();
+    thr.join();
+    DdmVmInternal.threadNotify(false);
+    // Make sure we saw at least one of Thread-create, Thread name, & thread death.
+    if (!types_seen[0] || !types_seen[1] || !types_seen[2]) {
+      System.out.println("Didn't see expected chunks for thread creation! got: " +
+          Arrays.toString(types_seen));
+    } else {
+      System.out.println("Saw expected thread events.");
+    }
+
+    // Test heap chunks are sent.
+    CURRENT_HANDLER = (type, cdata) -> {
+      // The actual data is far to noisy for this test as it includes information about global heap
+      // state.
+      if (type == TYPE_HPIF) {
+        System.out.println("Expected chunk type published: " + type);
+      }
+    };
+    final int HPIF_WHEN_NOW = 1;
+    if (!DdmVmInternal.heapInfoNotify(HPIF_WHEN_NOW)) {
+      System.out.println("Unexpected failure for heapInfoNotify!");
+    }
+
+    // method Tracing
+    CURRENT_HANDLER = (type, cdata) -> {
+      // This chunk includes timing and thread information so we just check the type.
+      if (type == TYPE_MPSE) {
+        System.out.println("Expected chunk type published: " + type);
+      }
+    };
+    VMDebug.startMethodTracingDdms(/*size: default*/0,
+                                   /*flags: none*/ 0,
+                                   /*sampling*/ false,
+                                   /*interval*/ 0);
+    doNothing();
+    doNothing();
+    doNothing();
+    doNothing();
+    VMDebug.stopMethodTracing();
   }
 
+  private static void doNothing() {}
   private static Chunk processChunk(byte[] val) {
     return processChunk(new Chunk(MY_DDMS_TYPE, val, 0, val.length));
   }
diff --git a/test/knownfailures.json b/test/knownfailures.json
index c6b6574f1b..5b2ebf58a4 100644
--- a/test/knownfailures.json
+++ b/test/knownfailures.json
@@ -202,6 +202,12 @@
         "bug": "http://b/34369284"
     },
     {
+        "tests": "1940-ddms-ext",
+        "description": ["Test expects to be able to start tracing but we cannot",
+                        "do that if tracing is already ongoing."],
+        "variant": "trace | stream"
+    },
+    {
         "tests": "137-cfi",
         "description": ["This test unrolls and expects managed frames, but",
                         "tracing means we run the interpreter."],