From 4df89bcb31cde334dc317cce705a75b0ff036a8a Mon Sep 17 00:00:00 2001 From: Eugene Susla Date: Tue, 28 Mar 2017 20:27:28 -0700 Subject: [PATCH] [DO NOT MERGE] Fix associations serialization optimization bug There mas a missing defensive copy causing false positive detections of "associations not changed" case, leading to xml file not being updated once at least one record is present Bug: 30932767 Test: Associate at least two different devices and ensure the xml has both. Change-Id: Ic0dc615dd2b847e137555c1084c616831b4dde83 --- .../internal/util/CollectionUtils.java | 25 +++++++++++++++++++ .../print/CompanionDeviceManagerService.java | 21 +++++++++------- 2 files changed, 37 insertions(+), 9 deletions(-) diff --git a/core/java/com/android/internal/util/CollectionUtils.java b/core/java/com/android/internal/util/CollectionUtils.java index 287f68cf5a55..183945ca1382 100644 --- a/core/java/com/android/internal/util/CollectionUtils.java +++ b/core/java/com/android/internal/util/CollectionUtils.java @@ -127,4 +127,29 @@ public class CollectionUtils { } return null; } + + /** + * Similar to {@link List#add}, but with support for list values of {@code null} and + * {@link Collections#emptyList} + */ + public static @NonNull List add(@Nullable List cur, T val) { + if (cur == null || cur == Collections.emptyList()) { + cur = new ArrayList<>(); + } + cur.add(val); + return cur; + } + + /** + * Similar to {@link List#remove}, but with support for list values of {@code null} and + * {@link Collections#emptyList} + */ + public static @NonNull List remove(@Nullable List cur, T val) { + if (cur == null || cur == Collections.emptyList()) { + return Collections.emptyList(); + } + cur.remove(val); + return cur; + } + } diff --git a/services/print/java/com/android/server/print/CompanionDeviceManagerService.java b/services/print/java/com/android/server/print/CompanionDeviceManagerService.java index 2c38f974db74..6b3271ff3467 100644 --- a/services/print/java/com/android/server/print/CompanionDeviceManagerService.java +++ b/services/print/java/com/android/server/print/CompanionDeviceManagerService.java @@ -17,6 +17,7 @@ package com.android.server.print; +import static com.android.internal.util.CollectionUtils.size; import static com.android.internal.util.Preconditions.checkArgument; import static com.android.internal.util.Preconditions.checkNotNull; @@ -221,7 +222,7 @@ public class CompanionDeviceManagerService extends SystemService implements Bind throws RemoteException { checkNotNull(deviceMacAddress); checkCallerIsSystemOr(callingPackage); - updateAssociations(associations -> ArrayUtils.remove(associations, + updateAssociations(associations -> CollectionUtils.remove(associations, new Association(getCallingUserId(), deviceMacAddress, callingPackage))); } @@ -350,22 +351,24 @@ public class CompanionDeviceManagerService extends SystemService implements Bind } private void recordAssociation(String priviledgedPackage, String deviceAddress) { - updateAssociations((associations) -> ArrayUtils.add(associations, + updateAssociations(associations -> CollectionUtils.add(associations, new Association(getCallingUserId(), deviceAddress, priviledgedPackage))); } - private void updateAssociations(Function, List> update) { + private void updateAssociations(Function, List> update) { updateAssociations(update, getCallingUserId()); } - private void updateAssociations(Function, List> update, + private void updateAssociations(Function, List> update, int userId) { final AtomicFile file = getStorageFileForUser(userId); synchronized (file) { - final ArrayList old = readAllAssociations(userId); - final List associations = update.apply(old); - if (Objects.equals(old, associations)) return; + List associations = readAllAssociations(userId); + final ArrayList old = new ArrayList<>(associations); + associations = update.apply(associations); + if (size(old) == size(associations)) return; + List finalAssociations = associations; file.write((out) -> { XmlSerializer xml = Xml.newSerializer(); try { @@ -374,8 +377,8 @@ public class CompanionDeviceManagerService extends SystemService implements Bind xml.startDocument(null, true); xml.startTag(null, XML_TAG_ASSOCIATIONS); - for (int i = 0; i < CollectionUtils.size(associations); i++) { - Association association = associations.get(i); + for (int i = 0; i < size(finalAssociations); i++) { + Association association = finalAssociations.get(i); xml.startTag(null, XML_TAG_ASSOCIATION) .attribute(null, XML_ATTR_PACKAGE, association.companionAppPackage) .attribute(null, XML_ATTR_DEVICE, association.deviceAddress)