From a93743f9c4688200796c5aeb87949ec661be5acf Mon Sep 17 00:00:00 2001 From: Fan Zhang Date: Wed, 12 Sep 2018 15:27:03 -0700 Subject: [PATCH] Create custom card controller/render as soon as possible. Custom cards might need to monitor lifecycle events, waiting for onFinishLoading is too late. Also make sure custom cards cannot change card type. Test: manual Change-Id: Ib8f8e6e48926a63c9d241ed9e9843c025e3f634a --- .../homepage/ContextualCardManager.java | 25 +++++------ .../ConditionContextualCardController.java | 1 - .../ConditionalContextualCard.java | 15 ++++++- .../ConditionalContextualCardTest.java | 42 +++++++++++++++++++ .../ConditionContextualCardRendererTest.java | 1 - 5 files changed, 69 insertions(+), 15 deletions(-) create mode 100644 tests/robotests/src/com/android/settings/homepage/ConditionalContextualCardTest.java diff --git a/src/com/android/settings/homepage/ContextualCardManager.java b/src/com/android/settings/homepage/ContextualCardManager.java index 1eec6a7af98..8bb035550b8 100644 --- a/src/com/android/settings/homepage/ContextualCardManager.java +++ b/src/com/android/settings/homepage/ContextualCardManager.java @@ -20,6 +20,7 @@ import static com.android.settings.homepage.CardContentLoader.CARD_CONTENT_LOADE import android.content.Context; import android.os.Bundle; +import android.util.Log; import android.widget.BaseAdapter; import androidx.annotation.NonNull; @@ -60,12 +61,15 @@ public class ContextualCardManager implements CardContentLoader.CardContentLoade private ContextualCardUpdateListener mListener; - public ContextualCardManager(Context context, Lifecycle lifecycle) { mContext = context; mLifecycle = lifecycle; mContextualCards = new ArrayList<>(); mControllerRendererPool = new ControllerRendererPool(); + //for data provided by Settings + for (int cardType : SETTINGS_CARDS) { + setupController(cardType); + } } void loadContextualCards(PersonalSettingsFragment fragment) { @@ -82,22 +86,19 @@ public class ContextualCardManager implements CardContentLoader.CardContentLoade setupController(card.getCardType()); } } - - //for data provided by Settings - for (int cardType : SETTINGS_CARDS) { - setupController(cardType); - } } private void setupController(int cardType) { final ContextualCardController controller = mControllerRendererPool.getController(mContext, cardType); - if (controller != null) { - controller.setCardUpdateListener(this); - if (controller instanceof LifecycleObserver) { - if (mLifecycle != null) { - mLifecycle.addObserver((LifecycleObserver) controller); - } + if (controller == null) { + Log.w(TAG, "Cannot find ContextualCardController for type " + cardType); + return; + } + controller.setCardUpdateListener(this); + if (controller instanceof LifecycleObserver) { + if (mLifecycle != null) { + mLifecycle.addObserver((LifecycleObserver) controller); } } } diff --git a/src/com/android/settings/homepage/conditional/ConditionContextualCardController.java b/src/com/android/settings/homepage/conditional/ConditionContextualCardController.java index 6701cfd0196..1db3dd3f9cc 100644 --- a/src/com/android/settings/homepage/conditional/ConditionContextualCardController.java +++ b/src/com/android/settings/homepage/conditional/ConditionContextualCardController.java @@ -96,7 +96,6 @@ public class ConditionContextualCardController implements ContextualCardControll .setActionText(condition.getActionText()) .setName(mContext.getPackageName() + "/" + condition.getTitle().toString()) - .setCardType(ContextualCard.CardType.CONDITIONAL) .setTitleText(condition.getTitle().toString()) .setSummaryText(condition.getSummary().toString()) .setIconDrawable(condition.getIcon()) diff --git a/src/com/android/settings/homepage/conditional/ConditionalContextualCard.java b/src/com/android/settings/homepage/conditional/ConditionalContextualCard.java index 62a4956efcd..828f90b76de 100644 --- a/src/com/android/settings/homepage/conditional/ConditionalContextualCard.java +++ b/src/com/android/settings/homepage/conditional/ConditionalContextualCard.java @@ -16,6 +16,8 @@ package com.android.settings.homepage.conditional; +import androidx.annotation.VisibleForTesting; + import com.android.settings.homepage.ContextualCard; /** @@ -38,6 +40,11 @@ public class ConditionalContextualCard extends ContextualCard { mActionText = builder.mActionText; } + @Override + public int getCardType() { + return CardType.CONDITIONAL; + } + public long getConditionId() { return mConditionId; } @@ -50,7 +57,7 @@ public class ConditionalContextualCard extends ContextualCard { return mActionText; } - static class Builder extends ContextualCard.Builder { + public static class Builder extends ContextualCard.Builder { private long mConditionId; private int mMetricsConstant; @@ -71,6 +78,12 @@ public class ConditionalContextualCard extends ContextualCard { return this; } + @Override + public Builder setCardType(int cardType) { + throw new IllegalArgumentException( + "Cannot change card type for " + getClass().getName()); + } + public ConditionalContextualCard build() { return new ConditionalContextualCard(this); } diff --git a/tests/robotests/src/com/android/settings/homepage/ConditionalContextualCardTest.java b/tests/robotests/src/com/android/settings/homepage/ConditionalContextualCardTest.java new file mode 100644 index 00000000000..ced8b0691a8 --- /dev/null +++ b/tests/robotests/src/com/android/settings/homepage/ConditionalContextualCardTest.java @@ -0,0 +1,42 @@ +/* + * 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.homepage; + +import static com.google.common.truth.Truth.assertThat; + +import com.android.settings.homepage.conditional.ConditionalContextualCard; +import com.android.settings.testutils.SettingsRobolectricTestRunner; + +import org.junit.Test; +import org.junit.runner.RunWith; + +@RunWith(SettingsRobolectricTestRunner.class) +public class ConditionalContextualCardTest { + + @Test(expected = IllegalArgumentException.class) + public void newInstance_changeCardType_shouldCrash() { + new ConditionalContextualCard.Builder() + .setCardType(ContextualCard.CardType.SUGGESTION) + .build(); + } + + @Test + public void getCardType_shouldAlwaysBeConditional() { + assertThat(new ConditionalContextualCard.Builder().build().getCardType()) + .isEqualTo(ContextualCard.CardType.CONDITIONAL); + } +} diff --git a/tests/robotests/src/com/android/settings/homepage/conditional/ConditionContextualCardRendererTest.java b/tests/robotests/src/com/android/settings/homepage/conditional/ConditionContextualCardRendererTest.java index 1ee052d20d2..dc56a7f16c0 100644 --- a/tests/robotests/src/com/android/settings/homepage/conditional/ConditionContextualCardRendererTest.java +++ b/tests/robotests/src/com/android/settings/homepage/conditional/ConditionContextualCardRendererTest.java @@ -101,7 +101,6 @@ public class ConditionContextualCardRendererTest { .setMetricsConstant(1) .setActionText("test_action") .setName("test_name") - .setCardType(ContextualCard.CardType.CONDITIONAL) .setTitleText("test_title") .setSummaryText("test_summary") .setIconDrawable(mContext.getDrawable(R.drawable.ic_do_not_disturb_on_24dp))