Do not call SendMessageToNativeLog on Android in AudioInputSyncWriter

content/browser/renderer_host/media/audio_input_sync_writer.cc

Index: content/browser/renderer_host/media/audio_input_sync_writer.cc
diff --git a/content/browser/renderer_host/media/audio_input_sync_writer.cc b/content/browser/renderer_host/media/audio_input_sync_writer.cc
index b9c8ee5e3b6d19c3a84acfbd1b695872c2baf4c1..c972bfaf513b74d65a568cce831c50d1f7df0d92 100644
--- a/content/browser/renderer_host/media/audio_input_sync_writer.cc
+++ b/content/browser/renderer_host/media/audio_input_sync_writer.cc
@@ -50,9 +50,19 @@ uint32 AudioInputSyncWriter::Write(const void* data,
          << interval.InMilliseconds() << "ms.";
     }
   }
-  if (!oss.str().empty())
+  if (!oss.str().empty()) {
     MediaStreamManager::SendMessageToNativeLog(oss.str());

[Feng Qian, 2014/05/19 23:27:01]

I felt it is safer to disabling logging instead of calling DetachFromVM because
we are unsure the performance implication.

[jiayl, 2014/05/19 23:30:20]

This is only called for the first audio sample data, or when there is
unexpectedly long delay. So for most of the times, it should be called once
only.
Should that make performance less a concern?

[Feng Qian, 2014/05/20 15:53:12]

thanks for explanation, sounds fine to me.

On 2014/05/19 23:30:20, jiayl wrote:

 
+    // MediaStreamManager::SendMessageToNativeLog posts a task to the UI thread,

[Feng Qian, 2014/05/20 15:53:12]

Should comments be inside #if defined(OS_ANDROID)?

+    // which will attach the audio thread to the Android java VM. Unlike chrome
+    // created threads, the audio thread is owned by the OS and does not detach
+    // itself from the VM on exit, causing a crash (crbug/365915). So we detach
+    // here to make sure the thread exits clean.
+#if defined(OS_ANDROID)
+    base::android::DetachFromVM();

[tommi (sloooow) - chröme, 2014/05/20 12:28:29]

Just so that I understand - from information in the bug, the problem occurs
while calling SendMessageToNativeLog().  So, since you're adding this call
_after_ SendMessageToNativeLog, does that mean that in the bug report, we were
seeing a second call to SendMessageToNativeLog?

If so, then that should be a very rare exception case according to jiayl.  If
not, then shouldn't this be done before the call to SendMessageToNativeLog?

[Feng Qian, 2014/05/20 15:53:12]

tommi@, not sure if I understand your question correctly. SendMessageToNativeLog
is called once.

AttachCurrentThread can be called multiple times, in this case, each
SendMessageToNativeLog calls it once.

DetachFromVM only needs to be called once before exiting thread. Performance
doesn't seem like an issue here.

On 2014/05/20 12:28:29, tommi wrote:

[jiayl, 2014/05/20 15:56:14]

The crash happens when the thread exists, not while calling
SendMessageToNativeLog.

SendMessageToNativeLog caused the thread attached to the VM and never detached,
which caused a failure of 'threadExitCheck' in dalvik.

+#endif
+  }
+
   last_write_time_ = base::Time::Now();
 
   uint8* ptr = static_cast<uint8*>(shared_memory_->memory());