From 989c1b3b1c81238d251ae5185dbe3faca5986884 Mon Sep 17 00:00:00 2001 From: Changyeon Jo Date: Tue, 15 Nov 2022 17:09:33 -0800 Subject: Conditionally handle an intent to close the system dialogs CarEvsCameraPreviewApp honors ACTION_CLOSE_SYSTEM_DIALOGS intent if and only if it is triggered by buttons in the car system bar or the home key. Bug: 254738932 Test: Trigger the home button and the system bar buttons to close CarEvsCameraPreviewApp Change-Id: Iec47624ec97ae89323fa13f135371d541c2980c0 Merged-In: Iec47624ec97ae89323fa13f135371d541c2980c0 (cherry picked from commit 01ad52deca21170fc6cf97c5301ea99bb32ca673) --- .../car/evs/CarEvsCameraPreviewActivity.java | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/tests/CarEvsCameraPreviewApp/src/com/google/android/car/evs/CarEvsCameraPreviewActivity.java b/tests/CarEvsCameraPreviewApp/src/com/google/android/car/evs/CarEvsCameraPreviewActivity.java index f5c9337b2e..01c0667213 100644 --- a/tests/CarEvsCameraPreviewApp/src/com/google/android/car/evs/CarEvsCameraPreviewActivity.java +++ b/tests/CarEvsCameraPreviewApp/src/com/google/android/car/evs/CarEvsCameraPreviewActivity.java @@ -50,6 +50,15 @@ import java.util.concurrent.Executors; public class CarEvsCameraPreviewActivity extends Activity { private static final String TAG = CarEvsCameraPreviewActivity.class.getSimpleName(); + /** + * ActivityManagerService encodes the reason for a request to close system dialogs with this + * key. + */ + private final static String EXTRA_DIALOG_CLOSE_REASON = "reason"; + /** This string literal is from com.android.systemui.car.systembar.CarSystemBarButton class. */ + private final static String DIALOG_CLOSE_REASON_CAR_SYSTEMBAR_BUTTON = "carsystembarbutton"; + /** This string literal is from com.android.server.policy.PhoneWindowManager class. */ + private final static String DIALOG_CLOSE_REASON_HOME_KEY = "homekey"; /** * Defines internal states. @@ -199,6 +208,17 @@ public class CarEvsCameraPreviewActivity extends Activity { @Override public void onReceive(Context context, Intent intent) { if (Intent.ACTION_CLOSE_SYSTEM_DIALOGS.equals(intent.getAction())) { + Bundle extras = intent.getExtras(); + if (extras != null) { + String reason = extras.getString(EXTRA_DIALOG_CLOSE_REASON); + if (!DIALOG_CLOSE_REASON_CAR_SYSTEMBAR_BUTTON.equals(reason) && + !DIALOG_CLOSE_REASON_HOME_KEY.equals(reason)) { + Log.i(TAG, "Ignore a request to close the system dialog with a reason = " + + reason); + return; + } + Log.d(TAG, "Requested to close the dialog, reason = " + reason); + } finish(); } else { Log.e(TAG, "Unexpected intent " + intent); @@ -213,7 +233,7 @@ public class CarEvsCameraPreviewActivity extends Activity { // Need to register the receiver for all users, because we want to receive the Intent after // the user is changed. registerReceiverForAllUsers(mBroadcastReceiver, filter, /* broadcastPermission= */ null, - /* scheduler= */ null, Context.RECEIVER_NOT_EXPORTED); + /* scheduler= */ null, Context.RECEIVER_EXPORTED); } @Override -- cgit v1.2.3 From 3da83556dc8db0d703db8e2d7227c11c0b4c7842 Mon Sep 17 00:00:00 2001 From: Howard Date: Thu, 17 Nov 2022 10:58:18 -0800 Subject: Handle NPE for query in ConnectivityPublisher. NetworkStatsService may throw NPE when getUidComplete, we need to handle such cases. Bug: 258519877 Test: atest CarServiceUnitTest:ConnectivityPublisherTest Change-Id: Ie55d1925b903dd5348c714214cc44b7b3f6cb27f Merged-In: Ie55d1925b903dd5348c714214cc44b7b3f6cb27f (cherry picked from commit cbedef222ae7207c12ade9d0142b84f5417d0864) --- .../telemetry/publisher/ConnectivityPublisher.java | 36 ++++++++++++++++------ .../publisher/ConnectivityPublisherTest.java | 27 ++++++++++++++++ 2 files changed, 54 insertions(+), 9 deletions(-) diff --git a/service/src/com/android/car/telemetry/publisher/ConnectivityPublisher.java b/service/src/com/android/car/telemetry/publisher/ConnectivityPublisher.java index f161cd5d42..154932e22f 100644 --- a/service/src/com/android/car/telemetry/publisher/ConnectivityPublisher.java +++ b/service/src/com/android/car/telemetry/publisher/ConnectivityPublisher.java @@ -173,7 +173,11 @@ public class ConnectivityPublisher extends AbstractPublisher { } try { for (QueryParam param : mSubscribers.keySet()) { - mTransportPreviousNetstats.put(param, getSummaryForAllUid(param)); + RefinedStats summary = getSummaryForAllUid(param); + if (summary == null) { + continue; + } + mTransportPreviousNetstats.put(param, summary); } } catch (RemoteException e) { // Can't do much if the NetworkStatsService is not available. Next netstats pull @@ -233,13 +237,17 @@ public class ConnectivityPublisher extends AbstractPublisher { RefinedStats current; try { current = getSummaryForAllUid(param); - } catch (RemoteException | NullPointerException e) { + } catch (RemoteException e) { // If the NetworkStatsService is not available, it retries in the next pull. Slogf.w(CarLog.TAG_TELEMETRY, e); mTraceLog.traceEnd(); return null; } + if (current == null) { + mTraceLog.traceEnd(); + return null; + } // By subtracting, it calculates network usage since the last pull. RefinedStats diff = RefinedStats.subtract(current, previous); @@ -260,7 +268,7 @@ public class ConnectivityPublisher extends AbstractPublisher { * *

TODO(b/218529196): run this method on a separate thread for better performance. */ - @NonNull + @Nullable private RefinedStats getSummaryForAllUid(@NonNull QueryParam param) throws RemoteException { if (DEBUG) { Slogf.d(CarLog.TAG_TELEMETRY, "getSummaryForAllUid " + param); @@ -273,12 +281,22 @@ public class ConnectivityPublisher extends AbstractPublisher { - elapsedMillisSinceBoot - NETSTATS_UID_DEFAULT_BUCKET_DURATION_MILLIS; - NetworkStatsWrapper nonTaggedStats = - mNetworkStatsManager.querySummary( - param.buildNetworkTemplate(), startMillis, currentTimeInMillis); - NetworkStatsWrapper taggedStats = - mNetworkStatsManager.queryTaggedSummary( - param.buildNetworkTemplate(), startMillis, currentTimeInMillis); + NetworkStatsWrapper nonTaggedStats; + NetworkStatsWrapper taggedStats; + // querySummary and queryTaggedSummary may throw NPE propagated from NetworkStatsService + // when its NetworkStatsRecorder failed to initialize and + // NetworkStatsRecorder.getOrLoadCompleteLocked() is called. + try { + nonTaggedStats = + mNetworkStatsManager.querySummary( + param.buildNetworkTemplate(), startMillis, currentTimeInMillis); + taggedStats = + mNetworkStatsManager.queryTaggedSummary( + param.buildNetworkTemplate(), startMillis, currentTimeInMillis); + } catch (NullPointerException e) { + Slogf.w(CarLog.TAG_TELEMETRY, e); + return null; + } RefinedStats result = new RefinedStats(startMillis, currentTimeInMillis); result.addNetworkStats(nonTaggedStats); diff --git a/tests/carservice_unit_test/src/com/android/car/telemetry/publisher/ConnectivityPublisherTest.java b/tests/carservice_unit_test/src/com/android/car/telemetry/publisher/ConnectivityPublisherTest.java index be142d084f..96bb817c0a 100644 --- a/tests/carservice_unit_test/src/com/android/car/telemetry/publisher/ConnectivityPublisherTest.java +++ b/tests/carservice_unit_test/src/com/android/car/telemetry/publisher/ConnectivityPublisherTest.java @@ -528,6 +528,21 @@ public class ConnectivityPublisherTest { .asList().containsExactly(1000L); } + @Test + public void testWhenQueryThrowsNullPointerExceptionIsCaught() { + mFakeManager.setSimulateFailedQuery(true); + mSessionControllerCallbackArgumentCaptor.getValue().onSessionStateChanged( + SESSION_ANNOTATION_BEGIN_1); + + // querySummary gets called for each QueryParam combination, but throws + // NullPointerException each time, which is caught + assertThat(mFakeManager.getMethodCallCount("querySummary")) + .isEqualTo(BASELINE_PULL_COUNT); + // queryTaggedSummary not reached because of previous NullPointerException in querySummary + assertThat(mFakeManager.getMethodCallCount("queryTaggedSummary")) + .isEqualTo(0); + } + private static class FakeDataSubscriber extends DataSubscriber { private final ArrayList mPushedData = new ArrayList<>(); @@ -570,6 +585,8 @@ public class ConnectivityPublisherTest { private final ArrayList mBuckets = new ArrayList<>(); private final HashMap mMethodCallCount = new HashMap<>(); + private boolean mSimulateFailedQuery = false; + private FakeNetworkStatsManager() { super(/* networkStatsManager= */ null); } @@ -624,10 +641,17 @@ public class ConnectivityPublisherTest { return mMethodCallCount.getOrDefault(name, 0); } + public void setSimulateFailedQuery(boolean simulateFailedQuery) { + mSimulateFailedQuery = simulateFailedQuery; + } + @Override @NonNull public NetworkStatsWrapper querySummary(NetworkTemplate template, long start, long end) { increaseMethodCall("querySummary", 1); + if (mSimulateFailedQuery) { + throw new NullPointerException(); + } return commonQuerySummary(false, template, start, end); } @@ -636,6 +660,9 @@ public class ConnectivityPublisherTest { public NetworkStatsWrapper queryTaggedSummary( NetworkTemplate template, long start, long end) { increaseMethodCall("queryTaggedSummary", 1); + if (mSimulateFailedQuery) { + throw new NullPointerException(); + } return commonQuerySummary(true, template, start, end); } -- cgit v1.2.3