diff options
author | 2020-07-22 16:25:25 -0400 | |
---|---|---|
committer | 2020-07-24 16:17:08 -0400 | |
commit | a0d8ef8a2653d5aa79e481b07b0d2f487a7f764a (patch) | |
tree | 412df616c8b8f3357ba452740c27a20674294929 | |
parent | 5067b5655c0d5ad0051c6b198a746e47f41f481f (diff) |
Streamline Dagger Initization.
This simplifies the dagger initilization, removing constructs that
provided no value to the process, but did serve to make the code
more complex to analyze.
Update dagger.md while we're here.
Fixes: 161911916
Test: manual
Change-Id: I84ced47d7cd6ce90664b68339acb7d7db765d56f
11 files changed, 60 insertions, 145 deletions
diff --git a/packages/CarSystemUI/src/com/android/systemui/CarSystemUIFactory.java b/packages/CarSystemUI/src/com/android/systemui/CarSystemUIFactory.java index 1a1b93b33caf..03ea9418415d 100644 --- a/packages/CarSystemUI/src/com/android/systemui/CarSystemUIFactory.java +++ b/packages/CarSystemUI/src/com/android/systemui/CarSystemUIFactory.java @@ -32,7 +32,7 @@ public class CarSystemUIFactory extends SystemUIFactory { @Override protected SystemUIRootComponent buildSystemUIRootComponent(Context context) { return DaggerCarSystemUIRootComponent.builder() - .contextHolder(new ContextHolder(context)) + .context(context) .build(); } diff --git a/packages/CarSystemUI/src/com/android/systemui/CarSystemUIRootComponent.java b/packages/CarSystemUI/src/com/android/systemui/CarSystemUIRootComponent.java index 088420eefa9e..ece3bee000f9 100644 --- a/packages/CarSystemUI/src/com/android/systemui/CarSystemUIRootComponent.java +++ b/packages/CarSystemUI/src/com/android/systemui/CarSystemUIRootComponent.java @@ -36,12 +36,17 @@ import dagger.Component; DependencyBinder.class, PipModule.class, OneHandedModule.class, - SystemUIFactory.ContextHolder.class, SystemServicesModule.class, SystemUIModule.class, CarSystemUIModule.class, CarSystemUIBinder.class }) public interface CarSystemUIRootComponent extends SystemUIRootComponent { - + /** + * Builder for a CarSystemUIRootComponent. + */ + @Component.Builder + interface Builder extends SystemUIRootComponent.Builder { + CarSystemUIRootComponent build(); + } } diff --git a/packages/SystemUI/docs/dagger.md b/packages/SystemUI/docs/dagger.md index bb68647ceb00..89170139e21c 100644 --- a/packages/SystemUI/docs/dagger.md +++ b/packages/SystemUI/docs/dagger.md @@ -41,8 +41,6 @@ public interface SystemUIRootComponent { The root component is composed of root modules, which in turn provide the global singleton dependencies across all of SystemUI. -- `ContextHolder` is just a wrapper that provides a context. - - `SystemUIFactory` `@Provides` dependencies that need to be overridden by SystemUI variants (like other form factors e.g. Car). @@ -52,41 +50,8 @@ variants (like other form factors e.g. Car). ### Adding injection to a new SystemUI object -Anything that depends on any `@Singleton` provider from SystemUIRootComponent -should be declared as a `@Subcomponent` of the root component. This requires -declaring your own interface for generating your own modules or just the -object you need injected. The subcomponent also needs to be added to -SystemUIRootComponent in SystemUIFactory so it can be acquired. - -```java -public interface SystemUIRootComponent { -+ @Singleton -+ Dependency.DependencyInjector createDependency(); -} - -public class Dependency extends SystemUI { - //... -+ @Subcomponent -+ public interface DependencyInjector { -+ Dependency createSystemUI(); -+ } -} -``` - -For objects which extend SystemUI and require injection, you can define an -injector that creates the injected object for you. This other class should -be referenced in [@string/config_systemUIServiceComponents](packages/SystemUI/res/values/config.xml). - -```java -public static class DependencyCreator implements Injector { - @Override - public SystemUI apply(Context context) { - return SystemUIFactory.getInstance().getRootComponent() - .createDependency() - .createSystemUI(); - } -} -``` +SystemUI object are made injectable by adding an entry in `SystemUIBinder`. SystemUIApplication uses +information in that file to locate and construct an instance of the requested SystemUI class. ### Adding a new injectable object @@ -147,7 +112,7 @@ whenever your fragment needs to be created. ```java public interface FragmentCreator { -+ NavigationBarFragment createNavigationBar(); + NavigationBarFragment createNavigationBar(); } ``` @@ -160,49 +125,17 @@ FragmentHostManager.get(view).create(NavigationBarFragment.class); ### Using injection with Views -Generally, you shouldn't need to inject for a view, as the view should -be relatively self contained and logic that requires injection should be -moved to a higher level construct such as a Fragment or a top-level SystemUI -component, see above for how to do injection for both of which. +DO NOT ADD NEW VIEW INJECTION. VIEW INJECTION IS BEING ACTIVELY DEPRECATED. -Still here? Yeah, ok, sysui has a lot of pre-existing views that contain a -lot of code that could benefit from injection and will need to be migrated -off from Dependency#get uses. Similar to how fragments are injected, the view -needs to be added to the interface -com.android.systemui.util.InjectionInflationController$ViewInstanceCreator. +Needing to inject objects into your View's constructor generally implies you +are doing more work in your presentation layer than is advisable. +Instead, create an injected controller for you view, inject into the +controller, and then attach the view to the controller after inflation. -```java -public interface ViewInstanceCreator { -+ QuickStatusBarHeader createQsHeader(); -} -``` - -Presumably you need to inflate that view from XML (otherwise why do you -need anything special? see earlier sections about generic injection). To obtain -an inflater that supports injected objects, call InjectionInflationController#injectable, -which will wrap the inflater it is passed in one that can create injected -objects when needed. - -```java -@Override -public View onCreateView(LayoutInflater inflater, @Nullable ViewGroup container, - Bundle savedInstanceState) { - return mInjectionInflater.injectable(inflater).inflate(R.layout.my_layout, container, false); -} -``` - -There is one other important thing to note about injecting with views. SysUI -already has a Context in its global dagger component, so if you simply inject -a Context, you will not get the one that the view should have with proper -theming. Because of this, always ensure to tag views that have @Inject with -the @Named view context. - -```java -public CustomView(@Named(VIEW_CONTEXT) Context themedViewContext, AttributeSet attrs, - OtherCustomDependency something) { - //... -} -``` +View injection generally causes headaches while testing, as inflating a view +(which may in turn inflate other views) implicitly causes a Dagger graph to +be stood up, which may or may not contain the appropriately +faked/mocked/stubbed objects. It is a hard to control process. ## Updating Dagger2 diff --git a/packages/SystemUI/src/com/android/systemui/Dependency.java b/packages/SystemUI/src/com/android/systemui/Dependency.java index 4dbb92e1e37f..58f8c07ad7c9 100644 --- a/packages/SystemUI/src/com/android/systemui/Dependency.java +++ b/packages/SystemUI/src/com/android/systemui/Dependency.java @@ -131,9 +131,9 @@ import java.util.function.Consumer; import javax.inject.Inject; import javax.inject.Named; +import javax.inject.Singleton; import dagger.Lazy; -import dagger.Subcomponent; /** * Class to handle ugly dependencies throughout sysui until we determine the @@ -150,6 +150,7 @@ import dagger.Subcomponent; * they have no clients they should not have any registered resources like bound * services, registered receivers, etc. */ +@Singleton public class Dependency { /** * Key for getting a the main looper. @@ -522,7 +523,12 @@ public class Dependency { mProviders.put(RecordingController.class, mRecordingController::get); mProviders.put(Divider.class, mDivider::get); - sDependency = this; + Dependency.setInstance(this); + } + + @VisibleForTesting + public static void setInstance(Dependency dependency) { + sDependency = dependency; } protected final <T> T getDependency(Class<T> cls) { @@ -549,7 +555,7 @@ public class Dependency { } @VisibleForTesting - protected <T> T createDependency(Object cls) { + public <T> T createDependency(Object cls) { Preconditions.checkArgument(cls instanceof DependencyKey<?> || cls instanceof Class<?>); @SuppressWarnings("unchecked") @@ -638,9 +644,4 @@ public class Dependency { return mDisplayName; } } - - @Subcomponent - public interface DependencyInjector { - void createSystemUI(Dependency dependency); - } } diff --git a/packages/SystemUI/src/com/android/systemui/SystemUIFactory.java b/packages/SystemUI/src/com/android/systemui/SystemUIFactory.java index 5674fdd3bb36..1a15c0aae8c1 100644 --- a/packages/SystemUI/src/com/android/systemui/SystemUIFactory.java +++ b/packages/SystemUI/src/com/android/systemui/SystemUIFactory.java @@ -16,7 +16,6 @@ package com.android.systemui; -import android.annotation.NonNull; import android.content.Context; import android.content.res.Resources; import android.os.Handler; @@ -30,7 +29,6 @@ import com.android.keyguard.KeyguardUpdateMonitor; import com.android.keyguard.ViewMediatorCallback; import com.android.systemui.bubbles.BubbleController; import com.android.systemui.dagger.DaggerSystemUIRootComponent; -import com.android.systemui.dagger.DependencyProvider; import com.android.systemui.dagger.SystemUIRootComponent; import com.android.systemui.keyguard.DismissCallbackRegistry; import com.android.systemui.plugins.FalsingManager; @@ -48,9 +46,6 @@ import com.android.systemui.statusbar.policy.KeyguardStateController; import java.util.concurrent.Executor; -import dagger.Module; -import dagger.Provides; - /** * Class factory to provide customizable SystemUI components. */ @@ -97,24 +92,13 @@ public class SystemUIFactory { // Every other part of our codebase currently relies on Dependency, so we // really need to ensure the Dependency gets initialized early on. - - Dependency dependency = new Dependency(); - mRootComponent.createDependency().createSystemUI(dependency); + Dependency dependency = mRootComponent.createDependency(); dependency.start(); } - protected void initWithRootComponent(@NonNull SystemUIRootComponent rootComponent) { - if (mRootComponent != null) { - throw new RuntimeException("Root component can be set only once."); - } - - mRootComponent = rootComponent; - } - protected SystemUIRootComponent buildSystemUIRootComponent(Context context) { return DaggerSystemUIRootComponent.builder() - .dependencyProvider(new DependencyProvider()) - .contextHolder(new ContextHolder(context)) + .context(context) .build(); } @@ -168,18 +152,4 @@ public class SystemUIFactory { Dependency.get(DozeParameters.class), Dependency.get(BubbleController.class)); } - - @Module - public static class ContextHolder { - private Context mContext; - - public ContextHolder(Context context) { - mContext = context; - } - - @Provides - public Context provideContext() { - return mContext; - } - } } diff --git a/packages/SystemUI/src/com/android/systemui/dagger/SystemUIRootComponent.java b/packages/SystemUI/src/com/android/systemui/dagger/SystemUIRootComponent.java index e5cc1384efa4..900c11f0830e 100644 --- a/packages/SystemUI/src/com/android/systemui/dagger/SystemUIRootComponent.java +++ b/packages/SystemUI/src/com/android/systemui/dagger/SystemUIRootComponent.java @@ -19,12 +19,12 @@ package com.android.systemui.dagger; import static com.android.systemui.Dependency.ALLOW_NOTIFICATION_LONG_PRESS_NAME; import android.content.ContentProvider; +import android.content.Context; import com.android.systemui.BootCompleteCacheImpl; import com.android.systemui.Dependency; import com.android.systemui.InitController; import com.android.systemui.SystemUIAppComponentFactory; -import com.android.systemui.SystemUIFactory; import com.android.systemui.dump.DumpManager; import com.android.systemui.fragments.FragmentService; import com.android.systemui.keyguard.KeyguardSliceProvider; @@ -37,6 +37,7 @@ import com.android.systemui.util.InjectionInflationController; import javax.inject.Named; import javax.inject.Singleton; +import dagger.BindsInstance; import dagger.Component; /** @@ -50,13 +51,23 @@ import dagger.Component; OneHandedModule.class, PipModule.class, SystemServicesModule.class, - SystemUIFactory.ContextHolder.class, SystemUIBinder.class, SystemUIModule.class, SystemUIDefaultModule.class}) public interface SystemUIRootComponent { /** + * Builder for a SystemUIRootComponent. + */ + @Component.Builder + interface Builder { + @BindsInstance + Builder context(Context context); + + SystemUIRootComponent build(); + } + + /** * Provides a BootCompleteCache. */ @Singleton @@ -78,7 +89,7 @@ public interface SystemUIRootComponent { * Main dependency providing module. */ @Singleton - Dependency.DependencyInjector createDependency(); + Dependency createDependency(); /** */ @Singleton diff --git a/packages/SystemUI/src/com/android/systemui/tv/TvSystemUIRootComponent.java b/packages/SystemUI/src/com/android/systemui/tv/TvSystemUIRootComponent.java index 2c8297cbfd88..dce38c109930 100644 --- a/packages/SystemUI/src/com/android/systemui/tv/TvSystemUIRootComponent.java +++ b/packages/SystemUI/src/com/android/systemui/tv/TvSystemUIRootComponent.java @@ -16,8 +16,6 @@ package com.android.systemui.tv; -import android.content.Context; - import com.android.systemui.dagger.DefaultComponentBinder; import com.android.systemui.dagger.DependencyBinder; import com.android.systemui.dagger.DependencyProvider; @@ -30,7 +28,6 @@ import com.android.systemui.onehanded.dagger.OneHandedModule; import javax.inject.Singleton; -import dagger.BindsInstance; import dagger.Component; /** @@ -52,9 +49,7 @@ public interface TvSystemUIRootComponent extends SystemUIRootComponent { * Component Builder interface. This allows to bind Context instance in the component */ @Component.Builder - interface Builder { - @BindsInstance Builder context(Context context); - + interface Builder extends SystemUIRootComponent.Builder { TvSystemUIRootComponent build(); } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/DependencyTest.java b/packages/SystemUI/tests/src/com/android/systemui/DependencyTest.java index 475ddc1ea11a..35be496d1a2c 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/DependencyTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/DependencyTest.java @@ -46,9 +46,8 @@ public class DependencyTest extends SysuiTestCase { @Test public void testInitDependency() { Dependency.clearDependencies(); - Dependency dependency = new Dependency(); - SystemUIFactory - .getInstance().getRootComponent().createDependency().createSystemUI(dependency); + Dependency dependency = + SystemUIFactory.getInstance().getRootComponent().createDependency(); dependency.start(); } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/SysuiBaseFragmentTest.java b/packages/SystemUI/tests/src/com/android/systemui/SysuiBaseFragmentTest.java index cf778504190a..3687b4ca7889 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/SysuiBaseFragmentTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/SysuiBaseFragmentTest.java @@ -54,7 +54,10 @@ public abstract class SysuiBaseFragmentTest extends BaseFragmentTest { @Before public void SysuiSetup() { SystemUIFactory.createFromConfig(mContext); - mDependency = new TestableDependency(mContext); + mDependency = new TestableDependency( + SystemUIFactory.getInstance().getRootComponent().createDependency()); + Dependency.setInstance(mDependency); + // TODO: Figure out another way to give reference to a SysuiTestableContext. mSysuiContext = (SysuiTestableContext) mContext; diff --git a/packages/SystemUI/tests/src/com/android/systemui/SysuiTestCase.java b/packages/SystemUI/tests/src/com/android/systemui/SysuiTestCase.java index b6cc2ee03f38..08e27841de03 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/SysuiTestCase.java +++ b/packages/SystemUI/tests/src/com/android/systemui/SysuiTestCase.java @@ -73,7 +73,9 @@ public abstract class SysuiTestCase { @Before public void SysuiSetup() throws Exception { SystemUIFactory.createFromConfig(mContext); - mDependency = new TestableDependency(mContext); + mDependency = new TestableDependency( + SystemUIFactory.getInstance().getRootComponent().createDependency()); + Dependency.setInstance(mDependency); mFakeBroadcastDispatcher = new FakeBroadcastDispatcher(mContext, mock(Looper.class), mock(Executor.class), mock(DumpManager.class), mock(BroadcastDispatcherLogger.class)); diff --git a/packages/SystemUI/tests/src/com/android/systemui/TestableDependency.java b/packages/SystemUI/tests/src/com/android/systemui/TestableDependency.java index a7f4fa5768b4..ee52c7804b69 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/TestableDependency.java +++ b/packages/SystemUI/tests/src/com/android/systemui/TestableDependency.java @@ -16,7 +16,6 @@ package com.android.systemui; import static org.mockito.Mockito.mock; -import android.content.Context; import android.util.ArrayMap; import android.util.ArraySet; import android.util.Log; @@ -26,13 +25,10 @@ public class TestableDependency extends Dependency { private final ArrayMap<Object, Object> mObjs = new ArrayMap<>(); private final ArraySet<Object> mInstantiatedObjects = new ArraySet<>(); + private final Dependency mParent; - public TestableDependency(Context context) { - SystemUIFactory.createFromConfig(context); - SystemUIFactory.getInstance().getRootComponent() - .createDependency() - .createSystemUI(this); - start(); + public TestableDependency(Dependency parent) { + mParent = parent; } public <T> T injectMockDependency(Class<T> cls) { @@ -53,11 +49,11 @@ public class TestableDependency extends Dependency { } @Override - protected <T> T createDependency(Object key) { + public <T> T createDependency(Object key) { if (mObjs.containsKey(key)) return (T) mObjs.get(key); mInstantiatedObjects.add(key); - return super.createDependency(key); + return mParent.createDependency(key); } @Override |