diff --git a/src/com/android/settings/core/InstrumentedFragment.java b/src/com/android/settings/core/InstrumentedFragment.java index 867389b3074..303d4d8498a 100644 --- a/src/com/android/settings/core/InstrumentedFragment.java +++ b/src/com/android/settings/core/InstrumentedFragment.java @@ -29,9 +29,12 @@ public abstract class InstrumentedFragment extends ObservableFragment implements protected MetricsFeatureProvider mMetricsFeatureProvider; + private final VisibilityLoggerMixin mVisibilityLoggerMixin; + public InstrumentedFragment() { // Mixin that logs visibility change for activity. - getLifecycle().addObserver(new VisibilityLoggerMixin(getMetricsCategory())); + mVisibilityLoggerMixin = new VisibilityLoggerMixin(getMetricsCategory()); + getLifecycle().addObserver(mVisibilityLoggerMixin); getLifecycle().addObserver(new SurveyMixin(this, getClass().getSimpleName())); } @@ -40,4 +43,10 @@ public abstract class InstrumentedFragment extends ObservableFragment implements super.onAttach(context); mMetricsFeatureProvider = FeatureFactory.getFactory(context).getMetricsFeatureProvider(); } + + @Override + public void onResume() { + mVisibilityLoggerMixin.setSourceMetricsCategory(getActivity()); + super.onResume(); + } } \ No newline at end of file diff --git a/src/com/android/settings/core/instrumentation/MetricsFeatureProvider.java b/src/com/android/settings/core/instrumentation/MetricsFeatureProvider.java index 66986d68564..b117ea1e455 100644 --- a/src/com/android/settings/core/instrumentation/MetricsFeatureProvider.java +++ b/src/com/android/settings/core/instrumentation/MetricsFeatureProvider.java @@ -15,7 +15,10 @@ */ package com.android.settings.core.instrumentation; +import android.content.ComponentName; import android.content.Context; +import android.content.Intent; +import android.text.TextUtils; import android.util.Pair; import com.android.internal.logging.nano.MetricsProto; @@ -100,4 +103,27 @@ public class MetricsFeatureProvider { } return ((Instrumentable) object).getMetricsCategory(); } + + public void logDashboardStartIntent(Context context, Intent intent, + int sourceMetricsCategory) { + if (intent == null) { + return; + } + final ComponentName cn = intent.getComponent(); + if (cn == null) { + final String action = intent.getAction(); + if (TextUtils.isEmpty(action)) { + // Not loggable + return; + } + action(context, MetricsProto.MetricsEvent.ACTION_SETTINGS_TILE_CLICK, action, + Pair.create(MetricsProto.MetricsEvent.FIELD_CONTEXT, sourceMetricsCategory)); + } else if (TextUtils.equals(cn.getPackageName(), context.getPackageName())) { + // Going to a Setting internal page, skip click logging in favor of page's own + // visibility logging. + return; + } + action(context, MetricsProto.MetricsEvent.ACTION_SETTINGS_TILE_CLICK, cn.flattenToString(), + Pair.create(MetricsProto.MetricsEvent.FIELD_CONTEXT, sourceMetricsCategory)); + } } diff --git a/src/com/android/settings/dashboard/DashboardFeatureProviderImpl.java b/src/com/android/settings/dashboard/DashboardFeatureProviderImpl.java index 0db8c20ec18..1184e893520 100644 --- a/src/com/android/settings/dashboard/DashboardFeatureProviderImpl.java +++ b/src/com/android/settings/dashboard/DashboardFeatureProviderImpl.java @@ -27,10 +27,8 @@ import android.support.v7.preference.Preference; import android.text.TextUtils; import android.util.Log; -import com.android.internal.logging.nano.MetricsProto; import com.android.internal.logging.nano.MetricsProto.MetricsEvent; import com.android.settings.SettingsActivity; -import com.android.settings.SubSettings; import com.android.settings.core.instrumentation.MetricsFeatureProvider; import com.android.settings.overlay.FeatureFactory; import com.android.settingslib.drawer.CategoryManager; @@ -154,7 +152,7 @@ public class DashboardFeatureProviderImpl implements DashboardFeatureProvider { intent.setAction(action); } pref.setOnPreferenceClickListener(preference -> { - launchIntentOrSelectProfile(activity, tile, intent); + launchIntentOrSelectProfile(activity, tile, intent, sourceMetricsCategory); return true; }); } @@ -204,37 +202,20 @@ public class DashboardFeatureProviderImpl implements DashboardFeatureProvider { MetricsEvent.DASHBOARD_SUMMARY) .putExtra(SettingsDrawerActivity.EXTRA_SHOW_MENU, true) .addFlags(Intent.FLAG_ACTIVITY_CLEAR_TASK); - launchIntentOrSelectProfile(activity, tile, intent); + launchIntentOrSelectProfile(activity, tile, intent, MetricsEvent.DASHBOARD_SUMMARY); } - private void launchIntentOrSelectProfile(Activity activity, Tile tile, Intent intent) { + private void launchIntentOrSelectProfile(Activity activity, Tile tile, Intent intent, + int sourceMetricCategory) { ProfileSelectDialog.updateUserHandlesIfNeeded(mContext, tile); if (tile.userHandle == null) { - logStartActivity(intent); + mMetricsFeatureProvider.logDashboardStartIntent(mContext, intent, sourceMetricCategory); activity.startActivityForResult(intent, 0); } else if (tile.userHandle.size() == 1) { - logStartActivity(intent); + mMetricsFeatureProvider.logDashboardStartIntent(mContext, intent, sourceMetricCategory); activity.startActivityForResultAsUser(intent, 0, tile.userHandle.get(0)); } else { ProfileSelectDialog.show(activity.getFragmentManager(), tile); } } - - private void logStartActivity(Intent intent) { - if (intent == null) { - return; - } - final ComponentName cn = intent.getComponent(); - if (cn == null) { - // Not loggable - return; - } else if (TextUtils.equals(cn.getPackageName(), mContext.getPackageName())) { - // Going to a Setting internal page, skip click logging in favor of page's own - // visibility logging. - return; - } - mMetricsFeatureProvider.action(mContext, - MetricsEvent.ACTION_SETTINGS_TILE_CLICK, - cn.flattenToString()); - } } diff --git a/src/com/android/settings/dashboard/DashboardFragment.java b/src/com/android/settings/dashboard/DashboardFragment.java index d650fc703fd..29cee5e9e01 100644 --- a/src/com/android/settings/dashboard/DashboardFragment.java +++ b/src/com/android/settings/dashboard/DashboardFragment.java @@ -157,6 +157,9 @@ public abstract class DashboardFragment extends SettingsPreferenceFragment @Override public boolean onPreferenceTreeClick(Preference preference) { Collection controllers = mPreferenceControllers.values(); + // If preference contains intent, log it before handling. + mMetricsFeatureProvider.logDashboardStartIntent( + getContext(), preference.getIntent(), getMetricsCategory()); // Give all controllers a chance to handle click. for (PreferenceController controller : controllers) { if (controller.handlePreferenceTreeClick(preference)) { diff --git a/tests/robotests/src/com/android/settings/core/instrumentation/MetricsFeatureProviderTest.java b/tests/robotests/src/com/android/settings/core/instrumentation/MetricsFeatureProviderTest.java index 41a2a719d37..f0889027650 100644 --- a/tests/robotests/src/com/android/settings/core/instrumentation/MetricsFeatureProviderTest.java +++ b/tests/robotests/src/com/android/settings/core/instrumentation/MetricsFeatureProviderTest.java @@ -15,35 +15,51 @@ */ package com.android.settings.core.instrumentation; +import android.content.ComponentName; import android.content.Context; +import android.content.Intent; +import android.util.Pair; + +import com.android.internal.logging.nano.MetricsProto.MetricsEvent; import com.android.settings.SettingsRobolectricTestRunner; 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.RuntimeEnvironment; import org.robolectric.annotation.Config; -import org.robolectric.shadows.ShadowApplication; +import org.robolectric.util.ReflectionHelpers; + +import java.util.ArrayList; +import java.util.List; import static com.google.common.truth.Truth.assertThat; +import static org.mockito.Matchers.anyString; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; @RunWith(SettingsRobolectricTestRunner.class) @Config(manifest = TestConfig.MANIFEST_PATH, sdk = TestConfig.SDK_VERSION) public class MetricsFeatureProviderTest { - private ShadowApplication mApplication; - private Context mContext; - @Mock private LogWriter mLogWriter; + private Context mContext; + private MetricsFeatureProvider mProvider; @Before public void setUp() { MockitoAnnotations.initMocks(this); - mApplication = ShadowApplication.getInstance(); - mContext = mApplication.getApplicationContext(); + mContext = RuntimeEnvironment.application; + mProvider = new MetricsFeatureProvider(); + List writers = new ArrayList<>(); + writers.add(mLogWriter); + ReflectionHelpers.setField(mProvider, "mLoggerWriters", writers); } @Test @@ -55,4 +71,25 @@ public class MetricsFeatureProviderTest { assertThat(feature1 == feature2).isTrue(); } + + @Test + public void logDashboardStartIntent_intentEmpty_shouldNotLog() { + mProvider.logDashboardStartIntent(mContext, null /* intent */, + MetricsEvent.SETTINGS_GESTURES); + + verifyNoMoreInteractions(mLogWriter); + } + + @Test + public void logDashboardStartIntent_intentIsExternal_shouldLog() { + final Intent intent = new Intent().setComponent(new ComponentName("pkg", "cls")); + + mProvider.logDashboardStartIntent(mContext, intent, MetricsEvent.SETTINGS_GESTURES); + + verify(mLogWriter).action( + eq(mContext), + eq(MetricsEvent.ACTION_SETTINGS_TILE_CLICK), + anyString(), + eq(Pair.create(MetricsEvent.FIELD_CONTEXT, MetricsEvent.SETTINGS_GESTURES))); + } } diff --git a/tests/robotests/src/com/android/settings/dashboard/DashboardFeatureProviderImplTest.java b/tests/robotests/src/com/android/settings/dashboard/DashboardFeatureProviderImplTest.java index ebe1f0ac92d..8e463432646 100644 --- a/tests/robotests/src/com/android/settings/dashboard/DashboardFeatureProviderImplTest.java +++ b/tests/robotests/src/com/android/settings/dashboard/DashboardFeatureProviderImplTest.java @@ -56,7 +56,6 @@ import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -166,10 +165,11 @@ public class DashboardFeatureProviderImplTest { mImpl.bindPreferenceToTile(mActivity, MetricsProto.MetricsEvent.SETTINGS_GESTURES, preference, tile, "123", Preference.DEFAULT_ORDER); preference.getOnPreferenceClickListener().onPreferenceClick(null); - verify(mFeatureFactory.metricsFeatureProvider).action( + + verify(mFeatureFactory.metricsFeatureProvider).logDashboardStartIntent( any(Context.class), - eq(MetricsProto.MetricsEvent.ACTION_SETTINGS_TILE_CLICK), - eq(tile.intent.getComponent().flattenToString())); + any(Intent.class), + eq(MetricsProto.MetricsEvent.SETTINGS_GESTURES)); verify(mActivity) .startActivityForResultAsUser(any(Intent.class), anyInt(), any(UserHandle.class)); } @@ -193,10 +193,10 @@ public class DashboardFeatureProviderImplTest { mImpl.bindPreferenceToTile(mActivity, MetricsProto.MetricsEvent.SETTINGS_GESTURES, preference, tile, "123", Preference.DEFAULT_ORDER); preference.getOnPreferenceClickListener().onPreferenceClick(null); - verify(mFeatureFactory.metricsFeatureProvider, never()).action( + verify(mFeatureFactory.metricsFeatureProvider).logDashboardStartIntent( any(Context.class), - eq(MetricsProto.MetricsEvent.ACTION_SETTINGS_TILE_CLICK), - eq(tile.intent.getComponent().flattenToString())); + any(Intent.class), + anyInt()); verify(mActivity) .startActivityForResultAsUser(any(Intent.class), anyInt(), any(UserHandle.class)); }