From c98a5bb2d806dde99325a15466a2bf144d216ae8 Mon Sep 17 00:00:00 2001 From: Yorke Lee Date: Tue, 28 Oct 2014 16:12:05 -0700 Subject: Refactor ActionBarController and tests The original CL to fix ActionBarControllerTest by using an ActivityInstrumentationTestCase was too heavyweight and caused flaky test. It was also indicative of a flaw in the original design that ActionBar couldn't be mocked out easily. Refactor the code so that: 1) getActionBar() is no longer needed 2) Respect the invariant that all access to the actionBar goes through the ActionBarController (see the changes in ListsFragment) so that any change to the ActionBar's hide offset can be tracked appropriately. Bug: 17487016 Change-Id: I935b683ddb301ca5d3952889af1381bdbdcc97d3 --- src/com/android/dialer/DialtactsActivity.java | 13 ++++-- src/com/android/dialer/list/ListsFragment.java | 11 ++--- .../android/dialer/widget/ActionBarController.java | 11 +++-- .../dialer/widget/ActionBarControllerTest.java | 47 +++++----------------- 4 files changed, 32 insertions(+), 50 deletions(-) diff --git a/src/com/android/dialer/DialtactsActivity.java b/src/com/android/dialer/DialtactsActivity.java index aacf20844..1c933504d 100644 --- a/src/com/android/dialer/DialtactsActivity.java +++ b/src/com/android/dialer/DialtactsActivity.java @@ -1134,6 +1134,11 @@ public class DialtactsActivity extends TransactionSafeActivity implements View.O return mActionBarController.isActionBarShowing(); } + @Override + public ActionBarController getActionBarController() { + return mActionBarController; + } + public boolean isDialpadShown() { return mIsDialpadShown; } @@ -1144,13 +1149,13 @@ public class DialtactsActivity extends TransactionSafeActivity implements View.O } @Override - public int getActionBarHeight() { - return mActionBarHeight; + public void setActionBarHideOffset(int offset) { + getActionBar().setHideOffset(offset); } @Override - public void setActionBarHideOffset(int hideOffset) { - mActionBarController.setHideOffset(hideOffset); + public int getActionBarHeight() { + return mActionBarHeight; } /** diff --git a/src/com/android/dialer/list/ListsFragment.java b/src/com/android/dialer/list/ListsFragment.java index de501f47e..8e2500aa8 100644 --- a/src/com/android/dialer/list/ListsFragment.java +++ b/src/com/android/dialer/list/ListsFragment.java @@ -30,6 +30,7 @@ import com.android.dialer.calllog.CallLogQueryHandler; import com.android.dialer.calllog.ContactInfoHelper; import com.android.dialer.list.ShortcutCardsAdapter.SwipeableShortcutCard; import com.android.dialer.util.DialerUtils; +import com.android.dialer.widget.ActionBarController; import com.android.dialer.widget.OverlappingPaneLayout; import com.android.dialer.widget.OverlappingPaneLayout.PanelSlideCallbacks; import com.android.dialerbind.ObjectFactory; @@ -68,8 +69,7 @@ public class ListsFragment extends AnalyticsFragment implements CallLogQueryHand public interface HostInterface { public void showCallHistory(); - public int getActionBarHeight(); - public void setActionBarHideOffset(int offset); + public ActionBarController getActionBarController(); } private ActionBar mActionBar; @@ -124,8 +124,9 @@ public class ListsFragment extends AnalyticsFragment implements CallLogQueryHand final int availableActionBarHeight = Math.min(mActionBar.getHeight(), topPaneHeight); - ((HostInterface) getActivity()).setActionBarHideOffset( - mActionBar.getHeight() - availableActionBarHeight); + final ActionBarController controller = + ((HostInterface) getActivity()).getActionBarController(); + controller.setHideOffset(mActionBar.getHeight() - availableActionBarHeight); if (!mActionBar.isShowing()) { mActionBar.show(); @@ -407,7 +408,7 @@ public class ListsFragment extends AnalyticsFragment implements CallLogQueryHand paneLayout.openPane(); paneLayout.setPanelSlideCallbacks(mPanelSlideCallbacks); paneLayout.setIntermediatePinnedOffset( - ((HostInterface) getActivity()).getActionBarHeight()); + ((HostInterface) getActivity()).getActionBarController().getActionBarHeight()); LayoutTransition transition = paneLayout.getLayoutTransition(); // Turns on animations for all types of layout changes so that they occur for diff --git a/src/com/android/dialer/widget/ActionBarController.java b/src/com/android/dialer/widget/ActionBarController.java index 265c03f6f..b9923d186 100644 --- a/src/com/android/dialer/widget/ActionBarController.java +++ b/src/com/android/dialer/widget/ActionBarController.java @@ -46,7 +46,8 @@ public class ActionBarController { public boolean hasSearchQuery(); public boolean shouldShowActionBar(); public int getActionBarHeight(); - public ActionBar getActionBar(); + public int getActionBarHideOffset(); + public void setActionBarHideOffset(int offset); } public ActionBarController(ActivityUi activityUi, SearchEditTextLayout searchBox) { @@ -166,14 +167,18 @@ public class ActionBarController { public void setHideOffset(int offset) { mIsActionBarSlidUp = offset >= mActivityUi.getActionBarHeight(); - mActivityUi.getActionBar().setHideOffset(offset); + mActivityUi.setActionBarHideOffset(offset); } /** * @return The offset the action bar is being translated upwards by */ public int getHideOffset() { - return mActivityUi.getActionBar().getHideOffset(); + return mActivityUi.getActionBarHideOffset(); + } + + public int getActionBarHeight() { + return mActivityUi.getActionBarHeight(); } /** diff --git a/tests/src/com/android/dialer/widget/ActionBarControllerTest.java b/tests/src/com/android/dialer/widget/ActionBarControllerTest.java index 2e388380f..316c15537 100644 --- a/tests/src/com/android/dialer/widget/ActionBarControllerTest.java +++ b/tests/src/com/android/dialer/widget/ActionBarControllerTest.java @@ -16,54 +16,25 @@ package com.android.dialer.widget; -import android.app.ActionBar; -import android.app.Activity; import android.content.Context; -import android.test.ActivityInstrumentationTestCase2; -import android.test.AndroidTestCase; import android.test.InstrumentationTestCase; import android.test.suitebuilder.annotation.SmallTest; -import android.view.View; -import com.android.dialer.DialtactsActivity; import com.android.dialer.widget.ActionBarController.ActivityUi; -import com.android.internal.app.WindowDecorActionBar; @SmallTest -public class ActionBarControllerTest extends ActivityInstrumentationTestCase2 { +public class ActionBarControllerTest extends InstrumentationTestCase { private static final int ACTION_BAR_HEIGHT = 100; private ActionBarController mActionBarController; private SearchEditTextLayout mSearchBox; private MockActivityUi mActivityUi; - private class MockActionBar extends WindowDecorActionBar { - private int mHideOffset = 0; - - public MockActionBar(Activity activity) { - super(activity); - } - - @Override - public void setHideOffset(int offset) { - mHideOffset = offset; - } - - @Override - public int getHideOffset() { - return mHideOffset; - } - } - private class MockActivityUi implements ActivityUi { boolean isInSearchUi; boolean hasSearchQuery; boolean shouldShowActionBar; - private ActionBar mActionBar; - - public MockActivityUi() { - mActionBar = new MockActionBar(getActivity()); - } + int actionBarHideOffset; @Override public boolean isInSearchUi() { @@ -86,8 +57,13 @@ public class ActionBarControllerTest extends ActivityInstrumentationTestCase2