From 622d698ff87e8afde15cb59d0187a0f94296b1dd Mon Sep 17 00:00:00 2001 From: John Zhen Mo Date: Thu, 8 Feb 2018 12:46:54 -0800 Subject: [PATCH 1/4] -Added LeakCanary to debug build for memory detection on activities and fragments. -Added LeakCanary no-op lib to release and beta builds. --- app/build.gradle | 4 ++++ .../java/org/schabi/newpipe/DebugApp.java | 11 +++++++++++ app/src/main/java/org/schabi/newpipe/App.java | 18 ++++++++++++++++++ .../java/org/schabi/newpipe/BaseFragment.java | 8 ++++++++ 4 files changed, 41 insertions(+) diff --git a/app/build.gradle b/app/build.gradle index 273616f91..d3084c2d3 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -89,4 +89,8 @@ dependencies { implementation 'frankiesardo:icepick:3.2.0' annotationProcessor 'frankiesardo:icepick-processor:3.2.0' + + debugImplementation 'com.squareup.leakcanary:leakcanary-android:1.5.4' + betaImplementation 'com.squareup.leakcanary:leakcanary-android-no-op:1.5.4' + releaseImplementation 'com.squareup.leakcanary:leakcanary-android-no-op:1.5.4' } diff --git a/app/src/debug/java/org/schabi/newpipe/DebugApp.java b/app/src/debug/java/org/schabi/newpipe/DebugApp.java index 4d37094ba..dba4179c2 100644 --- a/app/src/debug/java/org/schabi/newpipe/DebugApp.java +++ b/app/src/debug/java/org/schabi/newpipe/DebugApp.java @@ -4,6 +4,10 @@ import android.content.Context; import android.support.multidex.MultiDex; import com.facebook.stetho.Stetho; +import com.squareup.leakcanary.LeakCanary; +import com.squareup.leakcanary.RefWatcher; + +import java.util.concurrent.TimeUnit; public class DebugApp extends App { private static final String TAG = DebugApp.class.toString(); @@ -41,4 +45,11 @@ public class DebugApp extends App { // Initialize Stetho with the Initializer Stetho.initialize(initializer); } + + @Override + protected RefWatcher installLeakCanary() { + return LeakCanary.refWatcher(this) + .watchDelay(5, TimeUnit.SECONDS) + .buildAndInstall(); + } } diff --git a/app/src/main/java/org/schabi/newpipe/App.java b/app/src/main/java/org/schabi/newpipe/App.java index 79221db7f..e171c7726 100644 --- a/app/src/main/java/org/schabi/newpipe/App.java +++ b/app/src/main/java/org/schabi/newpipe/App.java @@ -10,6 +10,8 @@ import android.util.Log; import com.nostra13.universalimageloader.cache.memory.impl.LRULimitedMemoryCache; import com.nostra13.universalimageloader.core.ImageLoader; import com.nostra13.universalimageloader.core.ImageLoaderConfiguration; +import com.squareup.leakcanary.LeakCanary; +import com.squareup.leakcanary.RefWatcher; import org.acra.ACRA; import org.acra.config.ACRAConfiguration; @@ -54,6 +56,7 @@ import io.reactivex.plugins.RxJavaPlugins; public class App extends Application { protected static final String TAG = App.class.toString(); + private RefWatcher refWatcher; @SuppressWarnings("unchecked") private static final Class[] reportSenderFactoryClasses = new Class[]{AcraReportSenderFactory.class}; @@ -69,6 +72,13 @@ public class App extends Application { public void onCreate() { super.onCreate(); + if (LeakCanary.isInAnalyzerProcess(this)) { + // This process is dedicated to LeakCanary for heap analysis. + // You should not init your app in this process. + return; + } + refWatcher = installLeakCanary(); + // Initialize settings first because others inits can use its values SettingsActivity.initSettings(this); @@ -157,4 +167,12 @@ public class App extends Application { mNotificationManager.createNotificationChannel(mChannel); } + public static RefWatcher getRefWatcher(Context context) { + final App application = (App) context.getApplicationContext(); + return application.refWatcher; + } + + protected RefWatcher installLeakCanary() { + return RefWatcher.DISABLED; + } } diff --git a/app/src/main/java/org/schabi/newpipe/BaseFragment.java b/app/src/main/java/org/schabi/newpipe/BaseFragment.java index 6cd79e2c9..383e0dc4f 100644 --- a/app/src/main/java/org/schabi/newpipe/BaseFragment.java +++ b/app/src/main/java/org/schabi/newpipe/BaseFragment.java @@ -13,6 +13,7 @@ import android.view.View; import com.nostra13.universalimageloader.core.DisplayImageOptions; import com.nostra13.universalimageloader.core.ImageLoader; import com.nostra13.universalimageloader.core.display.FadeInBitmapDisplayer; +import com.squareup.leakcanary.RefWatcher; import icepick.Icepick; @@ -67,6 +68,13 @@ public abstract class BaseFragment extends Fragment { protected void onRestoreInstanceState(@NonNull Bundle savedInstanceState) { } + @Override + public void onDestroy() { + super.onDestroy(); + RefWatcher refWatcher = App.getRefWatcher(getActivity()); + refWatcher.watch(this); + } + /*////////////////////////////////////////////////////////////////////////// // Init //////////////////////////////////////////////////////////////////////////*/ From 829059ea0101a0ec1346a785d9d82f4231d29974 Mon Sep 17 00:00:00 2001 From: John Zhen Mo Date: Sat, 10 Feb 2018 11:07:17 -0800 Subject: [PATCH 2/4] -Added toggle for enabling leak canary heap dump. --- .../java/org/schabi/newpipe/DebugApp.java | 34 ++++++++++++++++++- app/src/main/java/org/schabi/newpipe/App.java | 2 ++ .../java/org/schabi/newpipe/BaseFragment.java | 3 +- .../java/org/schabi/newpipe/MainActivity.java | 24 +++++++++++++ app/src/main/res/menu/debug_menu.xml | 12 +++++++ app/src/main/res/values/settings_keys.xml | 3 ++ app/src/main/res/values/strings.xml | 3 ++ 7 files changed, 79 insertions(+), 2 deletions(-) create mode 100644 app/src/main/res/menu/debug_menu.xml diff --git a/app/src/debug/java/org/schabi/newpipe/DebugApp.java b/app/src/debug/java/org/schabi/newpipe/DebugApp.java index dba4179c2..ba1fd90cc 100644 --- a/app/src/debug/java/org/schabi/newpipe/DebugApp.java +++ b/app/src/debug/java/org/schabi/newpipe/DebugApp.java @@ -1,12 +1,20 @@ package org.schabi.newpipe; import android.content.Context; +import android.content.SharedPreferences; +import android.preference.PreferenceManager; +import android.support.annotation.NonNull; import android.support.multidex.MultiDex; import com.facebook.stetho.Stetho; +import com.squareup.leakcanary.AndroidHeapDumper; +import com.squareup.leakcanary.DefaultLeakDirectoryProvider; +import com.squareup.leakcanary.HeapDumper; import com.squareup.leakcanary.LeakCanary; +import com.squareup.leakcanary.LeakDirectoryProvider; import com.squareup.leakcanary.RefWatcher; +import java.io.File; import java.util.concurrent.TimeUnit; public class DebugApp extends App { @@ -49,7 +57,31 @@ public class DebugApp extends App { @Override protected RefWatcher installLeakCanary() { return LeakCanary.refWatcher(this) - .watchDelay(5, TimeUnit.SECONDS) + .heapDumper(new ToggleableHeapDumper(this)) + // give each object 10 seconds to be gc'ed, before leak canary gets nosy on it + .watchDelay(10, TimeUnit.SECONDS) .buildAndInstall(); } + + public static class ToggleableHeapDumper implements HeapDumper { + private final HeapDumper dumper; + private final SharedPreferences preferences; + private final String dumpingAllowanceKey; + + ToggleableHeapDumper(@NonNull final Context context) { + LeakDirectoryProvider leakDirectoryProvider = new DefaultLeakDirectoryProvider(context); + this.dumper = new AndroidHeapDumper(context, leakDirectoryProvider); + this.preferences = PreferenceManager.getDefaultSharedPreferences(context); + this.dumpingAllowanceKey = context.getString(R.string.allow_heap_dumping_key); + } + + private boolean isDumpingAllowed() { + return preferences.getBoolean(dumpingAllowanceKey, false); + } + + @Override + public File dumpHeap() { + return isDumpingAllowed() ? dumper.dumpHeap() : HeapDumper.RETRY_LATER; + } + } } diff --git a/app/src/main/java/org/schabi/newpipe/App.java b/app/src/main/java/org/schabi/newpipe/App.java index e171c7726..b15a38aae 100644 --- a/app/src/main/java/org/schabi/newpipe/App.java +++ b/app/src/main/java/org/schabi/newpipe/App.java @@ -5,6 +5,7 @@ import android.app.NotificationChannel; import android.app.NotificationManager; import android.content.Context; import android.os.Build; +import android.support.annotation.Nullable; import android.util.Log; import com.nostra13.universalimageloader.cache.memory.impl.LRULimitedMemoryCache; @@ -167,6 +168,7 @@ public class App extends Application { mNotificationManager.createNotificationChannel(mChannel); } + @Nullable public static RefWatcher getRefWatcher(Context context) { final App application = (App) context.getApplicationContext(); return application.refWatcher; diff --git a/app/src/main/java/org/schabi/newpipe/BaseFragment.java b/app/src/main/java/org/schabi/newpipe/BaseFragment.java index 383e0dc4f..d3e4a4b28 100644 --- a/app/src/main/java/org/schabi/newpipe/BaseFragment.java +++ b/app/src/main/java/org/schabi/newpipe/BaseFragment.java @@ -71,8 +71,9 @@ public abstract class BaseFragment extends Fragment { @Override public void onDestroy() { super.onDestroy(); + RefWatcher refWatcher = App.getRefWatcher(getActivity()); - refWatcher.watch(this); + if (refWatcher != null) refWatcher.watch(this); } /*////////////////////////////////////////////////////////////////////////// diff --git a/app/src/main/java/org/schabi/newpipe/MainActivity.java b/app/src/main/java/org/schabi/newpipe/MainActivity.java index 9a1ecd07a..329bd4165 100644 --- a/app/src/main/java/org/schabi/newpipe/MainActivity.java +++ b/app/src/main/java/org/schabi/newpipe/MainActivity.java @@ -211,6 +211,12 @@ public class MainActivity extends AppCompatActivity { } } + private void onHeapDumpToggled(@NonNull MenuItem item) { + final boolean newToggleState = !item.isChecked(); + sharedPreferences.edit().putBoolean(getString(R.string.allow_heap_dumping_key), + newToggleState).apply(); + item.setChecked(newToggleState); + } /*////////////////////////////////////////////////////////////////////////// // Menu //////////////////////////////////////////////////////////////////////////*/ @@ -232,6 +238,10 @@ public class MainActivity extends AppCompatActivity { inflater.inflate(R.menu.main_menu, menu); } + if (DEBUG) { + getMenuInflater().inflate(R.menu.debug_menu, menu); + } + ActionBar actionBar = getSupportActionBar(); if (actionBar != null) { actionBar.setDisplayHomeAsUpEnabled(false); @@ -242,6 +252,17 @@ public class MainActivity extends AppCompatActivity { return true; } + @Override + public boolean onPrepareOptionsMenu(Menu menu) { + MenuItem heapDumpToggle = menu.findItem(R.id.action_toggle_heap_dump); + if (heapDumpToggle != null) { + final boolean isToggled = sharedPreferences.getBoolean( + getString(R.string.allow_heap_dumping_key), false); + heapDumpToggle.setChecked(isToggled); + } + return super.onPrepareOptionsMenu(menu); + } + @Override public boolean onOptionsItemSelected(MenuItem item) { if (DEBUG) Log.d(TAG, "onOptionsItemSelected() called with: item = [" + item + "]"); @@ -262,6 +283,9 @@ public class MainActivity extends AppCompatActivity { case R.id.action_history: NavigationHelper.openHistory(this); return true; + case R.id.action_toggle_heap_dump: + onHeapDumpToggled(item); + return true; default: return super.onOptionsItemSelected(item); } diff --git a/app/src/main/res/menu/debug_menu.xml b/app/src/main/res/menu/debug_menu.xml new file mode 100644 index 000000000..448f9cf23 --- /dev/null +++ b/app/src/main/res/menu/debug_menu.xml @@ -0,0 +1,12 @@ + + + + + + \ No newline at end of file diff --git a/app/src/main/res/values/settings_keys.xml b/app/src/main/res/values/settings_keys.xml index 372b917e0..5934fdee6 100644 --- a/app/src/main/res/values/settings_keys.xml +++ b/app/src/main/res/values/settings_keys.xml @@ -83,6 +83,9 @@ last_orientation_landscape_key + + allow_heap_dumping_key + theme light_theme diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 767bffa10..fbdd22564 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -395,4 +395,7 @@ Added to playlist Playlist thumbnail changed Failed to delete playlist + + + Leak Canary From e7d148336b2c88c09835775c6e5e0e9d55c20a43 Mon Sep 17 00:00:00 2001 From: John Zhen Mo Date: Sat, 10 Feb 2018 12:24:23 -0800 Subject: [PATCH 3/4] -Changed leak canary toggle text to "monitor leaks". -Added toast when enabling/disabling heap dumping. --- .../java/org/schabi/newpipe/MainActivity.java | 19 +++++++++++++++---- app/src/main/res/values/strings.xml | 4 +++- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/MainActivity.java b/app/src/main/java/org/schabi/newpipe/MainActivity.java index 329bd4165..b54da2ad0 100644 --- a/app/src/main/java/org/schabi/newpipe/MainActivity.java +++ b/app/src/main/java/org/schabi/newpipe/MainActivity.java @@ -20,6 +20,7 @@ package org.schabi.newpipe; +import android.annotation.SuppressLint; import android.content.Intent; import android.content.SharedPreferences; import android.os.Bundle; @@ -39,6 +40,7 @@ import android.view.Menu; import android.view.MenuInflater; import android.view.MenuItem; import android.view.View; +import android.widget.Toast; import org.schabi.newpipe.extractor.StreamingService; import org.schabi.newpipe.fragments.BackPressable; @@ -211,11 +213,20 @@ public class MainActivity extends AppCompatActivity { } } + @SuppressLint("ShowToast") private void onHeapDumpToggled(@NonNull MenuItem item) { - final boolean newToggleState = !item.isChecked(); - sharedPreferences.edit().putBoolean(getString(R.string.allow_heap_dumping_key), - newToggleState).apply(); - item.setChecked(newToggleState); + final boolean isHeapDumpEnabled = !item.isChecked(); + + sharedPreferences.edit().putBoolean(getString(R.string.allow_heap_dumping_key), isHeapDumpEnabled).apply(); + item.setChecked(isHeapDumpEnabled); + + final String heapDumpNotice; + if (isHeapDumpEnabled) { + heapDumpNotice = getString(R.string.enable_leak_canary_notice); + } else { + heapDumpNotice = getString(R.string.disable_leak_canary_notice); + } + Toast.makeText(getApplicationContext(), heapDumpNotice, Toast.LENGTH_SHORT).show(); } /*////////////////////////////////////////////////////////////////////////// // Menu diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index fbdd22564..b1478fed4 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -397,5 +397,7 @@ Failed to delete playlist - Leak Canary + Monitor Leaks + Memory leak monitoring enabled, app may become unresponsive when heap dumping + Memory leak monitoring disabled From 263a816c3b9ad1de439a16d1a70964ece6bf74e2 Mon Sep 17 00:00:00 2001 From: John Zhen Mo Date: Sun, 11 Feb 2018 11:40:08 -0800 Subject: [PATCH 4/4] -Fixed preferences fetching. --- app/src/main/java/org/schabi/newpipe/MainActivity.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/MainActivity.java b/app/src/main/java/org/schabi/newpipe/MainActivity.java index b54da2ad0..ea6715f16 100644 --- a/app/src/main/java/org/schabi/newpipe/MainActivity.java +++ b/app/src/main/java/org/schabi/newpipe/MainActivity.java @@ -27,6 +27,7 @@ import android.os.Bundle; import android.os.Handler; import android.os.Looper; import android.preference.PreferenceManager; +import android.support.annotation.NonNull; import android.support.design.widget.NavigationView; import android.support.v4.app.Fragment; import android.support.v4.view.GravityCompat; @@ -217,7 +218,8 @@ public class MainActivity extends AppCompatActivity { private void onHeapDumpToggled(@NonNull MenuItem item) { final boolean isHeapDumpEnabled = !item.isChecked(); - sharedPreferences.edit().putBoolean(getString(R.string.allow_heap_dumping_key), isHeapDumpEnabled).apply(); + PreferenceManager.getDefaultSharedPreferences(this).edit() + .putBoolean(getString(R.string.allow_heap_dumping_key), isHeapDumpEnabled).apply(); item.setChecked(isHeapDumpEnabled); final String heapDumpNotice; @@ -267,8 +269,8 @@ public class MainActivity extends AppCompatActivity { public boolean onPrepareOptionsMenu(Menu menu) { MenuItem heapDumpToggle = menu.findItem(R.id.action_toggle_heap_dump); if (heapDumpToggle != null) { - final boolean isToggled = sharedPreferences.getBoolean( - getString(R.string.allow_heap_dumping_key), false); + final boolean isToggled = PreferenceManager.getDefaultSharedPreferences(this) + .getBoolean(getString(R.string.allow_heap_dumping_key), false); heapDumpToggle.setChecked(isToggled); } return super.onPrepareOptionsMenu(menu);