diff --git a/core/api/current.txt b/core/api/current.txt index e1d7fd5839463..4d741bf79a7d7 100644 --- a/core/api/current.txt +++ b/core/api/current.txt @@ -46803,6 +46803,7 @@ package android.util { public interface DumpableContainer { method public boolean addDumpable(@NonNull android.util.Dumpable); + method public boolean removeDumpable(@NonNull android.util.Dumpable); } public class EventLog { diff --git a/core/java/android/util/DumpableContainer.java b/core/java/android/util/DumpableContainer.java index 04d19dc419268..e7b2acd5b5006 100644 --- a/core/java/android/util/DumpableContainer.java +++ b/core/java/android/util/DumpableContainer.java @@ -37,4 +37,15 @@ public interface DumpableContainer { * @return {@code true} if the dumpable was added, {@code false} if the call was ignored. */ boolean addDumpable(@NonNull Dumpable dumpable); + + /** + * Removes the given {@link Dumpable dumpable} from the container. + * + * @param dumpable dumpable to be removed. + * + * @return {@code true} if the dumpable was removed, {@code false} if it was not previously + * {@link #addDumpable(Dumpable) added} with the same identify (object reference) and + * {@link Dumpable#getDumpableName() name}. + */ + boolean removeDumpable(@NonNull Dumpable dumpable); } diff --git a/core/java/com/android/internal/util/dump/DumpableContainerImpl.java b/core/java/com/android/internal/util/dump/DumpableContainerImpl.java index d3916846905b1..2e56ebf1e41c5 100644 --- a/core/java/com/android/internal/util/dump/DumpableContainerImpl.java +++ b/core/java/com/android/internal/util/dump/DumpableContainerImpl.java @@ -24,11 +24,12 @@ import android.util.Log; import java.io.PrintWriter; import java.util.Objects; -// TODO(b/149254050): add unit tests /** * Helper class for {@link DumpableContainer} implementations - they can "implement it by * association", i.e., by delegating the interface methods to a {@code DumpableContainerImpl}. * + *

This class is not thread safe. + * * @hide */ public final class DumpableContainerImpl implements DumpableContainer { @@ -58,6 +59,38 @@ public final class DumpableContainerImpl implements DumpableContainer { return true; } + @Override + public boolean removeDumpable(Dumpable dumpable) { + Objects.requireNonNull(dumpable, "dumpable"); + String name = dumpable.getDumpableName(); + if (name == null) { + if (DEBUG) { + Log.d(TAG, "Tried to remove nameless dumpable: " + dumpable); + } + return false; + } + + Dumpable candidate = mDumpables.get(name); + if (candidate == null) { + if (DEBUG) { + Log.d(TAG, "Dumpable with name " + name + " not found"); + } + return false; + } + + // Make sure it's the right one + if (candidate != dumpable) { + Log.w(TAG, "removeDumpable(): passed dumpable (" + dumpable + ") named " + name + + ", but internal dumpable with that name is " + candidate); + return false; + } + if (DEBUG) { + Log.d(TAG, "Removing dumpable named " + name); + } + mDumpables.remove(name); + return true; + } + /** * Dumps the number of dumpable, without a newline. */ diff --git a/core/tests/coretests/src/com/android/internal/util/DumpableContainerImplTest.java b/core/tests/coretests/src/com/android/internal/util/DumpableContainerImplTest.java index 4c58dfa0d55d8..589e4f98a32a1 100644 --- a/core/tests/coretests/src/com/android/internal/util/DumpableContainerImplTest.java +++ b/core/tests/coretests/src/com/android/internal/util/DumpableContainerImplTest.java @@ -27,6 +27,7 @@ import org.junit.Test; import java.io.PrintWriter; import java.io.StringWriter; +import java.util.concurrent.atomic.AtomicReference; public final class DumpableContainerImplTest { @@ -41,7 +42,7 @@ public final class DumpableContainerImplTest { @Test public void testAddDumpable_dumpableWithoutName() { - Dumpable noNamer = new Dumpable() { + Dumpable namelessDumpable = new Dumpable() { @Override public String getDumpableName() { @@ -54,7 +55,7 @@ public final class DumpableContainerImplTest { } }; - assertThrows(NullPointerException.class, () -> mImpl.addDumpable(noNamer)); + assertThrows(NullPointerException.class, () -> mImpl.addDumpable(namelessDumpable)); } @Test @@ -178,11 +179,76 @@ public final class DumpableContainerImplTest { + "......6 Args: 4,8,15,16,23,42,\n"); } + @Test + public void testRemoveDumpable_null() { + assertThrows(NullPointerException.class, () -> mImpl.removeDumpable(null)); + } + + @Test + public void testARemoveDumpable_dumpableWithoutName() { + // Need a non-null name initially otherwise it won't be added + AtomicReference name = new AtomicReference<>("A Dumpable Has No Name"); + Dumpable dumpable = new Dumpable() { + + @Override + public String getDumpableName() { + return name.get(); + } + + @Override + public void dump(PrintWriter writer, String[] args) { + throw new UnsupportedOperationException("D'OH!"); + } + + }; + assertWithMessage("addDumpable(with name)").that(mImpl.addDumpable(dumpable)).isTrue(); + + name.set(null); + assertWithMessage("removeDumpable(nameless)").that(mImpl.removeDumpable(dumpable)) + .isFalse(); + } + + @Test + public void testRemoveDumpable_empty() { + CustomDumpable dumpable = new CustomDumpable("The name is Bond", "James Bond!"); + + assertWithMessage("removeDumpable()").that(mImpl.removeDumpable(dumpable)).isFalse(); + } + + @Test + public void testRemoveDumpable_sameNameButDifferentDumpable() { + CustomDumpable real = new CustomDumpable("Slim Shade", "Please stand up!"); + CustomDumpable fake = new CustomDumpable("Slim Shade", "Please stand up!"); + + mImpl.addDumpable(real); + + assertWithMessage("removeDumpable(fake)").that(mImpl.removeDumpable(fake)).isFalse(); + assertWithMessage("removeDumpable(real)").that(mImpl.removeDumpable(real)).isTrue(); + } + + @Test + public void testRemoveDumpable_existing() { + CustomDumpable dumpable = new CustomDumpable("Homer", "D'ohmp!"); + + mImpl.addDumpable(dumpable); + mImpl.listDumpables("...", mWriter); + assertWithMessage("listDumpables()").that(getOutput()).isEqualTo("...1 dumpables: Homer\n"); + + assertWithMessage("removeDumpable()").that(mImpl.removeDumpable(dumpable)).isTrue(); + resetOutput(); + mImpl.listDumpables("...", mWriter); + assertWithMessage("listDumpables(...)").that(getOutput()).isEqualTo("...No dumpables\n"); + } + private String getOutput() { mSw.flush(); return mSw.toString(); } + private void resetOutput() { + mSw.getBuffer().setLength(0); + } + private static final class CustomDumpable implements Dumpable { public final String name; public final String content; diff --git a/packages/SystemUI/src/com/android/systemui/SystemUIApplication.java b/packages/SystemUI/src/com/android/systemui/SystemUIApplication.java index daca918ec0c51..5bdee2a61b9b8 100644 --- a/packages/SystemUI/src/com/android/systemui/SystemUIApplication.java +++ b/packages/SystemUI/src/com/android/systemui/SystemUIApplication.java @@ -272,7 +272,7 @@ public class SystemUIApplication extends Application implements mServicesStarted = true; } - // TODO(b/149254050): add unit tests? There doesn't seem to be a SystemUiApplicationTest... + // TODO(b/217567642): add unit tests? There doesn't seem to be a SystemUiApplicationTest... @Override public boolean addDumpable(Dumpable dumpable) { String name = dumpable.getDumpableName(); @@ -288,13 +288,21 @@ public class SystemUIApplication extends Application implements if (DEBUG) Log.d(TAG, "addDumpable(): adding '" + name + "' = " + dumpable); mDumpables.put(name, dumpable); - // TODO(b/149254050): replace com.android.systemui.dump.Dumpable by + // TODO(b/217567642): replace com.android.systemui.dump.Dumpable by // com.android.util.Dumpable and get rid of the intermediate lambda mDumpManager.registerDumpable(dumpable.getDumpableName(), (fd, pw, args) -> dumpable.dump(pw, args)); return true; } + // TODO(b/217567642): implement + @Override + public boolean removeDumpable(Dumpable dumpable) { + Log.w(TAG, "removeDumpable(" + dumpable + "): not implemented"); + + return false; + } + @Override public void onConfigurationChanged(Configuration newConfig) { if (mServicesStarted) {