Android: Fix default filename for kTraceStartup file

Putting chrometrace.log in the current working directory is not
useful on Android. Generate a filepath on the sdcard instead the
same way we do it for intent triggered tracing.

R=piman@chromium.org, wangxianzhu@chromium.org

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

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

5 files changed, 65 insertions, 37 deletions

diff --git a/build/android/adb_profile_chrome.py b/build/android/adb_profile_chrome.py
index ae69cbe..37f250c 100755
--- a/build/android/adb_profile_chrome.py
+++ b/build/android/adb_profile_chrome.py
@@ -45,7 +45,7 @@ class ChromeTracingController(object):
     self._trace_file = None
     self._trace_interval = None
     self._trace_start_re = \
-       re.compile(r'Logging performance trace to file: (.*)')
+       re.compile(r'Logging performance trace to file')
     self._trace_finish_re = \
        re.compile(r'Profiler finished[.] Results are in (.*)[.]')
     self._device.old_interface.StartMonitoringLogcat(clear=False)
@@ -62,25 +62,24 @@ class ChromeTracingController(object):
         '-e continuous' if self._ring_buffer else '')
     # Chrome logs two different messages related to tracing:
     #
-    # 1. "Logging performance trace to file [...]"
+    # 1. "Logging performance trace to file"
     # 2. "Profiler finished. Results are in [...]"
     #
     # The first one is printed when tracing starts and the second one indicates
     # that the trace file is ready to be pulled.
     try:
-      self._trace_file = self._device.old_interface.WaitForLogMatch(
-          self._trace_start_re, None, timeout=5).group(1)
+      self._device.old_interface.WaitForLogMatch(
+          self._trace_start_re, None, timeout=5)
     except pexpect.TIMEOUT:
       raise RuntimeError('Trace start marker not found. Is the correct version '
                          'of the browser running?')
 
   def StopTracing(self):
-    if not self._trace_file:
-      return
-    self._device.old_interface.BroadcastIntent(self._package_info.package,
-                                               'GPU_PROFILER_STOP')
-    self._device.old_interface.WaitForLogMatch(self._trace_finish_re, None,
-                                               timeout=120)
+    self._device.old_interface.BroadcastIntent(
+        self._package_info.package,
+        'GPU_PROFILER_STOP')
+    self._trace_file = self._device.old_interface.WaitForLogMatch(
+        self._trace_finish_re, None, timeout=120).group(1)
 
   def PullTrace(self):
     # Wait a bit for the browser to finish writing the trace file.
diff --git a/content/browser/android/tracing_controller_android.cc b/content/browser/android/tracing_controller_android.cc
index 75ac6d7..3c3ee75 100644
--- a/content/browser/android/tracing_controller_android.cc
+++ b/content/browser/android/tracing_controller_android.cc
@@ -30,16 +30,13 @@ void TracingControllerAndroid::Destroy(JNIEnv* env, jobject obj) {
 
 bool TracingControllerAndroid::StartTracing(JNIEnv* env,
                                             jobject obj,
-                                            jstring jfilename,
                                             jstring jcategories,
                                             jboolean record_continuously) {
-  file_path_ = base::FilePath(
-      base::android::ConvertJavaStringToUTF8(env, jfilename));
   std::string categories =
       base::android::ConvertJavaStringToUTF8(env, jcategories);
 
   // This log is required by adb_profile_chrome.py.
-  LOG(WARNING) << "Logging performance trace to file: " << file_path_.value();
+  LOG(WARNING) << "Logging performance trace to file";
 
   return TracingController::GetInstance()->EnableRecording(
       categories,
@@ -48,16 +45,29 @@ bool TracingControllerAndroid::StartTracing(JNIEnv* env,
       TracingController::EnableRecordingDoneCallback());
 }
 
-void TracingControllerAndroid::StopTracing(JNIEnv* env, jobject obj) {
+void TracingControllerAndroid::StopTracing(JNIEnv* env,
+                                           jobject obj,
+                                           jstring jfilepath) {
+  base::FilePath file_path(
+      base::android::ConvertJavaStringToUTF8(env, jfilepath));
   if (!TracingController::GetInstance()->DisableRecording(
-      file_path_,
+      file_path,
       base::Bind(&TracingControllerAndroid::OnTracingStopped,
                  weak_factory_.GetWeakPtr()))) {
     LOG(ERROR) << "EndTracingAsync failed, forcing an immediate stop";
-    OnTracingStopped(file_path_);
+    OnTracingStopped(file_path);
   }
 }
 
+void TracingControllerAndroid::GenerateTracingFilePath(
+    base::FilePath* file_path) {
+  JNIEnv* env = base::android::AttachCurrentThread();
+  ScopedJavaLocalRef<jstring> jfilename =
+      Java_TracingControllerAndroid_generateTracingFilePath(env);
+  *file_path = base::FilePath(
+      base::android::ConvertJavaStringToUTF8(env, jfilename.obj()));
+}
+
 void TracingControllerAndroid::OnTracingStopped(
     const base::FilePath& file_path) {
   JNIEnv* env = base::android::AttachCurrentThread();
diff --git a/content/browser/android/tracing_controller_android.h b/content/browser/android/tracing_controller_android.h
index 4d70e7b..cf56259 100644
--- a/content/browser/android/tracing_controller_android.h
+++ b/content/browser/android/tracing_controller_android.h
@@ -19,17 +19,16 @@ class TracingControllerAndroid {
 
   bool StartTracing(JNIEnv* env,
                     jobject obj,
-                    jstring filename,
                     jstring categories,
                     jboolean record_continuously);
-  void StopTracing(JNIEnv* env, jobject obj);
+  void StopTracing(JNIEnv* env, jobject obj, jstring jfilepath);
+  static void GenerateTracingFilePath(base::FilePath* file_path);
 
  private:
   ~TracingControllerAndroid();
   void OnTracingStopped(const base::FilePath& file_path);
 
   JavaObjectWeakGlobalRef weak_java_object_;
-  base::FilePath file_path_;
   base::WeakPtrFactory<TracingControllerAndroid> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(TracingControllerAndroid);
diff --git a/content/browser/browser_main_loop.cc b/content/browser/browser_main_loop.cc
index 2a6b823..5a7c5b9 100644
--- a/content/browser/browser_main_loop.cc
+++ b/content/browser/browser_main_loop.cc
@@ -70,6 +70,7 @@
 #include "base/android/jni_android.h"
 #include "content/browser/android/browser_startup_controller.h"
 #include "content/browser/android/surface_texture_peer_browser_impl.h"
+#include "content/browser/android/tracing_controller_android.h"
 #include "ui/gl/gl_surface.h"
 #endif
 
@@ -253,6 +254,10 @@ static void SetUpGLibLogHandler() {
 }
 #endif
 
+void OnStoppedStartupTracing(const base::FilePath& trace_file) {
+  LOG(INFO) << "Completed startup tracing to " << trace_file.value();
+}
+
 }  // namespace
 
 // The currently-running BrowserMainLoop.  There can be one or zero.
@@ -1095,8 +1100,12 @@ void BrowserMainLoop::InitStartupTracing(const CommandLine& command_line) {
     return;
 
   if (trace_file.empty()) {
+#if defined(OS_ANDROID)
+    TracingControllerAndroid::GenerateTracingFilePath(&trace_file);
+#else
     // Default to saving the startup trace into the current dir.
     trace_file = base::FilePath().AppendASCII("chrometrace.log");
+#endif
   }
 
   std::string delay_str = command_line.GetSwitchValueASCII(
@@ -1118,7 +1127,7 @@ void BrowserMainLoop::InitStartupTracing(const CommandLine& command_line) {
 void BrowserMainLoop::EndStartupTracing(const base::FilePath& trace_file) {
   is_tracing_startup_ = false;
   TracingController::GetInstance()->DisableRecording(
-      trace_file, TracingController::TracingFileResultCallback());
+      trace_file, base::Bind(&OnStoppedStartupTracing));
 }
 
 }  // namespace content
diff --git a/content/public/android/java/src/org/chromium/content/browser/TracingControllerAndroid.java b/content/public/android/java/src/org/chromium/content/browser/TracingControllerAndroid.java
index e405922..dbd8aa8 100644
--- a/content/public/android/java/src/org/chromium/content/browser/TracingControllerAndroid.java
+++ b/content/public/android/java/src/org/chromium/content/browser/TracingControllerAndroid.java
@@ -114,19 +114,13 @@ public class TracingControllerAndroid {
     }
 
     /**
-     * Start profiling to a new file in the Downloads directory.
-     *
-     * Calls #startTracing(String, boolean, String, boolean) with a new timestamped filename.
-     * @see #startTracing(String, boolean, String, boolean)
+     * Generates a unique filename to be used for tracing in the Downloads directory.
      */
-    public boolean startTracing(boolean showToasts, String categories,
-            boolean recordContinuously) {
-        mShowToasts = showToasts;
+    @CalledByNative
+    private static String generateTracingFilePath() {
         String state = Environment.getExternalStorageState();
         if (!Environment.MEDIA_MOUNTED.equals(state)) {
-            logAndToastError(
-                    mContext.getString(R.string.profiler_no_storage_toast));
-            return false;
+            return null;
         }
 
         // Generate a hopefully-unique filename using the UTC timestamp.
@@ -138,8 +132,25 @@ public class TracingControllerAndroid {
                 Environment.DIRECTORY_DOWNLOADS);
         File file = new File(
                 dir, "chrome-profile-results-" + formatter.format(new Date()));
+        return file.getPath();
+    }
 
-        return startTracing(file.getPath(), showToasts, categories, recordContinuously);
+    /**
+     * Start profiling to a new file in the Downloads directory.
+     *
+     * Calls #startTracing(String, boolean, String, boolean) with a new timestamped filename.
+     * @see #startTracing(String, boolean, String, boolean)
+     */
+    public boolean startTracing(boolean showToasts, String categories,
+            boolean recordContinuously) {
+        mShowToasts = showToasts;
+
+        String filePath = generateTracingFilePath();
+        if (filePath == null) {
+          logAndToastError(
+              mContext.getString(R.string.profiler_no_storage_toast));
+        }
+        return startTracing(filePath, showToasts, categories, recordContinuously);
     }
 
     /**
@@ -170,7 +181,7 @@ public class TracingControllerAndroid {
         if (mNativeTracingControllerAndroid == 0) {
             mNativeTracingControllerAndroid = nativeInit();
         }
-        if (!nativeStartTracing(mNativeTracingControllerAndroid, filename, categories,
+        if (!nativeStartTracing(mNativeTracingControllerAndroid, categories,
                 recordContinuously)) {
             logAndToastError(mContext.getString(R.string.profiler_error_toast));
             return false;
@@ -188,7 +199,7 @@ public class TracingControllerAndroid {
      */
     public void stopTracing() {
         if (isTracing()) {
-            nativeStopTracing(mNativeTracingControllerAndroid);
+            nativeStopTracing(mNativeTracingControllerAndroid, mFilename);
         }
     }
 
@@ -265,8 +276,8 @@ public class TracingControllerAndroid {
     private long mNativeTracingControllerAndroid;
     private native long nativeInit();
     private native void nativeDestroy(long nativeTracingControllerAndroid);
-    private native boolean nativeStartTracing(long nativeTracingControllerAndroid, String filename,
-            String categories, boolean recordContinuously);
-    private native void nativeStopTracing(long nativeTracingControllerAndroid);
+    private native boolean nativeStartTracing(
+            long nativeTracingControllerAndroid, String categories, boolean recordContinuously);
+    private native void nativeStopTracing(long nativeTracingControllerAndroid, String filename);
     private native String nativeGetDefaultCategories();
 }