From f1030e8cdfe957200bc5f8370640f831fd702392 Mon Sep 17 00:00:00 2001 From: Fan Zhang Date: Wed, 4 Apr 2018 15:51:06 -0700 Subject: [PATCH] Fix a bug where homepage is using staled locale for tiles When setting a new locale, SettingsActivity restarts to load everything in the new locale. Data (containing locale specific title/summary etc) is reloaded correctly and triggers a callback to UI to redraw. However we skip the first callback as an optimization for app startup time. When we restart fragment, we failed to save the state whether we have already seen the first callback. So when data with new locale text triggers the callback, it's being skipped and this make UI still render in old locale. The fix is to just save the state before fragment gets destroyed before locale change so the callback can trigger later. A better fix is: make data (Tile object) not cache text. Then we don't need to worry about locale cache at all. We should do this fix in the long term. Test: localeswitcher Test: adb shell am broadcast -a com.google.android.testing.i18n.localeswitcher.CHANGE_LOCALE -e LANGUAGE_TAG "zh" Test: adb shell am broadcast -a com.google.android.testing.i18n.localeswitcher.CHANGE_LOCALE -e LANGUAGE_TAG "ja" Fixes: 77470788 Bug: 77600770 Change-Id: Ic4223ddbb679db64d0fc3c29d16a5f61a66cc99c --- .../settings/dashboard/DashboardSummary.java | 21 +++-- .../DashboardSummaryInstrumentationTest.java | 79 +++++++++++++++++++ 2 files changed, 93 insertions(+), 7 deletions(-) create mode 100644 tests/unit/src/com/android/settings/dashboard/DashboardSummaryInstrumentationTest.java diff --git a/src/com/android/settings/dashboard/DashboardSummary.java b/src/com/android/settings/dashboard/DashboardSummary.java index fc1e28909c4..af7c90063aa 100644 --- a/src/com/android/settings/dashboard/DashboardSummary.java +++ b/src/com/android/settings/dashboard/DashboardSummary.java @@ -58,7 +58,8 @@ public class DashboardSummary extends InstrumentedFragment private static final int MAX_WAIT_MILLIS = 3000; private static final String TAG = "DashboardSummary"; - private static final String EXTRA_SCROLL_POSITION = "scroll_position"; + private static final String STATE_SCROLL_POSITION = "scroll_position"; + private static final String STATE_CATEGORIES_CHANGE_CALLED = "categories_change_called"; private final Handler mHandler = new Handler(); @@ -69,7 +70,8 @@ public class DashboardSummary extends InstrumentedFragment private LinearLayoutManager mLayoutManager; private SuggestionControllerMixin mSuggestionControllerMixin; private DashboardFeatureProvider mDashboardFeatureProvider; - private boolean isOnCategoriesChangedCalled; + @VisibleForTesting + boolean mIsOnCategoriesChangedCalled; private boolean mOnConditionsChangedCalled; private DashboardCategory mStagingCategory; @@ -115,6 +117,10 @@ public class DashboardSummary extends InstrumentedFragment mConditionManager = ConditionManager.get(activity, false); getLifecycle().addObserver(mConditionManager); + if (savedInstanceState != null) { + mIsOnCategoriesChangedCalled = + savedInstanceState.getBoolean(STATE_CATEGORIES_CHANGE_CALLED); + } if (DEBUG_TIMING) { Log.d(TAG, "onCreate took " + (System.currentTimeMillis() - startTime) + " ms"); } @@ -182,7 +188,8 @@ public class DashboardSummary extends InstrumentedFragment if (mLayoutManager == null) { return; } - outState.putInt(EXTRA_SCROLL_POSITION, mLayoutManager.findFirstVisibleItemPosition()); + outState.putBoolean(STATE_CATEGORIES_CHANGE_CALLED, mIsOnCategoriesChangedCalled); + outState.putInt(STATE_SCROLL_POSITION, mLayoutManager.findFirstVisibleItemPosition()); } @Override @@ -193,7 +200,7 @@ public class DashboardSummary extends InstrumentedFragment mLayoutManager = new LinearLayoutManager(getContext()); mLayoutManager.setOrientation(LinearLayoutManager.VERTICAL); if (bundle != null) { - int scrollPosition = bundle.getInt(EXTRA_SCROLL_POSITION); + int scrollPosition = bundle.getInt(STATE_SCROLL_POSITION); mLayoutManager.scrollToPosition(scrollPosition); } mDashboard.setLayoutManager(mLayoutManager); @@ -201,7 +208,7 @@ public class DashboardSummary extends InstrumentedFragment mDashboard.setListener(this); mDashboard.setItemAnimator(new DashboardItemAnimator()); mAdapter = new DashboardAdapter(getContext(), bundle, - mConditionManager.getConditions(), mSuggestionControllerMixin, getLifecycle()); + mConditionManager.getConditions(), mSuggestionControllerMixin, getLifecycle()); mDashboard.setAdapter(mAdapter); mSummaryLoader.setSummaryConsumer(mAdapter); ActionBarShadowController.attachToRecyclerView( @@ -224,10 +231,10 @@ public class DashboardSummary extends InstrumentedFragment // Bypass rebuildUI() on the first call of onCategoriesChanged, since rebuildUI() happens // in onViewCreated as well when app starts. But, on the subsequent calls we need to // rebuildUI() because there might be some changes to suggestions and categories. - if (isOnCategoriesChangedCalled) { + if (mIsOnCategoriesChangedCalled) { rebuildUI(); } - isOnCategoriesChangedCalled = true; + mIsOnCategoriesChangedCalled = true; } @Override diff --git a/tests/unit/src/com/android/settings/dashboard/DashboardSummaryInstrumentationTest.java b/tests/unit/src/com/android/settings/dashboard/DashboardSummaryInstrumentationTest.java new file mode 100644 index 00000000000..021013a732d --- /dev/null +++ b/tests/unit/src/com/android/settings/dashboard/DashboardSummaryInstrumentationTest.java @@ -0,0 +1,79 @@ +/* + * Copyright (C) 2018 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.settings.dashboard; + +import static com.google.common.truth.Truth.assertThat; + +import android.app.Activity; +import android.app.Fragment; +import android.app.Instrumentation; +import android.content.Context; +import android.content.Intent; +import android.content.pm.ActivityInfo; +import android.provider.Settings; +import android.support.test.InstrumentationRegistry; +import android.support.test.filters.SmallTest; +import android.support.test.runner.AndroidJUnit4; +import android.support.test.uiautomator.By; +import android.support.test.uiautomator.UiDevice; +import android.support.test.uiautomator.UiObject2; +import android.support.test.uiautomator.Until; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; + +import java.util.List; + +@RunWith(AndroidJUnit4.class) +@SmallTest +public class DashboardSummaryInstrumentationTest { + + private static final long TIMEOUT = 2000l; + + private Context mContext; + private Instrumentation mInstrumentation; + + private UiDevice mDevice; + + @Before + public void setUp() { + mInstrumentation = InstrumentationRegistry.getInstrumentation(); + mDevice = UiDevice.getInstance(mInstrumentation); + mContext = InstrumentationRegistry.getTargetContext(); + } + + @Test + public void rotate_shouldSaveCategoriesChangedState() { + final Intent intent = new Intent(Settings.ACTION_SETTINGS) + .addFlags(Intent.FLAG_ACTIVITY_NEW_TASK); + final Activity activity = mInstrumentation.startActivitySync(intent); + + activity.setRequestedOrientation(ActivityInfo.SCREEN_ORIENTATION_LANDSCAPE); + activity.setRequestedOrientation(ActivityInfo.SCREEN_ORIENTATION_PORTRAIT); + + final UiObject2 item = mDevice.wait(Until.findObject(By.res("android:id/title") + .text("Network & internet")), TIMEOUT); + assertThat(item).isNotNull(); + + final List fragments = activity.getFragmentManager().getFragments(); + final DashboardSummary fragment = (DashboardSummary) fragments.get(0); + + assertThat(fragment.mIsOnCategoriesChangedCalled).isTrue(); + } + +}