From bb47805b98fa53be7fcb5124e6daf9390c1f14f6 Mon Sep 17 00:00:00 2001 From: Carmen Jackson Date: Tue, 21 May 2019 20:24:48 -0700 Subject: Unregister the SharedPreferenceListener when the MainFragment pauses. This is now getting triggered when the MainFragment isn't visible on the screen, which results in a NPE when we try to getContext from this listener. Also, switch to using onStop/Start instead of onPause/Resume because that should be a little more appropriate. Bug: 133259473 Test: atest TraceurUiTests passes 10 times in a row. Manual testing. Merged-In: I18fe4b5c788b4a43bf1a16bf6b6d3cbae78df590 Change-Id: I18fe4b5c788b4a43bf1a16bf6b6d3cbae78df590 --- src/com/google/android/traceur/MainFragment.java | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/com/google/android/traceur/MainFragment.java b/src/com/google/android/traceur/MainFragment.java index 5db3577..a3cd01f 100644 --- a/src/com/google/android/traceur/MainFragment.java +++ b/src/com/google/android/traceur/MainFragment.java @@ -181,9 +181,6 @@ public class MainFragment extends PreferenceFragment { } }); - getPreferenceScreen().getSharedPreferences() - .registerOnSharedPreferenceChangeListener(mSharedPreferenceChangeListener); - refreshUi(); mRefreshReceiver = new BroadcastReceiver() { @@ -202,14 +199,18 @@ public class MainFragment extends PreferenceFragment { } @Override - public void onResume() { - super.onResume(); + public void onStart() { + super.onStart(); + getPreferenceScreen().getSharedPreferences() + .registerOnSharedPreferenceChangeListener(mSharedPreferenceChangeListener); getActivity().registerReceiver(mRefreshReceiver, new IntentFilter(ACTION_REFRESH_TAGS)); Receiver.updateTracing(getContext()); } @Override - public void onPause() { + public void onStop() { + getPreferenceScreen().getSharedPreferences() + .unregisterOnSharedPreferenceChangeListener(mSharedPreferenceChangeListener); getActivity().unregisterReceiver(mRefreshReceiver); if (mAlertDialog != null) { @@ -217,7 +218,7 @@ public class MainFragment extends PreferenceFragment { mAlertDialog = null; } - super.onPause(); + super.onStop(); } @Override -- cgit v1.2.3 From 7b47868e06a9b22cd1e8044f99de0ba6c1468cd5 Mon Sep 17 00:00:00 2001 From: Carmen Jackson Date: Mon, 20 May 2019 23:12:03 -0700 Subject: Use commit, not apply, when updating tracing state in sharedpreferences. Apply commits changes to disk asynchronously, which means that if we subsequently query the tracing state from sharedpreferences quickly we could experience a race condition. Bug: 124064006 Test: atest TraceurUiTests completes successfully, and manual tests also work. Note this bug was not consistently reproducible. Merged-In: I1047880e673b95f5c95b080ffb36805099c6be19 Change-Id: I1047880e673b95f5c95b080ffb36805099c6be19 --- src/com/google/android/traceur/QsService.java | 2 +- src/com/google/android/traceur/Receiver.java | 4 ++-- src/com/google/android/traceur/StopTraceService.java | 2 +- src/com/google/android/traceur/TraceService.java | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/com/google/android/traceur/QsService.java b/src/com/google/android/traceur/QsService.java index 852840b..0ddbf36 100644 --- a/src/com/google/android/traceur/QsService.java +++ b/src/com/google/android/traceur/QsService.java @@ -65,7 +65,7 @@ public class QsService extends TileService { public void onClick() { SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(this); boolean newTracingState = !prefs.getBoolean(getString(R.string.pref_key_tracing_on), false); - prefs.edit().putBoolean(getString(R.string.pref_key_tracing_on), newTracingState).apply(); + prefs.edit().putBoolean(getString(R.string.pref_key_tracing_on), newTracingState).commit(); Receiver.updateTracing(this); } diff --git a/src/com/google/android/traceur/Receiver.java b/src/com/google/android/traceur/Receiver.java index 987d775..3a207fc 100644 --- a/src/com/google/android/traceur/Receiver.java +++ b/src/com/google/android/traceur/Receiver.java @@ -76,7 +76,7 @@ public class Receiver extends BroadcastReceiver { updateDeveloperOptionsWatcher(context); updateTracing(context); } else if (STOP_ACTION.equals(intent.getAction())) { - prefs.edit().putBoolean(context.getString(R.string.pref_key_tracing_on), false).apply(); + prefs.edit().putBoolean(context.getString(R.string.pref_key_tracing_on), false).commit(); updateTracing(context); } else if (OPEN_ACTION.equals(intent.getAction())) { context.sendBroadcast(new Intent(Intent.ACTION_CLOSE_SYSTEM_DIALOGS)); @@ -198,7 +198,7 @@ public class Receiver extends BroadcastReceiver { PreferenceManager.getDefaultSharedPreferences(context); prefs.edit().putBoolean( context.getString(R.string.pref_key_quick_setting), false) - .apply(); + .commit(); updateQuickSettings(context); } } diff --git a/src/com/google/android/traceur/StopTraceService.java b/src/com/google/android/traceur/StopTraceService.java index c9df791..fcdb470 100644 --- a/src/com/google/android/traceur/StopTraceService.java +++ b/src/com/google/android/traceur/StopTraceService.java @@ -51,7 +51,7 @@ public class StopTraceService extends TraceService { PreferenceManager.getDefaultSharedPreferences(context) .edit().putBoolean(context.getString(R.string.pref_key_tracing_on), - false).apply(); + false).commit(); context.sendBroadcast(new Intent(MainFragment.ACTION_REFRESH_TAGS)); QsService.updateTile(); diff --git a/src/com/google/android/traceur/TraceService.java b/src/com/google/android/traceur/TraceService.java index a028645..f0716a7 100644 --- a/src/com/google/android/traceur/TraceService.java +++ b/src/com/google/android/traceur/TraceService.java @@ -138,7 +138,7 @@ public class TraceService extends IntentService { TraceUtils.traceStop(); PreferenceManager.getDefaultSharedPreferences(context) .edit().putBoolean(context.getString(R.string.pref_key_tracing_on), - false).apply(); + false).commit(); QsService.updateTile(); stopForeground(Service.STOP_FOREGROUND_REMOVE); } -- cgit v1.2.3 From a6f820bba1e7624cec3aa702bbe42224655997a8 Mon Sep 17 00:00:00 2001 From: Carmen Jackson Date: Tue, 21 May 2019 23:15:40 -0700 Subject: Add a second notification channel. This allows users to minimize the "Trace is being recorded" notification, so that it's more subtle and less annoying to have around for long periods of time. Because it's not ideal to let people swipe away that notification completely lest they forget they're tracing, this is hopefully a good compromise. The notification channel named "Trace is being recorded" applies to that notification, and the one named "Saving trace" applies to all of the other notifications. That's because "Saving trace" is the most important of the other group, and I don't currently have a better string available. Bug: 132918079 Test: Manual, changing the notifications and ensuring that they still appeared. Merged-In: Ibb1ad5fa4e54fef35a607efae75012ffdbf99eb2 Change-Id: Ibb1ad5fa4e54fef35a607efae75012ffdbf99eb2 --- src/com/google/android/traceur/FileSender.java | 2 +- src/com/google/android/traceur/Receiver.java | 31 ++++++++++++++++-------- src/com/google/android/traceur/TraceService.java | 4 +-- 3 files changed, 24 insertions(+), 13 deletions(-) diff --git a/src/com/google/android/traceur/FileSender.java b/src/com/google/android/traceur/FileSender.java index a824f9d..b069521 100644 --- a/src/com/google/android/traceur/FileSender.java +++ b/src/com/google/android/traceur/FileSender.java @@ -56,7 +56,7 @@ public class FileSender { intent.putExtra(Intent.EXTRA_INTENT, sendIntent); final Notification.Builder builder = - new Notification.Builder(context, Receiver.NOTIFICATION_CHANNEL) + new Notification.Builder(context, Receiver.NOTIFICATION_CHANNEL_OTHER) .setSmallIcon(R.drawable.stat_sys_adb) .setContentTitle(context.getString(R.string.trace_saved)) .setTicker(context.getString(R.string.trace_saved)) diff --git a/src/com/google/android/traceur/Receiver.java b/src/com/google/android/traceur/Receiver.java index 3a207fc..26a504e 100644 --- a/src/com/google/android/traceur/Receiver.java +++ b/src/com/google/android/traceur/Receiver.java @@ -49,7 +49,8 @@ public class Receiver extends BroadcastReceiver { public static final String STOP_ACTION = "com.android.traceur.STOP"; public static final String OPEN_ACTION = "com.android.traceur.OPEN"; - public static final String NOTIFICATION_CHANNEL = "system-tracing"; + public static final String NOTIFICATION_CHANNEL_TRACING = "trace-is-being-recorded"; + public static final String NOTIFICATION_CHANNEL_OTHER = "system-tracing"; private static final List TRACE_TAGS = Arrays.asList( "am", "binder_driver", "camera", "dalvik", "freq", "gfx", "hal", @@ -72,7 +73,7 @@ public class Receiver extends BroadcastReceiver { SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(context); if (Intent.ACTION_BOOT_COMPLETED.equals(intent.getAction())) { - createNotificationChannel(context); + createNotificationChannels(context); updateDeveloperOptionsWatcher(context); updateTracing(context); } else if (STOP_ACTION.equals(intent.getAction())) { @@ -215,7 +216,7 @@ public class Receiver extends BroadcastReceiver { String title = context.getString(R.string.tracing_categories_unavailable); String msg = TextUtils.join(", ", getActiveUnavailableTags(context, prefs)); final Notification.Builder builder = - new Notification.Builder(context, NOTIFICATION_CHANNEL) + new Notification.Builder(context, NOTIFICATION_CHANNEL_OTHER) .setSmallIcon(R.drawable.stat_sys_adb) .setContentTitle(title) .setTicker(title) @@ -236,17 +237,27 @@ public class Receiver extends BroadcastReceiver { .notify(Receiver.class.getName(), 0, builder.build()); } - private static void createNotificationChannel(Context context) { - NotificationChannel channel = new NotificationChannel( - NOTIFICATION_CHANNEL, context.getString(R.string.system_tracing), + private static void createNotificationChannels(Context context) { + NotificationChannel tracingChannel = new NotificationChannel( + NOTIFICATION_CHANNEL_TRACING, + context.getString(R.string.trace_is_being_recorded), NotificationManager.IMPORTANCE_HIGH); - channel.setBypassDnd(true); - channel.enableVibration(true); - channel.setSound(null, null); + tracingChannel.setBypassDnd(true); + tracingChannel.enableVibration(true); + tracingChannel.setSound(null, null); + + NotificationChannel saveTraceChannel = new NotificationChannel( + NOTIFICATION_CHANNEL_OTHER, + context.getString(R.string.saving_trace), + NotificationManager.IMPORTANCE_HIGH); + saveTraceChannel.setBypassDnd(true); + saveTraceChannel.enableVibration(true); + saveTraceChannel.setSound(null, null); NotificationManager notificationManager = context.getSystemService(NotificationManager.class); - notificationManager.createNotificationChannel(channel); + notificationManager.createNotificationChannel(tracingChannel); + notificationManager.createNotificationChannel(saveTraceChannel); } public static Set getActiveTags(Context context, SharedPreferences prefs, boolean onlyAvailable) { diff --git a/src/com/google/android/traceur/TraceService.java b/src/com/google/android/traceur/TraceService.java index f0716a7..0a242cb 100644 --- a/src/com/google/android/traceur/TraceService.java +++ b/src/com/google/android/traceur/TraceService.java @@ -111,7 +111,7 @@ public class TraceService extends IntentService { String msg = context.getString(R.string.tap_to_stop_tracing); Notification.Builder notification = - new Notification.Builder(context, Receiver.NOTIFICATION_CHANNEL) + new Notification.Builder(context, Receiver.NOTIFICATION_CHANNEL_TRACING) .setSmallIcon(R.drawable.stat_sys_adb) .setContentTitle(title) .setTicker(title) @@ -150,7 +150,7 @@ public class TraceService extends IntentService { getSystemService(NotificationManager.class); Notification.Builder notification = - new Notification.Builder(this, Receiver.NOTIFICATION_CHANNEL) + new Notification.Builder(this, Receiver.NOTIFICATION_CHANNEL_OTHER) .setSmallIcon(R.drawable.stat_sys_adb) .setContentTitle(getString(R.string.saving_trace)) .setTicker(getString(R.string.saving_trace)) -- cgit v1.2.3 From f38ca784fdd0cd957f324cf0d8f26477dbfe8d8d Mon Sep 17 00:00:00 2001 From: Carmen Jackson Date: Wed, 15 May 2019 17:39:37 -0700 Subject: Remove the atrace/Perfetto toggle, so Traceur only uses Perfetto. Also, update the tests to reference all of the perfetto-only views. Bug: 122892432 Bug: 132811730 Test: No longer see Perfetto toggle in the UI, but starting a trace starts a Perfetto trace. Test: atest TraceurUITests Test: If the user was previously running an atrace trace, after flashing a build with this change a Perfetto trace will start seamlessly. Test: Rebuilding with atrace as the trace backend is manually tested to work as expected. Merged-In: I182ebf8701efdbe224c90d500dbb1b7b0ce3a25c Change-Id: I182ebf8701efdbe224c90d500dbb1b7b0ce3a25c --- res/values/preference_keys.xml | 1 - res/xml/main.xml | 6 +-- src/com/google/android/traceur/MainFragment.java | 43 ++++++++-------------- .../google/android/traceur/StopTraceService.java | 1 - src/com/google/android/traceur/TraceService.java | 9 ----- src/com/google/android/traceur/TraceUtils.java | 24 ++---------- .../com/android/settings/ui/TraceurAppTests.java | 9 +++++ 7 files changed, 29 insertions(+), 64 deletions(-) diff --git a/res/values/preference_keys.xml b/res/values/preference_keys.xml index b23f3b9..a3daf86 100644 --- a/res/values/preference_keys.xml +++ b/res/values/preference_keys.xml @@ -9,5 +9,4 @@ max_long_trace_size max_long_trace_duration quick_setting_enabled - use_perfetto_2 diff --git a/res/xml/main.xml b/res/xml/main.xml index 0d47435..685e889 100644 --- a/res/xml/main.xml +++ b/res/xml/main.xml @@ -41,12 +41,12 @@ android:persistent="false" android:title="@string/clear_saved_traces" /> - diff --git a/src/com/google/android/traceur/MainFragment.java b/src/com/google/android/traceur/MainFragment.java index a3cd01f..e69b2b3 100644 --- a/src/com/google/android/traceur/MainFragment.java +++ b/src/com/google/android/traceur/MainFragment.java @@ -67,8 +67,6 @@ public class MainFragment extends PreferenceFragment { private MultiSelectListPreference mTags; - private SwitchPreference mUsePerfetto; - private boolean mRefreshing; private BroadcastReceiver mRefreshReceiver; @@ -166,21 +164,6 @@ public class MainFragment extends PreferenceFragment { } }); - mUsePerfetto = (SwitchPreference) findPreference(getActivity().getString(R.string.pref_key_use_perfetto)); - mUsePerfetto.setOnPreferenceChangeListener(new Preference.OnPreferenceChangeListener() { - @Override - public boolean onPreferenceChange(Preference preference, Object newValue) { - boolean shouldUsePerfetto = (boolean)newValue; - boolean success = TraceUtils.switchTraceEngine( - shouldUsePerfetto ? PerfettoUtils.NAME : AtraceUtils.NAME); - - if (success) { - mUsePerfetto.setChecked(shouldUsePerfetto); - } - return false; - } - }); - refreshUi(); mRefreshReceiver = new BroadcastReceiver() { @@ -247,9 +230,6 @@ public class MainFragment extends PreferenceFragment { mTracingOn.setChecked(mTracingOn.getPreferenceManager().getSharedPreferences().getBoolean( mTracingOn.getKey(), false)); - // Grey out the toggle to change the trace engine if a trace is in progress. - mUsePerfetto.setEnabled(!mTracingOn.isChecked()); - // Update category list to match the categories available on the system. Set> availableTags = TraceUtils.listCategories().entrySet(); ArrayList entries = new ArrayList(availableTags.size()); @@ -281,12 +261,21 @@ public class MainFragment extends PreferenceFragment { context.getString(R.string.pref_key_buffer_size)); bufferSize.setSummary(bufferSize.getEntry()); - ListPreference maxLongTraceSize = (ListPreference)findPreference( - context.getString(R.string.pref_key_max_long_trace_size)); - maxLongTraceSize.setSummary(maxLongTraceSize.getEntry()); - - ListPreference maxLongTraceDuration = (ListPreference)findPreference( - context.getString(R.string.pref_key_max_long_trace_duration)); - maxLongTraceDuration.setSummary(maxLongTraceDuration.getEntry()); + // If we are not using the Perfetto trace backend, + // hide the unsupported preferences. + if (TraceUtils.currentTraceEngine().equals(PerfettoUtils.NAME)) { + ListPreference maxLongTraceSize = (ListPreference)findPreference( + context.getString(R.string.pref_key_max_long_trace_size)); + maxLongTraceSize.setSummary(maxLongTraceSize.getEntry()); + + ListPreference maxLongTraceDuration = (ListPreference)findPreference( + context.getString(R.string.pref_key_max_long_trace_duration)); + maxLongTraceDuration.setSummary(maxLongTraceDuration.getEntry()); + } else { + Preference longTraceCategory = findPreference("long_trace_category"); + if (longTraceCategory != null) { + getPreferenceScreen().removePreference(longTraceCategory); + } + } } } diff --git a/src/com/google/android/traceur/StopTraceService.java b/src/com/google/android/traceur/StopTraceService.java index fcdb470..a8dd636 100644 --- a/src/com/google/android/traceur/StopTraceService.java +++ b/src/com/google/android/traceur/StopTraceService.java @@ -37,7 +37,6 @@ public class StopTraceService extends TraceService { */ @Override public void onHandleIntent(Intent intent) { - setupTraceEngine(); Context context = getApplicationContext(); SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(context); boolean prefsTracingOn = diff --git a/src/com/google/android/traceur/TraceService.java b/src/com/google/android/traceur/TraceService.java index 0a242cb..dc567b6 100644 --- a/src/com/google/android/traceur/TraceService.java +++ b/src/com/google/android/traceur/TraceService.java @@ -80,7 +80,6 @@ public class TraceService extends IntentService { @Override public void onHandleIntent(Intent intent) { - setupTraceEngine(); Context context = getApplicationContext(); if (intent.getAction().equals(INTENT_ACTION_START_TRACING)) { @@ -184,12 +183,4 @@ public class TraceService extends IntentService { stopForeground(Service.STOP_FOREGROUND_REMOVE); } - protected void setupTraceEngine() { - Context context = getApplicationContext(); - boolean usePerfetto = - PreferenceManager.getDefaultSharedPreferences(context) - .getBoolean(context.getString(R.string.pref_key_use_perfetto), true); - TraceUtils.switchTraceEngine( - usePerfetto ? PerfettoUtils.NAME : AtraceUtils.NAME); - } } diff --git a/src/com/google/android/traceur/TraceUtils.java b/src/com/google/android/traceur/TraceUtils.java index 6350795..924d193 100644 --- a/src/com/google/android/traceur/TraceUtils.java +++ b/src/com/google/android/traceur/TraceUtils.java @@ -41,7 +41,9 @@ public class TraceUtils { public static final String TRACE_DIRECTORY = "/data/local/traces/"; - private static TraceEngine mTraceEngine = new AtraceUtils(); + // To change Traceur to use atrace to collect traces, + // change mTraceEngine to point to AtraceUtils(). + private static TraceEngine mTraceEngine = new PerfettoUtils(); private static final Runtime RUNTIME = Runtime.getRuntime(); @@ -55,26 +57,6 @@ public class TraceUtils { public boolean isTracingOn(); } - public static boolean switchTraceEngine(String newTraceEngine) { - if (mTraceEngine.getName().equals(newTraceEngine)) { - Log.e(TAG, "Tried to switch to use " + newTraceEngine - + " for tracing, but you already were!"); - return true; - } - - if (PerfettoUtils.NAME.equals(newTraceEngine)) { - mTraceEngine = new PerfettoUtils(); - } else if (AtraceUtils.NAME.equals(newTraceEngine)) { - mTraceEngine = new AtraceUtils(); - } else { - throw new AssertionError("Tried to switch to use " + newTraceEngine - + " for tracing, but I don't know what that is!"); - } - - Log.v(TAG, "Switched to using " + newTraceEngine + " for tracing!"); - return true; - } - public static String currentTraceEngine() { return mTraceEngine.getName(); } diff --git a/uitests/src/com/android/settings/ui/TraceurAppTests.java b/uitests/src/com/android/settings/ui/TraceurAppTests.java index 18717d4..aa3dfa8 100644 --- a/uitests/src/com/android/settings/ui/TraceurAppTests.java +++ b/uitests/src/com/android/settings/ui/TraceurAppTests.java @@ -103,6 +103,15 @@ public class TraceurAppTests { assertNotNull("Clear saved traces element not found.", mDevice.wait(Until.findObject(By.text("Clear saved traces")), TIMEOUT)); + assertNotNull("Long traces element not found.", + mDevice.wait(Until.findObject(By.text("Long traces")), + TIMEOUT)); + assertNotNull("Maximum long trace size element not found.", + mDevice.wait(Until.findObject(By.text("Maximum long trace size")), + TIMEOUT)); + assertNotNull("Maximum long trace duration element not found.", + mDevice.wait(Until.findObject(By.text("Maximum long trace duration")), + TIMEOUT)); assertNotNull("Show Quick Settings tile switch not found.", mDevice.wait(Until.findObject(By.text("Show Quick Settings tile")), TIMEOUT)); -- cgit v1.2.3