From fabd370add9d0a6750abac7083329366ee04a268 Mon Sep 17 00:00:00 2001 From: Neil Fuller Date: Tue, 25 Jul 2017 12:01:03 +0100 Subject: Remove TODO associated with threading Remove TODO associated with threading. Nothing during testing suggests that there is anything more needed. Bug: 31008728 Test: make Change-Id: I874f6d3763d28f58138251ce08aed946895883a7 (cherry picked from commit ded915b9ea8908b1d72426c17003428b82c82d8d) --- src/main/com/android/timezone/updater/RulesCheckReceiver.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/com/android/timezone/updater/RulesCheckReceiver.java b/src/main/com/android/timezone/updater/RulesCheckReceiver.java index b2c30c6..bcb56e2 100644 --- a/src/main/com/android/timezone/updater/RulesCheckReceiver.java +++ b/src/main/com/android/timezone/updater/RulesCheckReceiver.java @@ -69,10 +69,7 @@ import libcore.io.Streams; * server for installation via the * {@link RulesManager#requestInstall(ParcelFileDescriptor, byte[], Callback)}. */ -// TODO(nfuller): Prevent multiple broadcasts being handled at once? // TODO(nfuller): Improve logging -// TODO(nfuller): Make the rules check async? -// TODO(nfuller): Need async generally for SystemService calls from BroadcastReceiver? public class RulesCheckReceiver extends BroadcastReceiver { final static String TAG = "RulesCheckReceiver"; @@ -80,6 +77,9 @@ public class RulesCheckReceiver extends BroadcastReceiver { @Override public void onReceive(Context context, Intent intent) { + // No need to make this synchronized, onReceive() is called on the main thread, there's no + // important object state that could be corrupted and the check token allows for ordering + // issues. if (!RulesUpdaterContract.ACTION_TRIGGER_RULES_UPDATE_CHECK.equals(intent.getAction())) { // Unknown. Do nothing. Log.w(TAG, "Unrecognized intent action received: " + intent -- cgit v1.2.3 From b668857f02d2304068704f6d0c5ecf7316c80c44 Mon Sep 17 00:00:00 2001 From: Neil Fuller Date: Thu, 20 Jul 2017 19:00:59 +0100 Subject: Add event-log logging to the RulesCheckReceiver Add event-log logging to the RulesCheckReceiver. Bug: 31008728 Test: Internal xTS tests Test: adb logcat -b events -v threadtime -v printable -v uid -d *:v Change-Id: I04c412a4765df69cc02edd790ce86a43155200df (cherry picked from commit 2274bbfad06ae5861206c5f6394c2e2133160679) --- Android.mk | 2 ++ .../android/timezone/updater/EventLogTags.logtags | 9 +++++++++ .../timezone/updater/RulesCheckReceiver.java | 23 ++++++++++++++-------- 3 files changed, 26 insertions(+), 8 deletions(-) create mode 100644 src/main/com/android/timezone/updater/EventLogTags.logtags diff --git a/Android.mk b/Android.mk index 568e1d4..08e0e86 100644 --- a/Android.mk +++ b/Android.mk @@ -19,6 +19,8 @@ include $(CLEAR_VARS) LOCAL_MODULE_TAGS := optional LOCAL_MODULE := time_zone_updater LOCAL_SRC_FILES := $(call all-java-files-under, src/main) +# Also include generated EventLogTags from the .logtags file. +LOCAL_SRC_FILES += $(call all-logtags-files-under, src/main) LOCAL_STATIC_JAVA_LIBRARIES := time_zone_distro include $(BUILD_STATIC_JAVA_LIBRARY) diff --git a/src/main/com/android/timezone/updater/EventLogTags.logtags b/src/main/com/android/timezone/updater/EventLogTags.logtags new file mode 100644 index 0000000..9f5342c --- /dev/null +++ b/src/main/com/android/timezone/updater/EventLogTags.logtags @@ -0,0 +1,9 @@ +# See system/core/logcat/event.logtags for a description of the format of this file. + +option java_package com.android.timezone.updater + +51690 timezone_check_trigger_received (token_bytes|3) +51691 timezone_check_read_from_data_app (token_bytes|3) +51692 timezone_check_request_uninstall (token_bytes|3) +51693 timezone_check_request_install (token_bytes|3) +51694 timezone_check_request_nothing (token_bytes|3), (success|1) diff --git a/src/main/com/android/timezone/updater/RulesCheckReceiver.java b/src/main/com/android/timezone/updater/RulesCheckReceiver.java index bcb56e2..92a8bd5 100644 --- a/src/main/com/android/timezone/updater/RulesCheckReceiver.java +++ b/src/main/com/android/timezone/updater/RulesCheckReceiver.java @@ -69,7 +69,6 @@ import libcore.io.Streams; * server for installation via the * {@link RulesManager#requestInstall(ParcelFileDescriptor, byte[], Callback)}. */ -// TODO(nfuller): Improve logging public class RulesCheckReceiver extends BroadcastReceiver { final static String TAG = "RulesCheckReceiver"; @@ -86,10 +85,10 @@ public class RulesCheckReceiver extends BroadcastReceiver { + ", action=" + intent.getAction()); return; } - mRulesManager = (RulesManager) context.getSystemService("timezone"); byte[] token = intent.getByteArrayExtra(RulesUpdaterContract.EXTRA_CHECK_TOKEN); + EventLogTags.writeTimezoneCheckTriggerReceived(Arrays.toString(token)); if (shouldUninstallCurrentInstall(context)) { Log.i(TAG, "Device should be returned to having no time zone distro installed, issuing" @@ -105,7 +104,7 @@ public class RulesCheckReceiver extends BroadcastReceiver { // the data application. // Obtain the information about what the data app is telling us to do. - DistroOperation operation = getOperation(context); + DistroOperation operation = getOperation(context, token); if (operation == null) { Log.w(TAG, "Unable to read time zone operation. Halting check."); boolean success = true; // No point in retrying. @@ -160,7 +159,8 @@ public class RulesCheckReceiver extends BroadcastReceiver { return applicationInfo.isPrivilegedApp() && !applicationInfo.isUpdatedSystemApp(); } - private DistroOperation getOperation(Context context) { + private DistroOperation getOperation(Context context, byte[] tokenBytes) { + EventLogTags.writeTimezoneCheckReadFromDataApp(Arrays.toString(tokenBytes)); Cursor c = context.getContentResolver() .query(TimeZoneRulesDataContract.Operation.CONTENT_URI, new String[] { @@ -205,6 +205,7 @@ public class RulesCheckReceiver extends BroadcastReceiver { RulesState rulesState = mRulesManager.getRulesState(); if (!rulesState.isDistroFormatVersionSupported(distroFormatVersion) || rulesState.isSystemVersionNewerThan(distroRulesVersion)) { + Log.d(TAG, "Candidate distro is not supported or is not better than system version."); // Nothing to do. handleCheckComplete(checkToken, true /* success */); return; @@ -222,6 +223,7 @@ public class RulesCheckReceiver extends BroadcastReceiver { // "seek" the ParcelFileDescriptor it can do so with fewer processes affected. File file = copyDataToLocalFile(context, inputFileDescriptor); if (file == null) { + Log.e(TAG, "Failed to copy distro data to a file."); // It's possible this may get better if the problem is related to storage space so we // signal success := false so it may be retried. boolean success = false; @@ -302,8 +304,10 @@ public class RulesCheckReceiver extends BroadcastReceiver { // Adopt the distroFileDescriptor here so the local file descriptor is closed, whatever the // outcome. try (ParcelFileDescriptor pfd = distroFileDescriptor) { + String tokenString = Arrays.toString(checkToken); + EventLogTags.writeTimezoneCheckRequestInstall(tokenString); int requestStatus = mRulesManager.requestInstall(pfd, checkToken, callback); - Log.i(TAG, "requestInstall() called, token=" + Arrays.toString(checkToken) + Log.i(TAG, "requestInstall() called, token=" + tokenString + ", returned " + requestStatus); } catch (Exception e) { Log.e(TAG, "Error calling requestInstall()", e); @@ -319,8 +323,10 @@ public class RulesCheckReceiver extends BroadcastReceiver { }; try { + String tokenString = Arrays.toString(checkToken); + EventLogTags.writeTimezoneCheckRequestUninstall(tokenString); int requestStatus = mRulesManager.requestUninstall(checkToken, callback); - Log.i(TAG, "requestUninstall() called, token=" + Arrays.toString(checkToken) + Log.i(TAG, "requestUninstall() called, token=" + tokenString + ", returned " + requestStatus); } catch (Exception e) { Log.e(TAG, "Error calling requestUninstall()", e); @@ -329,9 +335,10 @@ public class RulesCheckReceiver extends BroadcastReceiver { private void handleCheckComplete(final byte[] token, final boolean success) { try { + String tokenString = Arrays.toString(token); + EventLogTags.writeTimezoneCheckRequestNothing(tokenString, success ? 1 : 0); mRulesManager.requestNothing(token, success); - Log.i(TAG, "requestNothing() called, token=" + Arrays.toString(token) - + ", success=" + success); + Log.i(TAG, "requestNothing() called, token=" + tokenString + ", success=" + success); } catch (Exception e) { Log.e(TAG, "Error calling requestNothing()", e); } -- cgit v1.2.3