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
2018-12-19 14:17:26 -05:00
parent 27d01a624d
commit 9424af7315
9 changed files with 186 additions and 37 deletions
--- 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
--- 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
--- 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) {
--- 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();
    }
 }
--- 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> T create(Class<T> 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);
        }
    }

--- 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<View, FragmentHostState> mHosts = new ArrayMap<>();
+    private final ArrayMap<String, Method> mInjectionMap = new ArrayMap<>();
    private final Handler mHandler = new Handler();
+    private final FragmentCreator mFragmentCreator;
+
+    @Inject
+    public FragmentService(SystemUIFactory.SystemUIRootComponent rootComponent) {
+        mFragmentCreator = rootComponent.createFragmentCreator();
+        initInjectionMap();
+    }
+
+    ArrayMap<String, Method> 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;

--- 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) {
--- 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);
+    }
 }
--- 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.