From 5b01294d8414cf03bbf7f4a2b30268537c03a0d7 Mon Sep 17 00:00:00 2001
From: Doris Ling <dling@google.com>
Date: Thu, 24 Aug 2017 13:09:57 -0700
Subject: [PATCH] Fix jank in showing conditions and suggestions in cold start.

When we first initialize the dashboard view, and register the condition
listener, it will trigger the condition changed callback immediately.
This results in unnecessary refresh of the dashboard header. Add check
to not do the refresh when we first initialize the view.

Change-Id: If7c69637463734c150b7f5eb7f3c042cf73837fa
Fixes: 64811475
Test: make RunSettingsRoboTests
---
 .../settings/dashboard/DashboardSummary.java  | 20 +++++++++++++++----
 .../dashboard/DashboardSummaryTest.java       | 16 +++++++++++++++
 2 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/src/com/android/settings/dashboard/DashboardSummary.java b/src/com/android/settings/dashboard/DashboardSummary.java
index fba27fb7aac..be5fbdf15f7 100644
--- a/src/com/android/settings/dashboard/DashboardSummary.java
+++ b/src/com/android/settings/dashboard/DashboardSummary.java
@@ -75,6 +75,7 @@ public class DashboardSummary extends InstrumentedFragment
     private DashboardFeatureProvider mDashboardFeatureProvider;
     private SuggestionFeatureProvider mSuggestionFeatureProvider;
     private boolean isOnCategoriesChangedCalled;
+    private boolean mOnConditionsChangedCalled;
 
     @Override
     public int getMetricsCategory() {
@@ -237,10 +238,21 @@ public class DashboardSummary extends InstrumentedFragment
     @Override
     public void onConditionsChanged() {
         Log.d(TAG, "onConditionsChanged");
-        final boolean scrollToTop = mLayoutManager.findFirstCompletelyVisibleItemPosition() <= 1;
-        mAdapter.setConditions(mConditionManager.getConditions());
-        if (scrollToTop) {
-            mDashboard.scrollToPosition(0);
+        // Bypass refreshing the conditions on the first call of onConditionsChanged.
+        // onConditionsChanged is called immediately everytime we start listening to the conditions
+        // change when we gain window focus. Since the conditions are passed to the adapter's
+        // constructor when we create the view, the first handling is not necessary.
+        // But, on the subsequent calls we need to handle it because there might be real changes to
+        // conditions.
+        if (mOnConditionsChangedCalled) {
+            final boolean scrollToTop =
+                    mLayoutManager.findFirstCompletelyVisibleItemPosition() <= 1;
+            mAdapter.setConditions(mConditionManager.getConditions());
+            if (scrollToTop) {
+                mDashboard.scrollToPosition(0);
+            }
+        } else {
+            mOnConditionsChangedCalled = true;
         }
     }
 
diff --git a/tests/robotests/src/com/android/settings/dashboard/DashboardSummaryTest.java b/tests/robotests/src/com/android/settings/dashboard/DashboardSummaryTest.java
index 967ed241a2a..f3ed57c2fb1 100644
--- a/tests/robotests/src/com/android/settings/dashboard/DashboardSummaryTest.java
+++ b/tests/robotests/src/com/android/settings/dashboard/DashboardSummaryTest.java
@@ -36,6 +36,7 @@ import org.robolectric.annotation.Config;
 import org.robolectric.util.ReflectionHelpers;
 
 import static org.mockito.ArgumentMatchers.nullable;
+import static org.mockito.Mockito.any;
 import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
@@ -90,6 +91,7 @@ public class DashboardSummaryTest {
     public void onConditionChanged_PositionAtTop_ScrollToTop() {
         when(mLayoutManager.findFirstCompletelyVisibleItemPosition()).thenReturn(1);
         mSummary.onConditionsChanged();
+        mSummary.onConditionsChanged();
         verify(mDashboard).scrollToPosition(0);
     }
 
@@ -97,9 +99,23 @@ public class DashboardSummaryTest {
     public void onConditionChanged_PositionNotTop_RemainPosition() {
         when(mLayoutManager.findFirstCompletelyVisibleItemPosition()).thenReturn(2);
         mSummary.onConditionsChanged();
+        mSummary.onConditionsChanged();
         verify(mDashboard, never()).scrollToPosition(0);
     }
 
+    @Test
+    public void onConditionChanged_firstCall_shouldIgnore() {
+        mSummary.onConditionsChanged();
+        verify(mAdapter, never()).setConditions(any());
+    }
+
+    @Test
+    public void onConditionChanged_secondCall_shouldSetConditionsOnAdapter() {
+        mSummary.onConditionsChanged();
+        mSummary.onConditionsChanged();
+        verify(mAdapter).setConditions(any());
+    }
+
     @Test
     public void onCategoryChanged_noRebuildOnFirstCall() {
         doReturn(mock(Activity.class)).when(mSummary).getActivity();