From 9424af7315de764c6609dac7676c283481654202 Mon Sep 17 00:00:00 2001 From: Jason Monk Date: Wed, 19 Dec 2018 14:17:26 -0500 Subject: [PATCH] Add support for dagger injection with fragments Convert NavigationBarFragment as a proof of concept and remove all references to Dependency from NavigationBarFragment. Test: atest SystemUITests Change-Id: I0cdb5bc6ac455fce91e67b9e449cb7b78b1da9a4 --- packages/SystemUI/docs/dagger.md | 22 ++++++++ packages/SystemUI/proguard.flags | 3 ++ .../android/systemui/DependencyProvider.java | 7 --- .../com/android/systemui/SystemUIFactory.java | 7 +++ .../fragments/FragmentHostManager.java | 36 ++++++++++++- .../systemui/fragments/FragmentService.java | 47 +++++++++++++++++ .../phone/NavigationBarFragment.java | 33 +++++++----- .../phone/NavigationBarFragmentTest.java | 52 +++++++++++++------ .../src/android/testing/BaseFragmentTest.java | 16 +++++- 9 files changed, 186 insertions(+), 37 deletions(-) diff --git a/packages/SystemUI/docs/dagger.md b/packages/SystemUI/docs/dagger.md index 565d765f55563..8bfa1c28d9854 100644 --- a/packages/SystemUI/docs/dagger.md +++ b/packages/SystemUI/docs/dagger.md @@ -125,6 +125,28 @@ public class Dependency { } ``` +### Using injection with Fragments + +Fragments are created as part of the FragmentManager, so they need to be +setup so the manager knows how to create them. To do that, add a method +to com.android.systemui.fragments.FragmentService$FragmentCreator that +returns your fragment class. Thats all thats required, once the method +exists, FragmentService will automatically pick it up and use injection +whenever your fragment needs to be created. + +```java +public interface FragmentCreator { ++ NavigationBarFragment createNavigationBar(); +} +``` + +If you need to create your fragment (i.e. for the add or replace transaction), +then the FragmentHostManager can do this for you. + +```java +FragmentHostManager.get(view).create(NavigationBarFragment.class); +``` + ## TODO List - Eliminate usages of Depndency#get diff --git a/packages/SystemUI/proguard.flags b/packages/SystemUI/proguard.flags index cc6848f323e9a..d57fe8a36d619 100644 --- a/packages/SystemUI/proguard.flags +++ b/packages/SystemUI/proguard.flags @@ -28,4 +28,7 @@ -keep class com.android.systemui.plugins.** { *; } +-keep class com.android.systemui.fragments.FragmentService$FragmentCreator { + *; +} -keep class androidx.core.app.CoreComponentFactory diff --git a/packages/SystemUI/src/com/android/systemui/DependencyProvider.java b/packages/SystemUI/src/com/android/systemui/DependencyProvider.java index e828b2346b81a..76336bb8127f9 100644 --- a/packages/SystemUI/src/com/android/systemui/DependencyProvider.java +++ b/packages/SystemUI/src/com/android/systemui/DependencyProvider.java @@ -44,7 +44,6 @@ import com.android.settingslib.bluetooth.LocalBluetoothManager; import com.android.systemui.appops.AppOpsController; import com.android.systemui.appops.AppOpsControllerImpl; import com.android.systemui.colorextraction.SysuiColorExtractor; -import com.android.systemui.fragments.FragmentService; import com.android.systemui.keyguard.ScreenLifecycle; import com.android.systemui.keyguard.WakefulnessLifecycle; import com.android.systemui.plugins.ActivityStarter; @@ -403,12 +402,6 @@ public class DependencyProvider { return new WakefulnessLifecycle(); } - @Singleton - @Provides - public FragmentService provideFragmentService() { - return new FragmentService(); - } - @Singleton @Provides public ExtensionController provideExtensionController(Context context) { diff --git a/packages/SystemUI/src/com/android/systemui/SystemUIFactory.java b/packages/SystemUI/src/com/android/systemui/SystemUIFactory.java index 9bc91eef52d0a..f63b7a1493cd2 100644 --- a/packages/SystemUI/src/com/android/systemui/SystemUIFactory.java +++ b/packages/SystemUI/src/com/android/systemui/SystemUIFactory.java @@ -30,6 +30,7 @@ import com.android.internal.widget.LockPatternUtils; import com.android.keyguard.ViewMediatorCallback; import com.android.systemui.assist.AssistManager; import com.android.systemui.classifier.FalsingManager; +import com.android.systemui.fragments.FragmentService; import com.android.systemui.keyguard.DismissCallbackRegistry; import com.android.systemui.power.EnhancedEstimates; import com.android.systemui.power.EnhancedEstimatesImpl; @@ -215,5 +216,11 @@ public class SystemUIFactory { public interface SystemUIRootComponent { @Singleton Dependency.DependencyInjector createDependency(); + + /** + * FragmentCreator generates all Fragments that need injection. + */ + @Singleton + FragmentService.FragmentCreator createFragmentCreator(); } } diff --git a/packages/SystemUI/src/com/android/systemui/fragments/FragmentHostManager.java b/packages/SystemUI/src/com/android/systemui/fragments/FragmentHostManager.java index 60e39b21680bb..30f6e1affe0a0 100644 --- a/packages/SystemUI/src/com/android/systemui/fragments/FragmentHostManager.java +++ b/packages/SystemUI/src/com/android/systemui/fragments/FragmentHostManager.java @@ -41,6 +41,8 @@ import com.android.systemui.util.leak.LeakDetector; import java.io.FileDescriptor; import java.io.PrintWriter; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; import java.util.ArrayList; import java.util.HashMap; @@ -186,6 +188,13 @@ public class FragmentHostManager { mFragments.dispatchDestroy(); } + /** + * Creates a fragment that requires injection. + */ + public T create(Class fragmentCls) { + return (T) mPlugins.instantiate(mContext, fragmentCls.getName(), null); + } + public interface FragmentListener { void onFragmentViewCreated(String tag, Fragment fragment); @@ -294,13 +303,36 @@ public class FragmentHostManager { Fragment instantiate(Context context, String className, Bundle arguments) { Context extensionContext = mExtensionLookup.get(className); if (extensionContext != null) { - Fragment f = Fragment.instantiate(extensionContext, className, arguments); + Fragment f = instantiateWithInjections(extensionContext, className, arguments); if (f instanceof Plugin) { ((Plugin) f).onCreate(mContext, extensionContext); } return f; } - return Fragment.instantiate(context, className, arguments); + return instantiateWithInjections(context, className, arguments); + } + + private Fragment instantiateWithInjections(Context context, String className, + Bundle args) { + Method method = mManager.getInjectionMap().get(className); + if (method != null) { + try { + Fragment f = (Fragment) method.invoke(mManager.getFragmentCreator()); + // Setup the args, taken from Fragment#instantiate. + if (args != null) { + args.setClassLoader(f.getClass().getClassLoader()); + f.setArguments(args); + } + return f; + } catch (IllegalAccessException e) { + throw new Fragment.InstantiationException("Unable to instantiate " + className, + e); + } catch (InvocationTargetException e) { + throw new Fragment.InstantiationException("Unable to instantiate " + className, + e); + } + } + return Fragment.instantiate(context, className, args); } } diff --git a/packages/SystemUI/src/com/android/systemui/fragments/FragmentService.java b/packages/SystemUI/src/com/android/systemui/fragments/FragmentService.java index bf7d629c5d7ae..fa898fd574888 100644 --- a/packages/SystemUI/src/com/android/systemui/fragments/FragmentService.java +++ b/packages/SystemUI/src/com/android/systemui/fragments/FragmentService.java @@ -14,6 +14,7 @@ package com.android.systemui.fragments; +import android.app.Fragment; import android.content.res.Configuration; import android.os.Handler; import android.util.ArrayMap; @@ -21,20 +22,55 @@ import android.view.View; import com.android.systemui.ConfigurationChangedReceiver; import com.android.systemui.Dumpable; +import com.android.systemui.SystemUIFactory; +import com.android.systemui.statusbar.phone.NavigationBarFragment; import java.io.FileDescriptor; import java.io.PrintWriter; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; + +import javax.inject.Inject; +import javax.inject.Singleton; + +import dagger.Subcomponent; /** * Holds a map of root views to FragmentHostStates and generates them as needed. * Also dispatches the configuration changes to all current FragmentHostStates. */ +@Singleton public class FragmentService implements ConfigurationChangedReceiver, Dumpable { private static final String TAG = "FragmentService"; private final ArrayMap mHosts = new ArrayMap<>(); + private final ArrayMap mInjectionMap = new ArrayMap<>(); private final Handler mHandler = new Handler(); + private final FragmentCreator mFragmentCreator; + + @Inject + public FragmentService(SystemUIFactory.SystemUIRootComponent rootComponent) { + mFragmentCreator = rootComponent.createFragmentCreator(); + initInjectionMap(); + } + + ArrayMap getInjectionMap() { + return mInjectionMap; + } + + FragmentCreator getFragmentCreator() { + return mFragmentCreator; + } + + private void initInjectionMap() { + for (Method method : FragmentCreator.class.getDeclaredMethods()) { + if (Fragment.class.isAssignableFrom(method.getReturnType()) + && (method.getModifiers() & Modifier.PUBLIC) != 0) { + mInjectionMap.put(method.getReturnType().getName(), method); + } + } + } public FragmentHostManager getFragmentHostManager(View view) { View root = view.getRootView(); @@ -74,6 +110,17 @@ public class FragmentService implements ConfigurationChangedReceiver, Dumpable { } } + /** + * The subcomponent of dagger that holds all fragments that need injection. + */ + @Subcomponent + public interface FragmentCreator { + /** + * Inject a NavigationBarFragment. + */ + NavigationBarFragment createNavigationBarFragment(); + } + private class FragmentHostState { private final View mView; diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NavigationBarFragment.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NavigationBarFragment.java index 55655d5b62406..2daff2cb18bc9 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NavigationBarFragment.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NavigationBarFragment.java @@ -72,7 +72,6 @@ import androidx.annotation.VisibleForTesting; import com.android.internal.logging.MetricsLogger; import com.android.internal.logging.nano.MetricsProto.MetricsEvent; import com.android.internal.util.LatencyTracker; -import com.android.systemui.Dependency; import com.android.systemui.R; import com.android.systemui.SysUiServiceProvider; import com.android.systemui.assist.AssistManager; @@ -97,6 +96,8 @@ import java.util.List; import java.util.Locale; import java.util.function.Consumer; +import javax.inject.Inject; + /** * Fragment containing the NavigationBarFragment. Contains logic for what happens * on clicks and view states of the nav bar. @@ -111,11 +112,12 @@ public class NavigationBarFragment extends LifecycleFragment implements Callback /** Allow some time inbetween the long press for back and recents. */ private static final int LOCK_TO_APP_GESTURE_TOLERENCE = 200; - private final DeviceProvisionedController mDeviceProvisionedController = - Dependency.get(DeviceProvisionedController.class); + private final AccessibilityManagerWrapper mAccessibilityManagerWrapper; + protected final AssistManager mAssistManager; + private final MetricsLogger mMetricsLogger; + private final DeviceProvisionedController mDeviceProvisionedController; protected NavigationBarView mNavigationBarView = null; - protected AssistManager mAssistManager; private int mNavigationBarWindowState = WINDOW_STATE_SHOWING; @@ -124,7 +126,6 @@ public class NavigationBarFragment extends LifecycleFragment implements Callback private AccessibilityManager mAccessibilityManager; private MagnificationContentObserver mMagnificationObserver; private ContentResolver mContentResolver; - private final MetricsLogger mMetricsLogger = Dependency.get(MetricsLogger.class); private int mDisabledFlags1; private int mDisabledFlags2; @@ -193,6 +194,17 @@ public class NavigationBarFragment extends LifecycleFragment implements Callback } }; + @Inject + public NavigationBarFragment(AccessibilityManagerWrapper accessibilityManagerWrapper, + DeviceProvisionedController deviceProvisionedController, MetricsLogger metricsLogger, + AssistManager assistManager, OverviewProxyService overviewProxyService) { + mAccessibilityManagerWrapper = accessibilityManagerWrapper; + mDeviceProvisionedController = deviceProvisionedController; + mMetricsLogger = metricsLogger; + mAssistManager = assistManager; + mOverviewProxyService = overviewProxyService; + } + // ----- Fragment Lifecycle Callbacks ----- @Override @@ -205,8 +217,6 @@ public class NavigationBarFragment extends LifecycleFragment implements Callback mDivider = SysUiServiceProvider.getComponent(getContext(), Divider.class); mWindowManager = getContext().getSystemService(WindowManager.class); mAccessibilityManager = getContext().getSystemService(AccessibilityManager.class); - Dependency.get(AccessibilityManagerWrapper.class).addCallback( - mAccessibilityListener); mContentResolver = getContext().getContentResolver(); mMagnificationObserver = new MagnificationContentObserver( getContext().getMainThreadHandler()); @@ -218,15 +228,13 @@ public class NavigationBarFragment extends LifecycleFragment implements Callback mDisabledFlags1 = savedInstanceState.getInt(EXTRA_DISABLE_STATE, 0); mDisabledFlags2 = savedInstanceState.getInt(EXTRA_DISABLE2_STATE, 0); } - mAssistManager = Dependency.get(AssistManager.class); - mOverviewProxyService = Dependency.get(OverviewProxyService.class); + mAccessibilityManagerWrapper.addCallback(mAccessibilityListener); } @Override public void onDestroy() { super.onDestroy(); - Dependency.get(AccessibilityManagerWrapper.class).removeCallback( - mAccessibilityListener); + mAccessibilityManagerWrapper.removeCallback(mAccessibilityListener); mContentResolver.unregisterContentObserver(mMagnificationObserver); } @@ -892,7 +900,8 @@ public class NavigationBarFragment extends LifecycleFragment implements Callback if (DEBUG) Log.v(TAG, "addNavigationBar: about to add " + navigationBarView); if (navigationBarView == null) return null; - final NavigationBarFragment fragment = new NavigationBarFragment(); + final NavigationBarFragment fragment = FragmentHostManager.get(navigationBarView) + .create(NavigationBarFragment.class); navigationBarView.addOnAttachStateChangeListener(new View.OnAttachStateChangeListener() { @Override public void onViewAttachedToWindow(View v) { diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NavigationBarFragmentTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NavigationBarFragmentTest.java index 9e2db913aba5d..728723b4094c0 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NavigationBarFragmentTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NavigationBarFragmentTest.java @@ -14,10 +14,13 @@ package com.android.systemui.statusbar.phone; +import static org.junit.Assert.assertNotNull; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import android.app.Fragment; import android.content.Context; +import android.os.Bundle; import android.os.Looper; import android.support.test.filters.SmallTest; import android.testing.AndroidTestingRunner; @@ -27,13 +30,17 @@ import android.view.Display; import android.view.WindowManager; import android.view.accessibility.AccessibilityManager.AccessibilityServicesStateChangeListener; +import com.android.internal.logging.MetricsLogger; import com.android.systemui.Dependency; import com.android.systemui.SysuiBaseFragmentTest; +import com.android.systemui.assist.AssistManager; import com.android.systemui.recents.OverviewProxyService; import com.android.systemui.recents.Recents; import com.android.systemui.stackdivider.Divider; import com.android.systemui.statusbar.CommandQueue; import com.android.systemui.statusbar.policy.AccessibilityManagerWrapper; +import com.android.systemui.statusbar.policy.DeviceProvisionedController; +import com.android.systemui.statusbar.policy.DeviceProvisionedControllerImpl; import org.junit.Before; import org.junit.Test; @@ -44,6 +51,23 @@ import org.junit.runner.RunWith; @SmallTest public class NavigationBarFragmentTest extends SysuiBaseFragmentTest { + private OverviewProxyService mOverviewProxyService = + mDependency.injectMockDependency(OverviewProxyService.class); + private AccessibilityManagerWrapper mAccessibilityWrapper = + new AccessibilityManagerWrapper(mContext) { + Tracker mTracker = mLeakCheck.getTracker("accessibility_manager"); + + @Override + public void addCallback(AccessibilityServicesStateChangeListener listener) { + mTracker.getLeakInfo(listener).addAllocation(new Throwable()); + } + + @Override + public void removeCallback(AccessibilityServicesStateChangeListener listener) { + mTracker.getLeakInfo(listener).clearAllocations(); + } + }; + public NavigationBarFragmentTest() { super(NavigationBarFragment.class); } @@ -54,32 +78,19 @@ public class NavigationBarFragmentTest extends SysuiBaseFragmentTest { @Before public void setup() { - mDependency.injectTestDependency(Dependency.BG_LOOPER, Looper.getMainLooper()); mSysuiContext.putComponent(CommandQueue.class, mock(CommandQueue.class)); mSysuiContext.putComponent(StatusBar.class, mock(StatusBar.class)); mSysuiContext.putComponent(Recents.class, mock(Recents.class)); mSysuiContext.putComponent(Divider.class, mock(Divider.class)); injectLeakCheckedDependencies(ALL_SUPPORTED_CLASSES); - mDependency.injectMockDependency(OverviewProxyService.class); WindowManager windowManager = mock(WindowManager.class); Display defaultDisplay = mContext.getSystemService(WindowManager.class).getDefaultDisplay(); when(windowManager.getDefaultDisplay()).thenReturn( defaultDisplay); mContext.addMockSystemService(Context.WINDOW_SERVICE, windowManager); - Tracker tracker = mLeakCheck.getTracker("accessibility_manager"); - AccessibilityManagerWrapper wrapper = new AccessibilityManagerWrapper(mContext) { - @Override - public void addCallback(AccessibilityServicesStateChangeListener listener) { - tracker.getLeakInfo(listener).addAllocation(new Throwable()); - } - - @Override - public void removeCallback(AccessibilityServicesStateChangeListener listener) { - tracker.getLeakInfo(listener).clearAllocations(); - } - }; - mDependency.injectTestDependency(AccessibilityManagerWrapper.class, wrapper); + mDependency.injectTestDependency(Dependency.BG_LOOPER, Looper.getMainLooper()); + mDependency.injectTestDependency(AccessibilityManagerWrapper.class, mAccessibilityWrapper); } @Test @@ -91,4 +102,15 @@ public class NavigationBarFragmentTest extends SysuiBaseFragmentTest { navigationBarFragment.onHomeLongClick(navigationBarFragment.getView()); } + @Override + protected Fragment instantiate(Context context, String className, Bundle arguments) { + DeviceProvisionedController deviceProvisionedController = + new DeviceProvisionedControllerImpl(context); + assertNotNull(mAccessibilityWrapper); + return new NavigationBarFragment(mAccessibilityWrapper, + deviceProvisionedController, + new MetricsLogger(), + new AssistManager(deviceProvisionedController, mContext), + mOverviewProxyService); + } } diff --git a/tests/testables/src/android/testing/BaseFragmentTest.java b/tests/testables/src/android/testing/BaseFragmentTest.java index 5fa065a9135a2..d18c126a96c1f 100644 --- a/tests/testables/src/android/testing/BaseFragmentTest.java +++ b/tests/testables/src/android/testing/BaseFragmentTest.java @@ -21,7 +21,9 @@ import android.app.Fragment; import android.app.FragmentController; import android.app.FragmentHostCallback; import android.app.FragmentManagerNonConfig; +import android.content.Context; import android.graphics.PixelFormat; +import android.os.Bundle; import android.os.Handler; import android.os.Parcelable; import android.support.test.InstrumentationRegistry; @@ -75,7 +77,7 @@ public abstract class BaseFragmentTest { TestableLooper.get(this).runWithLooper(() -> { mHandler = new Handler(); - mFragment = mCls.newInstance(); + mFragment = instantiate(mContext, mCls.getName(), null); mFragments = FragmentController.createController(new HostCallbacks()); mFragments.attachHost(null); mFragments.getFragmentManager().beginTransaction() @@ -187,6 +189,13 @@ public abstract class BaseFragmentTest { TestableLooper.get(this).processAllMessages(); } + /** + * Method available for override to replace fragment instantiation. + */ + protected Fragment instantiate(Context context, String className, @Nullable Bundle arguments) { + return Fragment.instantiate(context, className, arguments); + } + private View findViewById(int id) { return mView.findViewById(id); } @@ -205,6 +214,11 @@ public abstract class BaseFragmentTest { public void onDump(String prefix, FileDescriptor fd, PrintWriter writer, String[] args) { } + @Override + public Fragment instantiate(Context context, String className, Bundle arguments) { + return BaseFragmentTest.this.instantiate(context, className, arguments); + } + @Override public boolean onShouldSaveFragmentState(Fragment fragment) { return true; // True for now.