Don't trigger full relayout if DateTimeView doesn't change
Right now, when an update is triggered, DateTimeView will trigger a full relayout of its parents because it always calls setText, even if it doesn't change the contents (later down the line, TextView will call requestLayout() for wrap_content, even if size didn't change). Checking if the content actually changed avoids that extra layout pass which avoids needless relayouts in e.g. SystemUI Notifications. Bug:220712538 Test: atest DateTimeView (new test failed before fix / passes after fix) Change-Id: Ib391d6715136b64b98f0bda3b3d4a3f991809c89
This commit is contained in:
@@ -32,6 +32,7 @@ import android.content.res.TypedArray;
|
||||
import android.database.ContentObserver;
|
||||
import android.os.Build;
|
||||
import android.os.Handler;
|
||||
import android.text.TextUtils;
|
||||
import android.util.AttributeSet;
|
||||
import android.util.PluralsMessageFormatter;
|
||||
import android.view.accessibility.AccessibilityNodeInfo;
|
||||
@@ -230,7 +231,7 @@ public class DateTimeView extends TextView {
|
||||
|
||||
// Set the text
|
||||
String text = format.format(new Date(time));
|
||||
setText(text);
|
||||
maybeSetText(text);
|
||||
|
||||
// Schedule the next update
|
||||
if (display == SHOW_TIME) {
|
||||
@@ -258,7 +259,7 @@ public class DateTimeView extends TextView {
|
||||
boolean past = (now >= mTimeMillis);
|
||||
String result;
|
||||
if (duration < MINUTE_IN_MILLIS) {
|
||||
setText(mNowText);
|
||||
maybeSetText(mNowText);
|
||||
mUpdateTimeMillis = mTimeMillis + MINUTE_IN_MILLIS + 1;
|
||||
return;
|
||||
} else if (duration < HOUR_IN_MILLIS) {
|
||||
@@ -308,7 +309,19 @@ public class DateTimeView extends TextView {
|
||||
mUpdateTimeMillis = mTimeMillis - millisIncrease * count + 1;
|
||||
}
|
||||
}
|
||||
setText(result);
|
||||
maybeSetText(result);
|
||||
}
|
||||
|
||||
/**
|
||||
* Sets text only if the text has actually changed. This prevents needles relayouts of this
|
||||
* view when set to wrap_content.
|
||||
*/
|
||||
private void maybeSetText(String text) {
|
||||
if (TextUtils.equals(getText(), text)) {
|
||||
return;
|
||||
}
|
||||
|
||||
setText(text);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -16,14 +16,20 @@
|
||||
|
||||
package android.widget;
|
||||
|
||||
import android.view.View;
|
||||
import android.view.ViewGroup;
|
||||
|
||||
import androidx.test.InstrumentationRegistry;
|
||||
import androidx.test.annotation.UiThreadTest;
|
||||
import androidx.test.filters.SmallTest;
|
||||
import androidx.test.runner.AndroidJUnit4;
|
||||
|
||||
import org.junit.Assert;
|
||||
import org.junit.Test;
|
||||
import org.junit.runner.RunWith;
|
||||
|
||||
import java.time.Duration;
|
||||
|
||||
@RunWith(AndroidJUnit4.class)
|
||||
@SmallTest
|
||||
public class DateTimeViewTest {
|
||||
@@ -39,7 +45,33 @@ public class DateTimeViewTest {
|
||||
dateTimeView.detachedFromWindow();
|
||||
}
|
||||
|
||||
@UiThreadTest
|
||||
@Test
|
||||
public void noChangeInRelativeText_doesNotTriggerRelayout() {
|
||||
// Week in the future is chosen because it'll result in a stable string during this test
|
||||
// run. This should be improved once the class is refactored to be more testable in
|
||||
// respect of clock retrieval.
|
||||
final long weekInTheFuture = System.currentTimeMillis() + Duration.ofDays(7).toMillis();
|
||||
final TestDateTimeView dateTimeView = new TestDateTimeView();
|
||||
dateTimeView.setLayoutParams(new ViewGroup.LayoutParams(ViewGroup.LayoutParams.WRAP_CONTENT,
|
||||
ViewGroup.LayoutParams.WRAP_CONTENT));
|
||||
dateTimeView.setShowRelativeTime(true);
|
||||
dateTimeView.setTime(weekInTheFuture);
|
||||
// View needs to be measured to request layout, skipping this would make this test pass
|
||||
// always.
|
||||
dateTimeView.measure(View.MeasureSpec.makeMeasureSpec(200, View.MeasureSpec.UNSPECIFIED),
|
||||
View.MeasureSpec.makeMeasureSpec(100, View.MeasureSpec.UNSPECIFIED));
|
||||
dateTimeView.reset();
|
||||
|
||||
// This should not change the text content and thus no relayout is expected.
|
||||
dateTimeView.setTime(weekInTheFuture + 1000);
|
||||
|
||||
Assert.assertFalse(dateTimeView.wasLayoutRequested());
|
||||
}
|
||||
|
||||
private static class TestDateTimeView extends DateTimeView {
|
||||
private boolean mRequestedLayout = false;
|
||||
|
||||
TestDateTimeView() {
|
||||
super(InstrumentationRegistry.getContext());
|
||||
}
|
||||
@@ -51,5 +83,18 @@ public class DateTimeViewTest {
|
||||
void detachedFromWindow() {
|
||||
super.onDetachedFromWindow();
|
||||
}
|
||||
|
||||
public void requestLayout() {
|
||||
super.requestLayout();
|
||||
mRequestedLayout = true;
|
||||
}
|
||||
|
||||
public boolean wasLayoutRequested() {
|
||||
return mRequestedLayout;
|
||||
}
|
||||
|
||||
public void reset() {
|
||||
mRequestedLayout = false;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user