Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[Central Surfaces][Disable] Remove old from the disable logger.

Browse Source 
2/3 locations were passing in `null` for `old`, and the third location
in CentralSurfacesCommandQueueCallbacks was using a simple Log.d that we
never look at in bug reports.

Bug: 277764509
Test: atest DisableFlagsLoggerTest
Change-Id: Ib2bc98053c801bb9af6e948945169481e3b16afa
This commit is contained in:
Caitlin Shkuratov
2023-06-13 14:32:25 +00:00
parent 52fcb00a7d
commit ebddcea829
7 changed files with 26 additions and 105 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
packages/SystemUI/src/com/android/systemui/qs/QSFragmentDisableFlagsLogger.kt
View File

@@ -34,7 +34,6 @@ class QSFragmentDisableFlagsLogger @Inject constructor(
},
{
disableFlagsLogger.getDisableFlagsString(
old = null,
new = DisableFlagsLogger.DisableState(int1, int2),
newAfterLocalModification =
DisableFlagsLogger.DisableState(long1.toInt(), long2.toInt())

35
packages/SystemUI/src/com/android/systemui/statusbar/disableflags/DisableFlagsLogger.kt
View File

@@ -71,15 +71,14 @@ class DisableFlagsLogger constructor(
}
/**
* Returns a string representing the, old, new, and new-after-modification disable flag states,
* as well as the differences between each of the states.
* Returns a string representing the new and new-after-modification disable flag states,
* as well as the differences between them (if there are any).
*
* Example if [old], [new], and [newAfterLocalModification] are all different:
* Old: EnaiHbcRso.qINgr | New: EnaihBcRso.qiNGR (changed: hB.iGR) | New after local
* modification: EnaihBcRso.qInGR (changed: .n)
* Example if [new] and [newAfterLocalModification] are different:
* New: EnaihBcRso.qiNGR | New after local modification: EnaihBCRso.qInGR (changed: C.In)
*
* Example if [old] and [new] are the same:
* EnaihBcRso.qiNGR (unchanged)
* Example if [new] and [newAfterLocalModification] are the same:
* New: EnaihBcRso.qiNGR
*
* A capital character signifies the flag is set and a lowercase character signifies that the
* flag isn't set. The flag states will be logged in the same order as the passed-in lists.
@@ -88,37 +87,17 @@ class DisableFlagsLogger constructor(
* is no difference. the new-after-modification state also won't be included if there's no
* difference from the new state.
*
* @param old the disable state that had been previously sent. Null if we don't need to log the
* previously sent state.
* @param new the new disable state that has just been sent.
* @param newAfterLocalModification the new disable states after a class has locally modified
* them. Null if the class does not locally modify.
*/
fun getDisableFlagsString(
old: DisableState? = null,
new: DisableState,
newAfterLocalModification: DisableState? = null
): String {
val builder = StringBuilder("Received new disable state: ")
// This if/else has slightly repetitive code but is easier to read.
if (old != null && old != new) {
builder.append("Old: ")
builder.append(getFlagsString(old))
builder.append(" | ")
builder.append("New: ")
builder.append(getFlagsString(new))
builder.append(" ")
builder.append(getDiffString(old, new))
} else if (old != null && old == new) {
// If old and new are the same, we only need to print one of them.
builder.append(getFlagsString(new))
builder.append(" ")
builder.append(getDiffString(old, new))
} else { // old == null
builder.append(getFlagsString(new))
// Don't get a diff string because we have no [old] to compare with.
}
builder.append(getFlagsString(new))
if (newAfterLocalModification != null && new != newAfterLocalModification) {
builder.append(" | New after local modification: ")

2
packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesCommandQueueCallbacks.java
View File

@@ -255,8 +255,6 @@ public class CentralSurfacesCommandQueueCallbacks implements CommandQueue.Callba
state2 = mRemoteInputQuickSettingsDisabler.adjustDisableFlags(state2);
Log.d(CentralSurfaces.TAG,
mDisableFlagsLogger.getDisableFlagsString(
/* old= */ new DisableFlagsLogger.DisableState(
mCentralSurfaces.getDisabled1(), mCentralSurfaces.getDisabled2()),
/* new= */ new DisableFlagsLogger.DisableState(
state1, state2BeforeAdjustment),
/* newStateAfterLocalModification= */ new DisableFlagsLogger.DisableState(

4
packages/SystemUI/src/com/android/systemui/statusbar/phone/fragment/CollapsedStatusBarFragmentLogger.kt
View File

@@ -33,7 +33,6 @@ class CollapsedStatusBarFragmentLogger @Inject constructor(
* modifications that were made to the flags locally.
*
* @param new see [DisableFlagsLogger.getDisableFlagsString]
* @param newAfterLocalModification see [DisableFlagsLogger.getDisableFlagsString]
*/
fun logDisableFlagChange(
new: DisableFlagsLogger.DisableState,
@@ -47,8 +46,7 @@ class CollapsedStatusBarFragmentLogger @Inject constructor(
},
{
disableFlagsLogger.getDisableFlagsString(
old = null,
new = DisableFlagsLogger.DisableState(int1, int2),
DisableFlagsLogger.DisableState(int1, int2),
)
}
)

2
packages/SystemUI/tests/src/com/android/systemui/qs/QSFragmentDisableFlagsLoggerTest.kt
View File

@@ -49,7 +49,7 @@ class QSFragmentDisableFlagsLoggerTest : SysuiTestCase() {
buffer.dump(PrintWriter(stringWriter), tailLength = 0)
val actualString = stringWriter.toString()
val expectedLogString = disableFlagsLogger.getDisableFlagsString(
old = null, new = state, newAfterLocalModification = state
new = state, newAfterLocalModification = state
)
assertThat(actualString).contains(expectedLogString)

86
packages/SystemUI/tests/src/com/android/systemui/statusbar/disableflags/DisableFlagsLoggerTest.kt
View File

@@ -36,74 +36,9 @@ class DisableFlagsLoggerTest : SysuiTestCase() {
private val disableFlagsLogger = DisableFlagsLogger(disable1Flags, disable2Flags)
@Test
fun getDisableFlagsString_oldAndNewSame_newAndUnchangedLoggedOldNotLogged() {
val state = DisableFlagsLogger.DisableState(
0b111, // ABC
0b01 // mN
)
val result = disableFlagsLogger.getDisableFlagsString(state, state)
assertThat(result).doesNotContain("Old")
assertThat(result).contains("ABC.mN")
assertThat(result).contains("(unchanged)")
}
@Test
fun getDisableFlagsString_oldAndNewDifferent_statesAndDiffLogged() {
val result = disableFlagsLogger.getDisableFlagsString(
DisableFlagsLogger.DisableState(
0b111, // ABC
0b01, // mN
),
DisableFlagsLogger.DisableState(
0b001, // abC
0b10 // Mn
)
)
assertThat(result).contains("Old: ABC.mN")
assertThat(result).contains("New: abC.Mn")
assertThat(result).contains("(changed: ab.Mn)")
}
@Test
fun getDisableFlagsString_onlyDisable2Different_diffLoggedCorrectly() {
val result = disableFlagsLogger.getDisableFlagsString(
DisableFlagsLogger.DisableState(
0b001, // abC
0b01, // mN
),
DisableFlagsLogger.DisableState(
0b001, // abC
0b00 // mn
)
)
assertThat(result).contains("(changed: .n)")
}
@Test
fun getDisableFlagsString_nullOld_onlyNewStateLogged() {
val result = disableFlagsLogger.getDisableFlagsString(
old = null,
new = DisableFlagsLogger.DisableState(
0b001, // abC
0b01, // mN
),
)
assertThat(result).doesNotContain("Old")
assertThat(result).contains("abC.mN")
assertThat(result).doesNotContain("(")
assertThat(result).doesNotContain(")")
}
@Test
fun getDisableFlagsString_nullLocalModification_localModNotLogged() {
val result = disableFlagsLogger.getDisableFlagsString(
DisableFlagsLogger.DisableState(0, 0),
DisableFlagsLogger.DisableState(1, 1),
newAfterLocalModification = null
)
@@ -118,9 +53,7 @@ class DisableFlagsLoggerTest : SysuiTestCase() {
0b10 // mn
)
val result = disableFlagsLogger.getDisableFlagsString(
DisableFlagsLogger.DisableState(0, 0), newState, newState
)
val result = disableFlagsLogger.getDisableFlagsString(newState, newState)
assertThat(result).doesNotContain("local modification")
}
@@ -128,7 +61,6 @@ class DisableFlagsLoggerTest : SysuiTestCase() {
@Test
fun getDisableFlagsString_newAfterLocalModificationDifferent_localModAndDiffLogged() {
val result = disableFlagsLogger.getDisableFlagsString(
old = DisableFlagsLogger.DisableState(0, 0),
new = DisableFlagsLogger.DisableState(
0b000, // abc
0b00 // mn
@@ -142,6 +74,22 @@ class DisableFlagsLoggerTest : SysuiTestCase() {
assertThat(result).contains("local modification: Abc.Mn (changed: A.M)")
}
@Test
fun getDisableFlagsString_onlyDisable2Different_diffLoggedCorrectly() {
val result = disableFlagsLogger.getDisableFlagsString(
DisableFlagsLogger.DisableState(
0b001, // abC
0b01, // mN
),
DisableFlagsLogger.DisableState(
0b001, // abC
0b00 // mn
)
)
assertThat(result).contains("(changed: .n)")
}
@Test
fun constructor_defaultDisableFlags_noException() {
// Just creating the logger with the default params will trigger the exception if there

1
packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/fragment/CollapsedStatusBarFragmentLoggerTest.kt
View File

@@ -50,7 +50,6 @@ class CollapsedStatusBarFragmentLoggerTest : SysuiTestCase() {
val actualString = stringWriter.toString()
val expectedLogString =
disableFlagsLogger.getDisableFlagsString(
old = null,
new = state,
newAfterLocalModification = null,
)