diff --git a/src/com/android/settings/core/instrumentation/MetricsFactory.java b/src/com/android/settings/core/instrumentation/MetricsFeatureProvider.java similarity index 54% rename from src/com/android/settings/core/instrumentation/MetricsFactory.java rename to src/com/android/settings/core/instrumentation/MetricsFeatureProvider.java index 8ea6d7005ef..1a9a451d3eb 100644 --- a/src/com/android/settings/core/instrumentation/MetricsFactory.java +++ b/src/com/android/settings/core/instrumentation/MetricsFeatureProvider.java @@ -13,33 +13,10 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package com.android.settings.core.instrumentation; -import android.support.annotation.VisibleForTesting; - -public class MetricsFactory { - - private static MetricsFactory sInstance; - - private LogWriter mLogger; - - public static MetricsFactory get() { - if (sInstance == null) { - sInstance = new MetricsFactory(); - } - return sInstance; - } - - public LogWriter getLogger() { - if (mLogger == null) { - mLogger = new EventLogWriter(); - } - return mLogger; - } - - @VisibleForTesting - void setLogger(LogWriter logger) { - mLogger = logger; - } +/** + * FeatureProvider for metrics. + */ +public interface MetricsFeatureProvider extends LogWriter { } diff --git a/src/com/android/settings/core/instrumentation/MetricsFeatureProviderImpl.java b/src/com/android/settings/core/instrumentation/MetricsFeatureProviderImpl.java new file mode 100644 index 00000000000..e4492befbe4 --- /dev/null +++ b/src/com/android/settings/core/instrumentation/MetricsFeatureProviderImpl.java @@ -0,0 +1,100 @@ +/* + * Copyright (C) 2016 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.android.settings.core.instrumentation; + +import android.content.Context; +import android.support.annotation.VisibleForTesting; + +import java.util.ArrayList; +import java.util.List; + +/** + * Implementation for {@link MetricsFeatureProvider} + */ +public class MetricsFeatureProviderImpl implements MetricsFeatureProvider { + + private List mLoggerWriters; + + public MetricsFeatureProviderImpl() { + mLoggerWriters = new ArrayList<>(); + installLogWriters(); + } + + protected void installLogWriters() { + mLoggerWriters.add(new EventLogWriter()); + } + + @Override + public void visible(Context context, int category) { + for (LogWriter writer : mLoggerWriters) { + writer.visible(context, category); + } + } + + @Override + public void hidden(Context context, int category) { + for (LogWriter writer : mLoggerWriters) { + writer.hidden(context, category); + } + } + + @Override + public void action(Context context, int category) { + for (LogWriter writer : mLoggerWriters) { + writer.action(context, category); + } + } + + @Override + public void action(Context context, int category, int value) { + for (LogWriter writer : mLoggerWriters) { + writer.action(context, category, value); + } + } + + @Override + public void action(Context context, int category, boolean value) { + for (LogWriter writer : mLoggerWriters) { + writer.action(context, category, value); + } + } + + @Override + public void action(Context context, int category, String pkg) { + for (LogWriter writer : mLoggerWriters) { + writer.action(context, category, pkg); + } + } + + @Override + public void count(Context context, String name, int value) { + for (LogWriter writer : mLoggerWriters) { + writer.count(context, name, value); + } + } + + @Override + public void histogram(Context context, String name, int bucket) { + for (LogWriter writer : mLoggerWriters) { + writer.histogram(context, name, bucket); + } + } + + @VisibleForTesting + public void addLogWriter(LogWriter logWriter) { + mLoggerWriters.add(logWriter); + } +} diff --git a/src/com/android/settings/core/instrumentation/SharedPreferencesLogger.java b/src/com/android/settings/core/instrumentation/SharedPreferencesLogger.java index 594ab96a2c1..6a67e45477d 100644 --- a/src/com/android/settings/core/instrumentation/SharedPreferencesLogger.java +++ b/src/com/android/settings/core/instrumentation/SharedPreferencesLogger.java @@ -23,6 +23,7 @@ import android.os.AsyncTask; import android.text.TextUtils; import com.android.internal.logging.MetricsProto.MetricsEvent; +import com.android.settings.overlay.FeatureFactory; import java.util.Map; import java.util.Set; @@ -31,12 +32,12 @@ public class SharedPreferencesLogger implements SharedPreferences { private final String mTag; private final Context mContext; - private final LogWriter mLogWriter; + private final MetricsFeatureProvider mMetricsFeature; public SharedPreferencesLogger(Context context, String tag) { mContext = context; mTag = tag; - mLogWriter = MetricsFactory.get().getLogger(); + mMetricsFeature = FeatureFactory.getFactory(context).getMetricsFeatureProvider(); } @Override @@ -95,12 +96,12 @@ public class SharedPreferencesLogger implements SharedPreferences { } private void logValue(String key, String value) { - mLogWriter.count(mContext, mTag + "/" + key + "|" + value, 1); + mMetricsFeature.count(mContext, mTag + "/" + key + "|" + value, 1); } private void logPackageName(String key, String value) { - mLogWriter.count(mContext, mTag + "/" + key, 1); - mLogWriter.action(mContext, MetricsEvent.ACTION_GENERIC_PACKAGE, + mMetricsFeature.count(mContext, mTag + "/" + key, 1); + mMetricsFeature.action(mContext, MetricsEvent.ACTION_GENERIC_PACKAGE, mTag + "/" + key + "|" + value); } diff --git a/src/com/android/settings/core/instrumentation/VisibilityLoggerMixin.java b/src/com/android/settings/core/instrumentation/VisibilityLoggerMixin.java index 28fdf34f583..f4fe5e86cc5 100644 --- a/src/com/android/settings/core/instrumentation/VisibilityLoggerMixin.java +++ b/src/com/android/settings/core/instrumentation/VisibilityLoggerMixin.java @@ -16,34 +16,45 @@ package com.android.settings.core.instrumentation; +import android.content.Context; + import com.android.settings.core.lifecycle.LifecycleObserver; +import com.android.settings.core.lifecycle.events.OnAttach; import com.android.settings.core.lifecycle.events.OnPause; import com.android.settings.core.lifecycle.events.OnResume; +import com.android.settings.overlay.FeatureFactory; /** * Logs visibility change of a fragment. */ -public class VisibilityLoggerMixin implements LifecycleObserver, OnResume, OnPause { +public class VisibilityLoggerMixin implements LifecycleObserver, OnResume, OnPause, OnAttach { private final int mMetricsCategory; - private final LogWriter mLogWriter; + + private MetricsFeatureProvider mMetricsFeature; public VisibilityLoggerMixin(int metricsCategory) { - this(metricsCategory, MetricsFactory.get().getLogger()); + // MetricsFeature will be set during onAttach. + this(metricsCategory, null /* metricsFeature */); } - public VisibilityLoggerMixin(int metricsCategory, LogWriter logWriter) { + public VisibilityLoggerMixin(int metricsCategory, MetricsFeatureProvider metricsFeature) { mMetricsCategory = metricsCategory; - mLogWriter = logWriter; + mMetricsFeature = metricsFeature; + } + + @Override + public void onAttach(Context context) { + mMetricsFeature = FeatureFactory.getFactory(context).getMetricsFeatureProvider(); } @Override public void onResume() { - mLogWriter.visible(null /* context */, mMetricsCategory); + mMetricsFeature.visible(null /* context */, mMetricsCategory); } @Override public void onPause() { - mLogWriter.hidden(null /* context */, mMetricsCategory); + mMetricsFeature.hidden(null /* context */, mMetricsCategory); } } diff --git a/src/com/android/settings/core/lifecycle/Lifecycle.java b/src/com/android/settings/core/lifecycle/Lifecycle.java index 8c60ef4b7cb..c1d6457dd20 100644 --- a/src/com/android/settings/core/lifecycle/Lifecycle.java +++ b/src/com/android/settings/core/lifecycle/Lifecycle.java @@ -16,7 +16,9 @@ package com.android.settings.core.lifecycle; import android.annotation.UiThread; +import android.content.Context; +import com.android.settings.core.lifecycle.events.OnAttach; import com.android.settings.core.lifecycle.events.OnDestroy; import com.android.settings.core.lifecycle.events.OnPause; import com.android.settings.core.lifecycle.events.OnResume; @@ -44,6 +46,14 @@ public class Lifecycle { return observer; } + public void onAttach(Context context) { + for (LifecycleObserver observer : mObservers) { + if (observer instanceof OnAttach) { + ((OnAttach) observer).onAttach(context); + } + } + } + public void onStart() { for (LifecycleObserver observer : mObservers) { if (observer instanceof OnStart) { diff --git a/src/com/android/settings/core/lifecycle/ObservableActivity.java b/src/com/android/settings/core/lifecycle/ObservableActivity.java index 179e3caac2f..bf64c859deb 100644 --- a/src/com/android/settings/core/lifecycle/ObservableActivity.java +++ b/src/com/android/settings/core/lifecycle/ObservableActivity.java @@ -15,7 +15,10 @@ */ package com.android.settings.core.lifecycle; +import android.annotation.Nullable; import android.app.Activity; +import android.os.Bundle; +import android.os.PersistableBundle; /** * {@link Activity} that has hooks to observe activity lifecycle events. @@ -28,6 +31,19 @@ public class ObservableActivity extends Activity { return mLifecycle; } + @Override + protected void onCreate(@Nullable Bundle savedInstanceState) { + mLifecycle.onAttach(this); + super.onCreate(savedInstanceState); + } + + @Override + public void onCreate(@Nullable Bundle savedInstanceState, + @Nullable PersistableBundle persistentState) { + mLifecycle.onAttach(this); + super.onCreate(savedInstanceState, persistentState); + } + @Override protected void onStart() { mLifecycle.onStart(); diff --git a/src/com/android/settings/core/lifecycle/ObservableDialogFragment.java b/src/com/android/settings/core/lifecycle/ObservableDialogFragment.java index 17b41688e9e..7b35ce575b0 100644 --- a/src/com/android/settings/core/lifecycle/ObservableDialogFragment.java +++ b/src/com/android/settings/core/lifecycle/ObservableDialogFragment.java @@ -16,6 +16,7 @@ package com.android.settings.core.lifecycle; import android.app.DialogFragment; +import android.content.Context; /** * {@link DialogFragment} that has hooks to observe fragment lifecycle events. @@ -24,6 +25,12 @@ public class ObservableDialogFragment extends DialogFragment { protected final Lifecycle mLifecycle = new Lifecycle(); + @Override + public void onAttach(Context context) { + super.onAttach(context); + mLifecycle.onAttach(context); + } + @Override public void onStart() { mLifecycle.onStart(); diff --git a/src/com/android/settings/core/lifecycle/ObservablePreferenceFragment.java b/src/com/android/settings/core/lifecycle/ObservablePreferenceFragment.java index f994a8ca31e..dff1d8f76ee 100644 --- a/src/com/android/settings/core/lifecycle/ObservablePreferenceFragment.java +++ b/src/com/android/settings/core/lifecycle/ObservablePreferenceFragment.java @@ -17,6 +17,7 @@ package com.android.settings.core.lifecycle; import android.annotation.CallSuper; +import android.content.Context; import android.support.v14.preference.PreferenceFragment; /** @@ -30,6 +31,13 @@ public abstract class ObservablePreferenceFragment extends PreferenceFragment { return mLifecycle; } + @CallSuper + @Override + public void onAttach(Context context) { + super.onAttach(context); + mLifecycle.onAttach(context); + } + @CallSuper @Override public void onStart() { diff --git a/src/com/android/settings/core/lifecycle/events/OnAttach.java b/src/com/android/settings/core/lifecycle/events/OnAttach.java new file mode 100644 index 00000000000..74fbe2f394f --- /dev/null +++ b/src/com/android/settings/core/lifecycle/events/OnAttach.java @@ -0,0 +1,22 @@ +/* + * Copyright (C) 2016 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.android.settings.core.lifecycle.events; + +import android.content.Context; + +public interface OnAttach { + void onAttach(Context context); +} diff --git a/src/com/android/settings/overlay/FeatureFactory.java b/src/com/android/settings/overlay/FeatureFactory.java index 70d3a6fa59b..83ea2979784 100644 --- a/src/com/android/settings/overlay/FeatureFactory.java +++ b/src/com/android/settings/overlay/FeatureFactory.java @@ -21,6 +21,7 @@ import android.text.TextUtils; import android.util.Log; import com.android.settings.R; +import com.android.settings.core.instrumentation.MetricsFeatureProvider; import com.android.settings.fuelgauge.PowerUsageFeatureProvider; /** @@ -62,6 +63,8 @@ public abstract class FeatureFactory { public abstract SupportFeatureProvider getSupportFeatureProvider(Context context); + public abstract MetricsFeatureProvider getMetricsFeatureProvider(); + public abstract PowerUsageFeatureProvider getPowerUsageFeatureProvider(); public static final class FactoryNotFoundException extends RuntimeException { diff --git a/src/com/android/settings/overlay/FeatureFactoryImpl.java b/src/com/android/settings/overlay/FeatureFactoryImpl.java index b1b3d97227c..035a7f9dc71 100644 --- a/src/com/android/settings/overlay/FeatureFactoryImpl.java +++ b/src/com/android/settings/overlay/FeatureFactoryImpl.java @@ -18,6 +18,9 @@ package com.android.settings.overlay; import android.content.Context; import android.support.annotation.Keep; + +import com.android.settings.core.instrumentation.MetricsFeatureProvider; +import com.android.settings.core.instrumentation.MetricsFeatureProviderImpl; import com.android.settings.fuelgauge.PowerUsageFeatureProvider; /** @@ -26,11 +29,21 @@ import com.android.settings.fuelgauge.PowerUsageFeatureProvider; @Keep public final class FeatureFactoryImpl extends FeatureFactory { + private MetricsFeatureProvider mMetricsFeatureProvider; + @Override public SupportFeatureProvider getSupportFeatureProvider(Context context) { return null; } + @Override + public MetricsFeatureProvider getMetricsFeatureProvider() { + if (mMetricsFeatureProvider == null) { + mMetricsFeatureProvider = new MetricsFeatureProviderImpl(); + } + return mMetricsFeatureProvider; + } + @Override public PowerUsageFeatureProvider getPowerUsageFeatureProvider() { return null; diff --git a/tests/robotests/src/com/android/settings/core/instrumentation/MetricsFactoryTest.java b/tests/robotests/src/com/android/settings/core/instrumentation/MetricsFeatureProviderTest.java similarity index 52% rename from tests/robotests/src/com/android/settings/core/instrumentation/MetricsFactoryTest.java rename to tests/robotests/src/com/android/settings/core/instrumentation/MetricsFeatureProviderTest.java index 552353539b0..973f13fb1fc 100644 --- a/tests/robotests/src/com/android/settings/core/instrumentation/MetricsFactoryTest.java +++ b/tests/robotests/src/com/android/settings/core/instrumentation/MetricsFeatureProviderTest.java @@ -15,31 +15,46 @@ */ package com.android.settings.core.instrumentation; -import com.android.settings.TestConfig; +import android.content.Context; +import com.android.settings.TestConfig; +import com.android.settings.overlay.FeatureFactory; + +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; import org.robolectric.RobolectricTestRunner; import org.robolectric.annotation.Config; +import org.robolectric.shadows.ShadowApplication; import static org.junit.Assert.assertTrue; @RunWith(RobolectricTestRunner.class) @Config(manifest = TestConfig.MANIFEST_PATH, sdk = TestConfig.SDK_VERSION) -public class MetricsFactoryTest { +public class MetricsFeatureProviderTest { - @Test - public void factoryShouldReuseCachedInstance() { - MetricsFactory factory1 = MetricsFactory.get(); - MetricsFactory factory2 = MetricsFactory.get(); - assertTrue(factory1 == factory2); + private ShadowApplication mApplication; + private Context mContext; + + @Mock + private LogWriter mLogWriter; + + @Before + public void setUp() { + MockitoAnnotations.initMocks(this); + mApplication = ShadowApplication.getInstance(); + mContext = mApplication.getApplicationContext(); } @Test - public void factoryShouldCacheLogger() { - MetricsFactory factory = MetricsFactory.get(); - LogWriter logger1 = factory.getLogger(); - LogWriter logger2 = factory.getLogger(); - assertTrue(logger1 == logger2); + public void getFactory_shouldReuseCachedInstance() { + MetricsFeatureProvider feature1 = + FeatureFactory.getFactory(mContext).getMetricsFeatureProvider(); + MetricsFeatureProvider feature2 = + FeatureFactory.getFactory(mContext).getMetricsFeatureProvider(); + + assertTrue(feature1 == feature2); } } diff --git a/tests/robotests/src/com/android/settings/core/instrumentation/SharedPreferenceLoggerTest.java b/tests/robotests/src/com/android/settings/core/instrumentation/SharedPreferenceLoggerTest.java index ccea6f0c3a5..11f2784f545 100644 --- a/tests/robotests/src/com/android/settings/core/instrumentation/SharedPreferenceLoggerTest.java +++ b/tests/robotests/src/com/android/settings/core/instrumentation/SharedPreferenceLoggerTest.java @@ -18,6 +18,7 @@ package com.android.settings.core.instrumentation; import android.content.Context; import com.android.settings.TestConfig; +import com.android.settings.overlay.FeatureFactory; import org.junit.Before; import org.junit.Test; @@ -31,6 +32,7 @@ import org.robolectric.shadows.ShadowApplication; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.anyString; +import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; @RunWith(RobolectricTestRunner.class) @@ -43,6 +45,7 @@ public class SharedPreferenceLoggerTest { @Mock private LogWriter mLogWriter; + private MetricsFeatureProvider mMetricsFeature; private ShadowApplication mApplication; private SharedPreferencesLogger mSharedPrefLogger; @@ -50,7 +53,10 @@ public class SharedPreferenceLoggerTest { public void init() { MockitoAnnotations.initMocks(this); mApplication = ShadowApplication.getInstance(); - MetricsFactory.get().setLogger(mLogWriter); + Context context = mApplication.getApplicationContext(); + mMetricsFeature = FeatureFactory.getFactory(context).getMetricsFeatureProvider(); + ((MetricsFeatureProviderImpl) mMetricsFeature).addLogWriter(mLogWriter); + mSharedPrefLogger = new SharedPreferencesLogger( mApplication.getApplicationContext(), TEST_TAG); } diff --git a/tests/robotests/src/com/android/settings/core/instrumentation/VisibilityLoggerMixinTest.java b/tests/robotests/src/com/android/settings/core/instrumentation/VisibilityLoggerMixinTest.java index 6d2abb3f30f..62800091b8b 100644 --- a/tests/robotests/src/com/android/settings/core/instrumentation/VisibilityLoggerMixinTest.java +++ b/tests/robotests/src/com/android/settings/core/instrumentation/VisibilityLoggerMixinTest.java @@ -38,26 +38,27 @@ import static org.mockito.Mockito.verify; public class VisibilityLoggerMixinTest { @Mock - private EventLogWriter mLogger; + private MetricsFeatureProvider mMetricsFeature; + private VisibilityLoggerMixin mMixin; @Before public void init() { MockitoAnnotations.initMocks(this); - mMixin = new VisibilityLoggerMixin(TestInstrumentable.TEST_METRIC, mLogger); + mMixin = new VisibilityLoggerMixin(TestInstrumentable.TEST_METRIC, mMetricsFeature); } @Test public void shouldLogVisibleOnResume() { mMixin.onResume(); - verify(mLogger, times(1)) + verify(mMetricsFeature, times(1)) .visible(any(Context.class), eq(TestInstrumentable.TEST_METRIC)); } @Test public void shouldLogHideOnPause() { mMixin.onPause(); - verify(mLogger, times(1)) + verify(mMetricsFeature, times(1)) .hidden(any(Context.class), eq(TestInstrumentable.TEST_METRIC)); } diff --git a/tests/robotests/src/com/android/settings/core/lifecycle/LifecycleTest.java b/tests/robotests/src/com/android/settings/core/lifecycle/LifecycleTest.java index 0294fc0777c..9016a779707 100644 --- a/tests/robotests/src/com/android/settings/core/lifecycle/LifecycleTest.java +++ b/tests/robotests/src/com/android/settings/core/lifecycle/LifecycleTest.java @@ -15,7 +15,10 @@ */ package com.android.settings.core.lifecycle; +import android.content.Context; + import com.android.settings.TestConfig; +import com.android.settings.core.lifecycle.events.OnAttach; import com.android.settings.core.lifecycle.events.OnDestroy; import com.android.settings.core.lifecycle.events.OnPause; import com.android.settings.core.lifecycle.events.OnResume; @@ -57,15 +60,23 @@ public class LifecycleTest { } - public static class TestObserver implements LifecycleObserver, OnStart, OnResume, + public static class TestObserver implements LifecycleObserver, OnAttach, OnStart, OnResume, OnPause, OnStop, OnDestroy { + boolean mOnAttachObserved; + boolean mOnAttachHasContext; boolean mOnStartObserved; boolean mOnResumeObserved; boolean mOnPauseObserved; boolean mOnStopObserved; boolean mOnDestroyObserved; + @Override + public void onAttach(Context context) { + mOnAttachObserved = true; + mOnAttachHasContext = context != null; + } + @Override public void onStart() { mOnStartObserved = true; @@ -115,8 +126,10 @@ public class LifecycleTest { Robolectric.buildFragment(TestDialogFragment.class); TestDialogFragment fragment = fragmentController.get(); - fragmentController.create().start().resume().pause().stop().destroy(); + fragmentController.attach().create().start().resume().pause().stop().destroy(); + assertTrue(fragment.mFragObserver.mOnAttachObserved); + assertTrue(fragment.mFragObserver.mOnAttachHasContext); assertTrue(fragment.mFragObserver.mOnStartObserved); assertTrue(fragment.mFragObserver.mOnResumeObserved); assertTrue(fragment.mFragObserver.mOnPauseObserved);