From 037e34c82390281ce8de55e3da99e45efa4a19a5 Mon Sep 17 00:00:00 2001 From: Christoph Studer Date: Wed, 30 Apr 2014 20:06:04 +0200 Subject: [PATCH] Fix notification visibility reporting Require the screen to be on for visibility reporting. Change-Id: I600e2fa2861bddd41ab9f9f0f381d8b1c4946afa --- .../statusbar/phone/PhoneStatusBar.java | 44 +++++++++++++------ 1 file changed, 31 insertions(+), 13 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/PhoneStatusBar.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/PhoneStatusBar.java index 545352c2a40e8..8ad3bb0e6d651 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/PhoneStatusBar.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/PhoneStatusBar.java @@ -379,7 +379,7 @@ public class PhoneStatusBar extends BaseStatusBar implements DemoMode { private boolean mSettingsStarted; private boolean mSettingsCancelled; private boolean mSettingsClosing; - private int mNotificationPadding; + private boolean mVisible; private final OnChildLocationsChangedListener mOnChildLocationsChangedListener = new OnChildLocationsChangedListener() { @@ -719,8 +719,6 @@ public class PhoneStatusBar extends BaseStatusBar implements DemoMode { } // Quick Settings (where available, some restrictions apply) - mNotificationPadding = mContext.getResources() - .getDimensionPixelSize(R.dimen.notification_side_padding); if (mHasQuickSettings) { // Quick Settings needs a container to survive mSettingsContainer = (QuickSettingsContainerView) @@ -2551,12 +2549,14 @@ public class PhoneStatusBar extends BaseStatusBar implements DemoMode { notifyNavigationBarScreenOn(false); notifyHeadsUpScreenOn(false); finishBarAnimations(); + stopNotificationLogging(); } else if (Intent.ACTION_SCREEN_ON.equals(action)) { mScreenOn = true; // work around problem where mDisplay.getRotation() is not stable while screen is off (bug 7086018) repositionNavigationBar(); notifyNavigationBarScreenOn(true); + startNotificationLoggingIfScreenOnAndVisible(); } else if (ACTION_DEMO.equals(action)) { Bundle bundle = intent.getExtras(); @@ -2730,22 +2730,40 @@ public class PhoneStatusBar extends BaseStatusBar implements DemoMode { @Override protected void visibilityChanged(boolean visible) { + mVisible = visible; if (visible) { - mStackScroller.setChildLocationsChangedListener(mNotificationLocationsChangedListener); + startNotificationLoggingIfScreenOnAndVisible(); } else { - // Report all notifications as invisible and turn down the - // reporter. - if (!mCurrentlyVisibleNotifications.isEmpty()) { - logNotificationVisibilityChanges( - Collections.emptyList(), mCurrentlyVisibleNotifications); - mCurrentlyVisibleNotifications.clear(); - } - mHandler.removeCallbacks(mVisibilityReporter); - mStackScroller.setChildLocationsChangedListener(null); + stopNotificationLogging(); } super.visibilityChanged(visible); } + private void stopNotificationLogging() { + // Report all notifications as invisible and turn down the + // reporter. + if (!mCurrentlyVisibleNotifications.isEmpty()) { + logNotificationVisibilityChanges( + Collections.emptyList(), mCurrentlyVisibleNotifications); + mCurrentlyVisibleNotifications.clear(); + } + mHandler.removeCallbacks(mVisibilityReporter); + mStackScroller.setChildLocationsChangedListener(null); + } + + private void startNotificationLoggingIfScreenOnAndVisible() { + if (mVisible && mScreenOn) { + mStackScroller.setChildLocationsChangedListener(mNotificationLocationsChangedListener); + // Some transitions like mScreenOn=false -> mScreenOn=true don't + // cause the scroller to emit child location events. Hence generate + // one ourselves to guarantee that we're reporting visible + // notifications. + // (Note that in cases where the scroller does emit events, this + // additional event doesn't break anything.) + mNotificationLocationsChangedListener.onChildLocationsChanged(mStackScroller); + } + } + private void logNotificationVisibilityChanges( Collection newlyVisible, Collection noLongerVisible) { if (newlyVisible.isEmpty() && noLongerVisible.isEmpty()) {