diff options
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 |