From e33b1bf4e90c30be00325abf046f230edb6475bd Mon Sep 17 00:00:00 2001 From: Adam He Date: Thu, 6 May 2021 14:31:16 -0700 Subject: [PATCH] Fix potential null error in clearEvents. * This is follow up from previous patch to fix an unresolved comment. * We're still changing the clearEvents mechanic to not null the list but actually clear the list of events. We also change when the flush->clearEvents call happen during the destroy process so it occurs on the handlerThread when each session does its own cleanup. Bug: 185162720 Test: atest CtsContentCaptureServiceTestCases Change-Id: Ide5490903deec8c4bc8aae5dc76b1fadca29dbd0 --- .../contentcapture/ContentCaptureSession.java | 6 +----- .../MainContentCaptureSession.java | 16 ++++++++++++---- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/core/java/android/view/contentcapture/ContentCaptureSession.java b/core/java/android/view/contentcapture/ContentCaptureSession.java index c6332686bf7f2..cc47f09d4e8d0 100644 --- a/core/java/android/view/contentcapture/ContentCaptureSession.java +++ b/core/java/android/view/contentcapture/ContentCaptureSession.java @@ -341,11 +341,7 @@ public abstract class ContentCaptureSession implements AutoCloseable { } } - try { - flush(FLUSH_REASON_SESSION_FINISHED); - } finally { - onDestroy(); - } + onDestroy(); } abstract void onDestroy(); diff --git a/core/java/android/view/contentcapture/MainContentCaptureSession.java b/core/java/android/view/contentcapture/MainContentCaptureSession.java index d8ac779ddc275..bcb914208958f 100644 --- a/core/java/android/view/contentcapture/MainContentCaptureSession.java +++ b/core/java/android/view/contentcapture/MainContentCaptureSession.java @@ -263,7 +263,13 @@ public final class MainContentCaptureSession extends ContentCaptureSession { @Override void onDestroy() { mHandler.removeMessages(MSG_FLUSH); - mHandler.post(() -> destroySession()); + mHandler.post(() -> { + try { + flush(FLUSH_REASON_SESSION_FINISHED); + } finally { + destroySession(); + } + }); } /** @@ -571,9 +577,11 @@ public final class MainContentCaptureSession extends ContentCaptureSession { private ParceledListSlice clearEvents() { // NOTE: we must save a reference to the current mEvents and then set it to to null, // otherwise clearing it would clear it in the receiving side if the service is also local. - final List events = mEvents == null - ? Collections.EMPTY_LIST - : new ArrayList<>(mEvents); + if (mEvents == null) { + return new ParceledListSlice<>(Collections.EMPTY_LIST); + } + + final List events = new ArrayList<>(mEvents); mEvents.clear(); return new ParceledListSlice<>(events); }