diff options
| author | Beth Thibodeau <ethibodeau@google.com> | 2019-05-29 03:14:17 +0000 |
|---|---|---|
| committer | Android (Google) Code Review <android-gerrit@google.com> | 2019-05-29 03:14:17 +0000 |
| commit | 01ed4e27981bcf4fb402e8a3491138fb1dd23367 (patch) | |
| tree | 4095c77aad3dfa997165ce404ec2a0e9c01a4a7d | |
| parent | 67cec194bd3b95fe9bc37aee25856811cb4d41a0 (diff) | |
| parent | 4b05fbcf49873f689e970a33ce8a1b4e3f923127 (diff) | |
Merge "Add logging for media notification seekbar" into qt-dev
3 files changed, 247 insertions, 11 deletions
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/wrapper/NotificationMediaTemplateViewWrapper.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/wrapper/NotificationMediaTemplateViewWrapper.java index 91c43a142db5..ee2dacd67f46 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/wrapper/NotificationMediaTemplateViewWrapper.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/wrapper/NotificationMediaTemplateViewWrapper.java @@ -25,6 +25,7 @@ import android.media.MediaMetadata; import android.media.session.MediaController; import android.media.session.MediaSession; import android.media.session.PlaybackState; +import android.metrics.LogMaker; import android.os.Handler; import android.text.format.DateUtils; import android.util.Log; @@ -35,6 +36,9 @@ import android.widget.SeekBar; import android.widget.TextView; import com.android.internal.R; +import com.android.internal.annotations.VisibleForTesting; +import com.android.internal.logging.MetricsLogger; +import com.android.internal.logging.nano.MetricsProto.MetricsEvent; import com.android.systemui.Dependency; import com.android.systemui.statusbar.NotificationMediaManager; import com.android.systemui.statusbar.TransformableView; @@ -62,8 +66,11 @@ public class NotificationMediaTemplateViewWrapper extends NotificationTemplateVi private NotificationMediaManager mMediaManager; private View mSeekBarView; private Context mContext; + private MetricsLogger mMetricsLogger; - private SeekBar.OnSeekBarChangeListener mSeekListener = new SeekBar.OnSeekBarChangeListener() { + @VisibleForTesting + protected SeekBar.OnSeekBarChangeListener mSeekListener = + new SeekBar.OnSeekBarChangeListener() { @Override public void onProgressChanged(SeekBar seekBar, int progress, boolean fromUser) { } @@ -76,6 +83,7 @@ public class NotificationMediaTemplateViewWrapper extends NotificationTemplateVi public void onStopTrackingTouch(SeekBar seekBar) { if (mMediaController != null && canSeekMedia()) { mMediaController.getTransportControls().seekTo(mSeekBar.getProgress()); + mMetricsLogger.write(newLog(MetricsEvent.TYPE_UPDATE)); } } }; @@ -93,7 +101,8 @@ public class NotificationMediaTemplateViewWrapper extends NotificationTemplateVi // Update the UI once, in case playback info changed while we were paused mUpdatePlaybackUi.run(); clearTimer(); - } else if (mSeekBarTimer == null) { + } else if (mSeekBarTimer == null && mSeekBarView != null + && mSeekBarView.getVisibility() != View.GONE) { startTimer(); } } @@ -104,6 +113,7 @@ public class NotificationMediaTemplateViewWrapper extends NotificationTemplateVi super(ctx, view, row); mContext = ctx; mMediaManager = Dependency.get(NotificationMediaManager.class); + mMetricsLogger = Dependency.get(MetricsLogger.class); } private void resolveViews() { @@ -121,11 +131,13 @@ public class NotificationMediaTemplateViewWrapper extends NotificationTemplateVi } // Check for existing media controller and clean up / create as necessary + boolean controllerUpdated = false; if (mMediaController == null || !mMediaController.getSessionToken().equals(token)) { if (mMediaController != null) { mMediaController.unregisterCallback(mMediaCallback); } mMediaController = new MediaController(mContext, token); + controllerUpdated = true; } if (mMediaController.getMetadata() != null) { @@ -134,14 +146,21 @@ public class NotificationMediaTemplateViewWrapper extends NotificationTemplateVi if (duration <= 0) { // Don't include the seekbar if this is a livestream Log.d(TAG, "removing seekbar"); - if (mSeekBarView != null) { + if (mSeekBarView != null && mSeekBarView.getVisibility() != View.GONE) { mSeekBarView.setVisibility(View.GONE); + mMetricsLogger.write(newLog(MetricsEvent.TYPE_CLOSE)); + clearTimer(); + } else if (mSeekBarView == null && controllerUpdated) { + // Only log if the controller changed, otherwise we would log multiple times for + // the same notification when user pauses/resumes + mMetricsLogger.write(newLog(MetricsEvent.TYPE_CLOSE)); } return; } else { // Otherwise, make sure the seekbar is visible - if (mSeekBarView != null) { + if (mSeekBarView != null && mSeekBarView.getVisibility() == View.GONE) { mSeekBarView.setVisibility(View.VISIBLE); + mMetricsLogger.write(newLog(MetricsEvent.TYPE_OPEN)); } } } @@ -153,6 +172,7 @@ public class NotificationMediaTemplateViewWrapper extends NotificationTemplateVi stub.setLayoutInflater(layoutInflater); stub.setLayoutResource(R.layout.notification_material_media_seekbar); mSeekBarView = stub.inflate(); + mMetricsLogger.write(newLog(MetricsEvent.TYPE_OPEN)); mSeekBar = mSeekBarView.findViewById(R.id.notification_media_progress_bar); mSeekBar.setOnSeekBarChangeListener(mSeekListener); @@ -161,17 +181,14 @@ public class NotificationMediaTemplateViewWrapper extends NotificationTemplateVi mSeekBarTotalTime = mSeekBarView.findViewById(R.id.notification_media_total_time); if (mSeekBarTimer == null) { - // Disable seeking if it is not supported for this media session - if (!canSeekMedia()) { - mSeekBar.getThumb().setAlpha(0); - mSeekBar.setEnabled(false); + if (canSeekMedia()) { + // Log initial state, since it will not be updated + mMetricsLogger.write(newLog(MetricsEvent.TYPE_DETAIL, 1)); } else { - mSeekBar.getThumb().setAlpha(255); - mSeekBar.setEnabled(true); + setScrubberVisible(false); } startTimer(); - mMediaController.registerCallback(mMediaCallback); } } @@ -209,6 +226,16 @@ public class NotificationMediaTemplateViewWrapper extends NotificationTemplateVi return ((actions & PlaybackState.ACTION_SEEK_TO) != 0); } + private void setScrubberVisible(boolean isVisible) { + if (mSeekBar == null || mSeekBar.isEnabled() == isVisible) { + return; + } + + mSeekBar.getThumb().setAlpha(isVisible ? 255 : 0); + mSeekBar.setEnabled(isVisible); + mMetricsLogger.write(newLog(MetricsEvent.TYPE_DETAIL, isVisible ? 1 : 0)); + } + protected final Runnable mUpdatePlaybackUi = new Runnable() { @Override public void run() { @@ -228,6 +255,9 @@ public class NotificationMediaTemplateViewWrapper extends NotificationTemplateVi mSeekBar.setProgress((int) position); mSeekBarElapsedTime.setText(millisecondsToTimeString(position)); + + // Update scrubber in case available actions have changed + setScrubberVisible(canSeekMedia()); } else { Log.d(TAG, "Controller missing data " + metadata + " " + playbackState); clearTimer(); @@ -293,4 +323,28 @@ public class NotificationMediaTemplateViewWrapper extends NotificationTemplateVi public boolean shouldClipToRounding(boolean topRounded, boolean bottomRounded) { return true; } + + /** + * Returns an initialized LogMaker for logging changes to the seekbar + * @return new LogMaker + */ + private LogMaker newLog(int event) { + String packageName = mRow.getEntry().notification.getPackageName(); + + return new LogMaker(MetricsEvent.MEDIA_NOTIFICATION_SEEKBAR) + .setType(event) + .setPackageName(packageName); + } + + /** + * Returns an initialized LogMaker for logging changes with subtypes + * @return new LogMaker + */ + private LogMaker newLog(int event, int subtype) { + String packageName = mRow.getEntry().notification.getPackageName(); + return new LogMaker(MetricsEvent.MEDIA_NOTIFICATION_SEEKBAR) + .setType(event) + .setSubtype(subtype) + .setPackageName(packageName); + } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/wrapper/NotificationMediaTemplateViewWrapperTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/wrapper/NotificationMediaTemplateViewWrapperTest.java new file mode 100644 index 000000000000..df41a8424fda --- /dev/null +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/wrapper/NotificationMediaTemplateViewWrapperTest.java @@ -0,0 +1,173 @@ +/* + * Copyright (C) 2019 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.systemui.statusbar.notification.row.wrapper; + +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.argThat; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +import android.app.Notification; +import android.media.MediaMetadata; +import android.media.session.MediaSession; +import android.media.session.PlaybackState; +import android.testing.AndroidTestingRunner; +import android.testing.TestableLooper; +import android.testing.TestableLooper.RunWithLooper; +import android.view.View; +import android.widget.RemoteViews; +import android.widget.SeekBar; + +import androidx.test.filters.SmallTest; + +import com.android.internal.logging.MetricsLogger; +import com.android.internal.logging.nano.MetricsProto.MetricsEvent; +import com.android.systemui.R; +import com.android.systemui.SysuiTestCase; +import com.android.systemui.statusbar.NotificationTestHelper; +import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; + + +@SmallTest +@RunWith(AndroidTestingRunner.class) +@RunWithLooper +public class NotificationMediaTemplateViewWrapperTest extends SysuiTestCase { + + private ExpandableNotificationRow mRow; + private Notification mNotif; + private View mView; + private NotificationMediaTemplateViewWrapper mWrapper; + + @Mock + private MetricsLogger mMetricsLogger; + + @Before + public void setUp() { + MockitoAnnotations.initMocks(this); + com.android.systemui.util.Assert.sMainLooper = TestableLooper.get(this).getLooper(); + + mDependency.injectTestDependency(MetricsLogger.class, mMetricsLogger); + } + + private void makeTestNotification(long duration, boolean allowSeeking) throws Exception { + Notification.Builder builder = new Notification.Builder(mContext) + .setSmallIcon(R.drawable.ic_person) + .setContentTitle("Title") + .setContentText("Text"); + + MediaMetadata metadata = new MediaMetadata.Builder() + .putLong(MediaMetadata.METADATA_KEY_DURATION, duration) + .build(); + MediaSession session = new MediaSession(mContext, "TEST_CHANNEL"); + session.setMetadata(metadata); + + PlaybackState playbackState = new PlaybackState.Builder() + .setActions(allowSeeking ? PlaybackState.ACTION_SEEK_TO : 0) + .build(); + + session.setPlaybackState(playbackState); + + builder.setStyle(new Notification.MediaStyle() + .setMediaSession(session.getSessionToken()) + ); + + mNotif = builder.build(); + assertTrue(mNotif.hasMediaSession()); + + mRow = new NotificationTestHelper(mContext).createRow(mNotif); + + RemoteViews views = new RemoteViews(mContext.getPackageName(), + com.android.internal.R.layout.notification_template_material_big_media); + mView = views.apply(mContext, null); + mWrapper = new NotificationMediaTemplateViewWrapper(mContext, + mView, mRow); + mWrapper.onContentUpdated(mRow); + } + + @Test + public void testLogging_NoSeekbar() throws Exception { + // Media sessions with duration <= 0 should not include a seekbar + makeTestNotification(0, false); + + verify(mMetricsLogger).write(argThat(logMaker -> + logMaker.getCategory() == MetricsEvent.MEDIA_NOTIFICATION_SEEKBAR + && logMaker.getType() == MetricsEvent.TYPE_CLOSE + )); + + verify(mMetricsLogger, times(0)).write(argThat(logMaker -> + logMaker.getCategory() == MetricsEvent.MEDIA_NOTIFICATION_SEEKBAR + && logMaker.getType() == MetricsEvent.TYPE_OPEN + )); + } + + @Test + public void testLogging_HasSeekbarNoScrubber() throws Exception { + // Media sessions that do not support seeking should have a seekbar, but no scrubber + makeTestNotification(1000, false); + + verify(mMetricsLogger).write(argThat(logMaker -> + logMaker.getCategory() == MetricsEvent.MEDIA_NOTIFICATION_SEEKBAR + && logMaker.getType() == MetricsEvent.TYPE_OPEN + )); + + // Ensure the callback runs at least once + mWrapper.mUpdatePlaybackUi.run(); + + verify(mMetricsLogger).write(argThat(logMaker -> + logMaker.getCategory() == MetricsEvent.MEDIA_NOTIFICATION_SEEKBAR + && logMaker.getType() == MetricsEvent.TYPE_DETAIL + && logMaker.getSubtype() == 0 + )); + } + + @Test + public void testLogging_HasSeekbarAndScrubber() throws Exception { + makeTestNotification(1000, true); + + verify(mMetricsLogger).write(argThat(logMaker -> + logMaker.getCategory() == MetricsEvent.MEDIA_NOTIFICATION_SEEKBAR + && logMaker.getType() == MetricsEvent.TYPE_OPEN + )); + + verify(mMetricsLogger).write(argThat(logMaker -> + logMaker.getCategory() == MetricsEvent.MEDIA_NOTIFICATION_SEEKBAR + && logMaker.getType() == MetricsEvent.TYPE_DETAIL + && logMaker.getSubtype() == 1 + )); + } + + @Test + public void testLogging_UpdateSeekbar() throws Exception { + makeTestNotification(1000, true); + + SeekBar seekbar = mView.findViewById( + com.android.internal.R.id.notification_media_progress_bar); + assertTrue(seekbar != null); + + mWrapper.mSeekListener.onStopTrackingTouch(seekbar); + + verify(mMetricsLogger).write(argThat(logMaker -> + logMaker.getCategory() == MetricsEvent.MEDIA_NOTIFICATION_SEEKBAR + && logMaker.getType() == MetricsEvent.TYPE_UPDATE)); + } +} diff --git a/proto/src/metrics_constants/metrics_constants.proto b/proto/src/metrics_constants/metrics_constants.proto index 30619810a6cb..0f0e6f9fb446 100644 --- a/proto/src/metrics_constants/metrics_constants.proto +++ b/proto/src/metrics_constants/metrics_constants.proto @@ -7379,6 +7379,15 @@ message MetricsEvent { // Custom tag for NotificationItem. Hash of the NAS that made adjustments. FIELD_NOTIFICATION_ASSISTANT_SERVICE_HASH = 1742; + // Report interactions with seekbar on media notifications + // OPEN: Seekbar is visible + // CLOSE: Seekbar is not visible + // DETAIL: Seekbar scrubber enabled / disabled + // Subtype: 0 disabled, cannot seek; 1 enabled, can seek + // UPDATE: Scrubber was moved by user + // CATEGORY: NOTIFICATION + MEDIA_NOTIFICATION_SEEKBAR = 1743; + // ---- End Q Constants, all Q constants go above this line ---- // Add new aosp constants above this line. // END OF AOSP CONSTANTS |
