From 89cff1ab753290860444da2bd40bf1a9734471da Mon Sep 17 00:00:00 2001
From: Fan Zhang <zhfan@google.com>
Date: Sat, 3 Jun 2017 17:46:55 -0700
Subject: [PATCH] Misc fixes in settingslib

Bug: 62039146
Fix: 62138362
Test: make RunSettingsLibRoboTests

- Do not use enhanced loop in lifecycle events to avoid Concurrent
modification exception.
- Tweak footer preference icon layout
- Add some logging to help debug future suggestion issues

Change-Id: I08eb3aca4b776a7d7e5db75dff8f92904d482aa3
---
 .../SettingsLib/res/layout/preference_footer.xml   |  6 ++--
 .../settingslib/core/lifecycle/Lifecycle.java      | 36 ++++++++++++++--------
 .../settingslib/suggestions/SuggestionParser.java  |  6 ++--
 .../settingslib/core/lifecycle/LifecycleTest.java  | 22 +++++++++++++
 4 files changed, 52 insertions(+), 18 deletions(-)

diff --git a/packages/SettingsLib/res/layout/preference_footer.xml b/packages/SettingsLib/res/layout/preference_footer.xml
index 1df4f7095ac5..d83e249d586b 100644
--- a/packages/SettingsLib/res/layout/preference_footer.xml
+++ b/packages/SettingsLib/res/layout/preference_footer.xml
@@ -26,14 +26,14 @@
     android:clipToPadding="false">
 
     <LinearLayout
-        android:id="@+id/icon_container"
+        android:id="@+id/icon_frame"
         android:layout_width="wrap_content"
         android:layout_height="wrap_content"
-        android:minWidth="60dp"
+        android:minWidth="56dp"
         android:gravity="start|top"
         android:orientation="horizontal"
         android:paddingEnd="12dp"
-        android:paddingTop="20dp"
+        android:paddingTop="16dp"
         android:paddingBottom="4dp">
         <ImageView
             android:id="@android:id/icon"
diff --git a/packages/SettingsLib/src/com/android/settingslib/core/lifecycle/Lifecycle.java b/packages/SettingsLib/src/com/android/settingslib/core/lifecycle/Lifecycle.java
index c63c6f4d83d0..b2351a9ee0e4 100644
--- a/packages/SettingsLib/src/com/android/settingslib/core/lifecycle/Lifecycle.java
+++ b/packages/SettingsLib/src/com/android/settingslib/core/lifecycle/Lifecycle.java
@@ -59,7 +59,8 @@ public class Lifecycle {
     }
 
     public void onAttach(Context context) {
-        for (LifecycleObserver observer : mObservers) {
+        for (int i = 0, size = mObservers.size(); i < size; i++) {
+            final LifecycleObserver observer = mObservers.get(i);
             if (observer instanceof OnAttach) {
                 ((OnAttach) observer).onAttach(context);
             }
@@ -67,7 +68,8 @@ public class Lifecycle {
     }
 
     public void onCreate(Bundle savedInstanceState) {
-        for (LifecycleObserver observer : mObservers) {
+        for (int i = 0, size = mObservers.size(); i < size; i++) {
+            final LifecycleObserver observer = mObservers.get(i);
             if (observer instanceof OnCreate) {
                 ((OnCreate) observer).onCreate(savedInstanceState);
             }
@@ -75,7 +77,8 @@ public class Lifecycle {
     }
 
     public void onStart() {
-        for (LifecycleObserver observer : mObservers) {
+        for (int i = 0, size = mObservers.size(); i < size; i++) {
+            final LifecycleObserver observer = mObservers.get(i);
             if (observer instanceof OnStart) {
                 ((OnStart) observer).onStart();
             }
@@ -83,7 +86,8 @@ public class Lifecycle {
     }
 
     public void setPreferenceScreen(PreferenceScreen preferenceScreen) {
-        for (LifecycleObserver observer : mObservers) {
+        for (int i = 0, size = mObservers.size(); i < size; i++) {
+            final LifecycleObserver observer = mObservers.get(i);
             if (observer instanceof SetPreferenceScreen) {
                 ((SetPreferenceScreen) observer).setPreferenceScreen(preferenceScreen);
             }
@@ -91,7 +95,8 @@ public class Lifecycle {
     }
 
     public void onResume() {
-        for (LifecycleObserver observer : mObservers) {
+        for (int i = 0, size = mObservers.size(); i < size; i++) {
+            final LifecycleObserver observer = mObservers.get(i);
             if (observer instanceof OnResume) {
                 ((OnResume) observer).onResume();
             }
@@ -99,7 +104,8 @@ public class Lifecycle {
     }
 
     public void onPause() {
-        for (LifecycleObserver observer : mObservers) {
+        for (int i = 0, size = mObservers.size(); i < size; i++) {
+            final LifecycleObserver observer = mObservers.get(i);
             if (observer instanceof OnPause) {
                 ((OnPause) observer).onPause();
             }
@@ -107,7 +113,8 @@ public class Lifecycle {
     }
 
     public void onSaveInstanceState(Bundle outState) {
-        for (LifecycleObserver observer : mObservers) {
+        for (int i = 0, size = mObservers.size(); i < size; i++) {
+            final LifecycleObserver observer = mObservers.get(i);
             if (observer instanceof OnSaveInstanceState) {
                 ((OnSaveInstanceState) observer).onSaveInstanceState(outState);
             }
@@ -115,7 +122,8 @@ public class Lifecycle {
     }
 
     public void onStop() {
-        for (LifecycleObserver observer : mObservers) {
+        for (int i = 0, size = mObservers.size(); i < size; i++) {
+            final LifecycleObserver observer = mObservers.get(i);
             if (observer instanceof OnStop) {
                 ((OnStop) observer).onStop();
             }
@@ -123,7 +131,8 @@ public class Lifecycle {
     }
 
     public void onDestroy() {
-        for (LifecycleObserver observer : mObservers) {
+        for (int i = 0, size = mObservers.size(); i < size; i++) {
+            final LifecycleObserver observer = mObservers.get(i);
             if (observer instanceof OnDestroy) {
                 ((OnDestroy) observer).onDestroy();
             }
@@ -131,7 +140,8 @@ public class Lifecycle {
     }
 
     public void onCreateOptionsMenu(final Menu menu, final @Nullable MenuInflater inflater) {
-        for (LifecycleObserver observer : mObservers) {
+        for (int i = 0, size = mObservers.size(); i < size; i++) {
+            final LifecycleObserver observer = mObservers.get(i);
             if (observer instanceof OnCreateOptionsMenu) {
                 ((OnCreateOptionsMenu) observer).onCreateOptionsMenu(menu, inflater);
             }
@@ -139,7 +149,8 @@ public class Lifecycle {
     }
 
     public void onPrepareOptionsMenu(final Menu menu) {
-        for (LifecycleObserver observer : mObservers) {
+        for (int i = 0, size = mObservers.size(); i < size; i++) {
+            final LifecycleObserver observer = mObservers.get(i);
             if (observer instanceof OnPrepareOptionsMenu) {
                 ((OnPrepareOptionsMenu) observer).onPrepareOptionsMenu(menu);
             }
@@ -147,7 +158,8 @@ public class Lifecycle {
     }
 
     public boolean onOptionsItemSelected(final MenuItem menuItem) {
-        for (LifecycleObserver observer : mObservers) {
+        for (int i = 0, size = mObservers.size(); i < size; i++) {
+            final LifecycleObserver observer = mObservers.get(i);
             if (observer instanceof OnOptionsItemSelected) {
                 if (((OnOptionsItemSelected) observer).onOptionsItemSelected(menuItem)) {
                     return true;
diff --git a/packages/SettingsLib/src/com/android/settingslib/suggestions/SuggestionParser.java b/packages/SettingsLib/src/com/android/settingslib/suggestions/SuggestionParser.java
index 23df01684b93..a28bece100eb 100644
--- a/packages/SettingsLib/src/com/android/settingslib/suggestions/SuggestionParser.java
+++ b/packages/SettingsLib/src/com/android/settingslib/suggestions/SuggestionParser.java
@@ -34,7 +34,6 @@ import android.support.annotation.VisibleForTesting;
 import android.text.TextUtils;
 import android.text.format.DateUtils;
 import android.util.ArrayMap;
-import android.util.ArraySet;
 import android.util.AttributeSet;
 import android.util.Log;
 import android.util.Pair;
@@ -50,7 +49,6 @@ import org.xmlpull.v1.XmlPullParserException;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.Set;
 
 public class SuggestionParser {
 
@@ -352,7 +350,9 @@ public class SuggestionParser {
             return false;
         }
         final long setupTime = mSharedPrefs.getLong(keySetupTime, 0);
-        return currentTime - setupTime > category.exclusiveExpireDaysInMillis;
+        final long elapsedTime = currentTime - setupTime;
+        Log.d(TAG, "Day " + elapsedTime / DateUtils.DAY_IN_MILLIS + " for " + category.category);
+        return elapsedTime > category.exclusiveExpireDaysInMillis;
     }
 
     private boolean isDismissed(Tile suggestion, boolean isSmartSuggestionEnabled) {
diff --git a/packages/SettingsLib/tests/robotests/src/com/android/settingslib/core/lifecycle/LifecycleTest.java b/packages/SettingsLib/tests/robotests/src/com/android/settingslib/core/lifecycle/LifecycleTest.java
index 9004ba419c93..585633226e59 100644
--- a/packages/SettingsLib/tests/robotests/src/com/android/settingslib/core/lifecycle/LifecycleTest.java
+++ b/packages/SettingsLib/tests/robotests/src/com/android/settingslib/core/lifecycle/LifecycleTest.java
@@ -210,6 +210,13 @@ public class LifecycleTest {
         assertThat(fragment.mFragObserver.mOnOptionsItemSelectedObserved).isTrue();
     }
 
+    @Test
+    public void addObserverDuringObserve_shoudNotCrash() {
+        Lifecycle lifecycle = new Lifecycle();
+        lifecycle.addObserver(new OnStartObserver(lifecycle));
+        lifecycle.onStart();
+    }
+
     private static class OptionItemAccepter implements LifecycleObserver, OnOptionsItemSelected {
         public boolean wasCalled = false;
 
@@ -236,4 +243,19 @@ public class LifecycleTest {
 
         assertThat(accepter.wasCalled).isFalse();
     }
+
+    private class OnStartObserver implements LifecycleObserver, OnStart {
+
+        private final Lifecycle mLifecycle;
+
+        public OnStartObserver(Lifecycle lifecycle) {
+            mLifecycle = lifecycle;
+        }
+
+        @Override
+        public void onStart() {
+            mLifecycle.addObserver(new LifecycleObserver() {
+            });
+        }
+    }
 }
-- 
2.11.0