From ff329c43299db1e45a42bb927713cf17a3436115 Mon Sep 17 00:00:00 2001 From: Makoto Onuki Date: Thu, 5 Jan 2017 11:19:56 -0800 Subject: [PATCH] Bugreport sharing is now work profile aware Now we look at work profile too, if available, when looking for a preferred email address. TODO: The chooser activity should default to work profile too, but it seems to require a lot more work, so not done yet. Test: manual tests Test: adb shell am instrument -w -e class com.android.shell.BugreportProgressServiceTest com.android.shell.tests Bug 30865666 Change-Id: I9b4776d53455a23bfdc4960da25e98bd1dc1b2a1 --- .../shell/BugreportProgressService.java | 78 ++-- packages/Shell/tests/Android.mk | 1 + .../shell/BugreportProgressServiceTest.java | 351 ++++++++++++++++++ 3 files changed, 400 insertions(+), 30 deletions(-) create mode 100644 packages/Shell/tests/src/com/android/shell/BugreportProgressServiceTest.java diff --git a/packages/Shell/src/com/android/shell/BugreportProgressService.java b/packages/Shell/src/com/android/shell/BugreportProgressService.java index 47abd4fc2d22a..26568ccccaf22 100644 --- a/packages/Shell/src/com/android/shell/BugreportProgressService.java +++ b/packages/Shell/src/com/android/shell/BugreportProgressService.java @@ -82,11 +82,14 @@ import android.os.Parcelable; import android.os.RemoteException; import android.os.ServiceManager; import android.os.SystemProperties; +import android.os.UserHandle; +import android.os.UserManager; import android.os.Vibrator; import android.support.v4.content.FileProvider; import android.text.TextUtils; import android.text.format.DateUtils; import android.util.Log; +import android.util.Pair; import android.util.Patterns; import android.util.SparseArray; import android.view.View; @@ -861,9 +864,16 @@ public class BugreportProgressService extends Service { intent.setClipData(clipData); intent.putParcelableArrayListExtra(Intent.EXTRA_STREAM, attachments); - final Account sendToAccount = findSendToAccount(context); + final Pair sendToAccount = findSendToAccount(context, + SystemProperties.get("sendbug.preferred.domain")); if (sendToAccount != null) { - intent.putExtra(Intent.EXTRA_EMAIL, new String[] { sendToAccount.name }); + intent.putExtra(Intent.EXTRA_EMAIL, new String[] { sendToAccount.second.name }); + + // TODO Open the chooser activity on work profile by default. + // If we just use startActivityAsUser(), then the launched app couldn't read + // attachments. + // We probably need to change ChooserActivity to take an extra argument for the + // default profile. } return intent; @@ -1109,44 +1119,52 @@ public class BugreportProgressService extends Service { } /** - * Find the best matching {@link Account} based on build properties. + * Find the best matching {@link Account} based on build properties. If none found, returns + * the first account that looks like an email address. */ - private static Account findSendToAccount(Context context) { - final AccountManager am = (AccountManager) context.getSystemService( - Context.ACCOUNT_SERVICE); + @VisibleForTesting + static Pair findSendToAccount(Context context, String preferredDomain) { + final UserManager um = context.getSystemService(UserManager.class); + final AccountManager am = context.getSystemService(AccountManager.class); - String preferredDomain = SystemProperties.get("sendbug.preferred.domain"); - if (!preferredDomain.startsWith("@")) { + if (preferredDomain != null && !preferredDomain.startsWith("@")) { preferredDomain = "@" + preferredDomain; } - final Account[] accounts; - try { - accounts = am.getAccounts(); - } catch (RuntimeException e) { - Log.e(TAG, "Could not get accounts for preferred domain " + preferredDomain, e); - return null; - } - if (DEBUG) Log.d(TAG, "Number of accounts: " + accounts.length); - Account foundAccount = null; - for (Account account : accounts) { - if (Patterns.EMAIL_ADDRESS.matcher(account.name).matches()) { - if (!preferredDomain.isEmpty()) { - // if we have a preferred domain and it matches, return; otherwise keep - // looking - if (account.name.endsWith(preferredDomain)) { - return account; + Pair first = null; + + for (UserHandle user : um.getUserProfiles()) { + final Account[] accounts; + try { + accounts = am.getAccountsAsUser(user.getIdentifier()); + } catch (RuntimeException e) { + Log.e(TAG, "Could not get accounts for preferred domain " + preferredDomain + + " for user " + user, e); + continue; + } + if (DEBUG) Log.d(TAG, "User: " + user + " Number of accounts: " + accounts.length); + for (Account account : accounts) { + if (Patterns.EMAIL_ADDRESS.matcher(account.name).matches()) { + final Pair candidate = Pair.create(user, account); + + if (!TextUtils.isEmpty(preferredDomain)) { + // if we have a preferred domain and it matches, return; otherwise keep + // looking + if (account.name.endsWith(preferredDomain)) { + return candidate; + } + // if we don't have a preferred domain, just return since it looks like + // an email address } else { - foundAccount = account; + return candidate; + } + if (first == null) { + first = candidate; } - // if we don't have a preferred domain, just return since it looks like - // an email address - } else { - return account; } } } - return foundAccount; + return first; } static Uri getUri(Context context, File file) { diff --git a/packages/Shell/tests/Android.mk b/packages/Shell/tests/Android.mk index 872eb7ac7dcf4..0424eb070474c 100644 --- a/packages/Shell/tests/Android.mk +++ b/packages/Shell/tests/Android.mk @@ -10,6 +10,7 @@ LOCAL_JAVA_LIBRARIES := android.test.runner LOCAL_STATIC_JAVA_LIBRARIES := \ android-support-test \ + mockito-target-minus-junit4 \ ub-uiautomator \ LOCAL_PACKAGE_NAME := ShellTests diff --git a/packages/Shell/tests/src/com/android/shell/BugreportProgressServiceTest.java b/packages/Shell/tests/src/com/android/shell/BugreportProgressServiceTest.java new file mode 100644 index 0000000000000..cef74ae39c60f --- /dev/null +++ b/packages/Shell/tests/src/com/android/shell/BugreportProgressServiceTest.java @@ -0,0 +1,351 @@ +/* + * Copyright (C) 2016 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.android.shell; + +import static com.android.shell.BugreportProgressService.findSendToAccount; + +import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.assertNull; + +import static org.mockito.Matchers.anyInt; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.when; + +import android.accounts.Account; +import android.accounts.AccountManager; +import android.content.Context; +import android.os.UserHandle; +import android.os.UserManager; +import android.support.test.filters.SmallTest; +import android.support.test.runner.AndroidJUnit4; +import android.test.mock.MockContext; +import android.util.Pair; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; + +import java.util.Arrays; +import java.util.List; + +/** + * Test for {@link BugreportProgressServiceTest}. + * + * Usage: + adb shell am instrument -w \ + -e class com.android.shell.BugreportProgressServiceTest \ + com.android.shell.tests + */ +@SmallTest +@RunWith(AndroidJUnit4.class) +public class BugreportProgressServiceTest { + private static class MyContext extends MockContext { + @Mock + public UserManager userManager; + + @Mock + public AccountManager accountManager; + + public MyContext() { + MockitoAnnotations.initMocks(this); + } + + @Override + public Object getSystemService(String name) { + switch (name) { + case Context.USER_SERVICE: + return userManager; + case Context.ACCOUNT_SERVICE: + return accountManager; + default: + return super.getSystemService(name); + } + } + + @Override + public String getSystemServiceName(Class serviceClass) { + if (UserManager.class.equals(serviceClass)) { + return Context.USER_SERVICE; + } + if (AccountManager.class.equals(serviceClass)) { + return Context.ACCOUNT_SERVICE; + } + return super.getSystemServiceName(serviceClass); + } + } + + private final MyContext mTestContext = new MyContext(); + + private static List list(T... array) { + return Arrays.asList(array); + } + + private static T[] array(T... array) { + return array; + } + + private Account account(String email) { + return new Account(email, "test.com"); + } + + private void checkFindSendToAccount( + int expectedUserId, String expectedEmail, String preferredDomain) { + final Pair actual = findSendToAccount(mTestContext, preferredDomain); + assertEquals(UserHandle.of(expectedUserId), actual.first); + assertEquals(account(expectedEmail), actual.second); + } + + @Test + public void findSendToAccount_noWorkProfile() { + when(mTestContext.userManager.getUserProfiles()).thenReturn( + list(UserHandle.of(UserHandle.USER_SYSTEM))); + + // No accounts. + when(mTestContext.accountManager.getAccountsAsUser(eq(UserHandle.USER_SYSTEM))).thenReturn( + array()); + + assertNull(findSendToAccount(mTestContext, null)); + assertNull(findSendToAccount(mTestContext, "")); + assertNull(findSendToAccount(mTestContext, "android.com")); + assertNull(findSendToAccount(mTestContext, "@android.com")); + + // 1 account + when(mTestContext.accountManager.getAccountsAsUser(eq(UserHandle.USER_SYSTEM))).thenReturn( + array(account("abc@gmail.com"))); + + checkFindSendToAccount(0, "abc@gmail.com", null); + checkFindSendToAccount(0, "abc@gmail.com", ""); + checkFindSendToAccount(0, "abc@gmail.com", "android.com"); + checkFindSendToAccount(0, "abc@gmail.com", "@android.com"); + checkFindSendToAccount(0, "abc@gmail.com", "gmail.com"); + checkFindSendToAccount(0, "abc@gmail.com", "@gmail.com"); + + // 2 accounts, same domain + when(mTestContext.accountManager.getAccountsAsUser(eq(UserHandle.USER_SYSTEM))).thenReturn( + array(account("abc@gmail.com"), account("def@gmail.com"))); + + checkFindSendToAccount(0, "abc@gmail.com", null); + checkFindSendToAccount(0, "abc@gmail.com", ""); + checkFindSendToAccount(0, "abc@gmail.com", "android.com"); + checkFindSendToAccount(0, "abc@gmail.com", "@android.com"); + checkFindSendToAccount(0, "abc@gmail.com", "gmail.com"); + checkFindSendToAccount(0, "abc@gmail.com", "@gmail.com"); + + // 2 accounts, different domains + when(mTestContext.accountManager.getAccountsAsUser(eq(UserHandle.USER_SYSTEM))).thenReturn( + array(account("abc@gmail.com"), account("def@android.com"))); + + checkFindSendToAccount(0, "abc@gmail.com", null); + checkFindSendToAccount(0, "abc@gmail.com", ""); + checkFindSendToAccount(0, "def@android.com", "android.com"); + checkFindSendToAccount(0, "def@android.com", "@android.com"); + checkFindSendToAccount(0, "abc@gmail.com", "gmail.com"); + checkFindSendToAccount(0, "abc@gmail.com", "@gmail.com"); + + // Plut an account that doesn't look like an email address. + when(mTestContext.accountManager.getAccountsAsUser(eq(UserHandle.USER_SYSTEM))).thenReturn( + array(account("notemail"), account("abc@gmail.com"), account("def@android.com"))); + + checkFindSendToAccount(0, "abc@gmail.com", null); + checkFindSendToAccount(0, "abc@gmail.com", ""); + checkFindSendToAccount(0, "def@android.com", "android.com"); + checkFindSendToAccount(0, "def@android.com", "@android.com"); + checkFindSendToAccount(0, "abc@gmail.com", "gmail.com"); + checkFindSendToAccount(0, "abc@gmail.com", "@gmail.com"); + } + + /** + * Same as {@link #findSendToAccount_noWorkProfile()}, but with work profile, which has no + * accounts. The expected results are the same as the original. + */ + @Test + public void findSendToAccount_withWorkProfile_noAccounts() { + when(mTestContext.userManager.getUserProfiles()).thenReturn( + list(UserHandle.of(UserHandle.USER_SYSTEM), UserHandle.of(10))); + + // Work profile has no accounts + when(mTestContext.accountManager.getAccountsAsUser(eq(10))).thenReturn( + array()); + + // No accounts. + when(mTestContext.accountManager.getAccountsAsUser(eq(UserHandle.USER_SYSTEM))).thenReturn( + array()); + + assertNull(findSendToAccount(mTestContext, null)); + assertNull(findSendToAccount(mTestContext, "")); + assertNull(findSendToAccount(mTestContext, "android.com")); + assertNull(findSendToAccount(mTestContext, "@android.com")); + + // 1 account + when(mTestContext.accountManager.getAccountsAsUser(eq(UserHandle.USER_SYSTEM))).thenReturn( + array(account("abc@gmail.com"))); + + checkFindSendToAccount(0, "abc@gmail.com", null); + checkFindSendToAccount(0, "abc@gmail.com", ""); + checkFindSendToAccount(0, "abc@gmail.com", "android.com"); + checkFindSendToAccount(0, "abc@gmail.com", "@android.com"); + checkFindSendToAccount(0, "abc@gmail.com", "gmail.com"); + checkFindSendToAccount(0, "abc@gmail.com", "@gmail.com"); + + // 2 accounts, same domain + when(mTestContext.accountManager.getAccountsAsUser(eq(UserHandle.USER_SYSTEM))).thenReturn( + array(account("abc@gmail.com"), account("def@gmail.com"))); + + checkFindSendToAccount(0, "abc@gmail.com", null); + checkFindSendToAccount(0, "abc@gmail.com", ""); + checkFindSendToAccount(0, "abc@gmail.com", "android.com"); + checkFindSendToAccount(0, "abc@gmail.com", "@android.com"); + checkFindSendToAccount(0, "abc@gmail.com", "gmail.com"); + checkFindSendToAccount(0, "abc@gmail.com", "@gmail.com"); + + // 2 accounts, different domains + when(mTestContext.accountManager.getAccountsAsUser(eq(UserHandle.USER_SYSTEM))).thenReturn( + array(account("abc@gmail.com"), account("def@android.com"))); + + checkFindSendToAccount(0, "abc@gmail.com", null); + checkFindSendToAccount(0, "abc@gmail.com", ""); + checkFindSendToAccount(0, "def@android.com", "android.com"); + checkFindSendToAccount(0, "def@android.com", "@android.com"); + checkFindSendToAccount(0, "abc@gmail.com", "gmail.com"); + checkFindSendToAccount(0, "abc@gmail.com", "@gmail.com"); + + // Plut an account that doesn't look like an email address. + when(mTestContext.accountManager.getAccountsAsUser(eq(UserHandle.USER_SYSTEM))).thenReturn( + array(account("notemail"), account("abc@gmail.com"), account("def@android.com"))); + + checkFindSendToAccount(0, "abc@gmail.com", null); + checkFindSendToAccount(0, "abc@gmail.com", ""); + checkFindSendToAccount(0, "def@android.com", "android.com"); + checkFindSendToAccount(0, "def@android.com", "@android.com"); + checkFindSendToAccount(0, "abc@gmail.com", "gmail.com"); + checkFindSendToAccount(0, "abc@gmail.com", "@gmail.com"); + } + + /** + * Same as {@link #findSendToAccount_noWorkProfile()}, but with work profile, which has + * 1 account. The expected results are the same as the original, expect for the "no accounts + * on the primary profile" case. + */ + @Test + public void findSendToAccount_withWorkProfile_1account() { + when(mTestContext.userManager.getUserProfiles()).thenReturn( + list(UserHandle.of(UserHandle.USER_SYSTEM), UserHandle.of(10))); + + // Work profile has no accounts + when(mTestContext.accountManager.getAccountsAsUser(eq(10))).thenReturn( + array(account("xyz@gmail.com"))); + + // No accounts. + when(mTestContext.accountManager.getAccountsAsUser(eq(UserHandle.USER_SYSTEM))).thenReturn( + array()); + + checkFindSendToAccount(10, "xyz@gmail.com", null); + checkFindSendToAccount(10, "xyz@gmail.com", ""); + checkFindSendToAccount(10, "xyz@gmail.com", "android.com"); + checkFindSendToAccount(10, "xyz@gmail.com", "@android.com"); + + // 1 account + when(mTestContext.accountManager.getAccountsAsUser(eq(UserHandle.USER_SYSTEM))).thenReturn( + array(account("abc@gmail.com"))); + + checkFindSendToAccount(0, "abc@gmail.com", null); + checkFindSendToAccount(0, "abc@gmail.com", ""); + checkFindSendToAccount(0, "abc@gmail.com", "android.com"); + checkFindSendToAccount(0, "abc@gmail.com", "@android.com"); + checkFindSendToAccount(0, "abc@gmail.com", "gmail.com"); + checkFindSendToAccount(0, "abc@gmail.com", "@gmail.com"); + + // 2 accounts, same domain + when(mTestContext.accountManager.getAccountsAsUser(eq(UserHandle.USER_SYSTEM))).thenReturn( + array(account("abc@gmail.com"), account("def@gmail.com"))); + + checkFindSendToAccount(0, "abc@gmail.com", null); + checkFindSendToAccount(0, "abc@gmail.com", ""); + checkFindSendToAccount(0, "abc@gmail.com", "android.com"); + checkFindSendToAccount(0, "abc@gmail.com", "@android.com"); + checkFindSendToAccount(0, "abc@gmail.com", "gmail.com"); + checkFindSendToAccount(0, "abc@gmail.com", "@gmail.com"); + + // 2 accounts, different domains + when(mTestContext.accountManager.getAccountsAsUser(eq(UserHandle.USER_SYSTEM))).thenReturn( + array(account("abc@gmail.com"), account("def@android.com"))); + + checkFindSendToAccount(0, "abc@gmail.com", null); + checkFindSendToAccount(0, "abc@gmail.com", ""); + checkFindSendToAccount(0, "def@android.com", "android.com"); + checkFindSendToAccount(0, "def@android.com", "@android.com"); + checkFindSendToAccount(0, "abc@gmail.com", "gmail.com"); + checkFindSendToAccount(0, "abc@gmail.com", "@gmail.com"); + + // Plut an account that doesn't look like an email address. + when(mTestContext.accountManager.getAccountsAsUser(eq(UserHandle.USER_SYSTEM))).thenReturn( + array(account("notemail"), account("abc@gmail.com"), account("def@android.com"))); + + checkFindSendToAccount(0, "abc@gmail.com", null); + checkFindSendToAccount(0, "abc@gmail.com", ""); + checkFindSendToAccount(0, "def@android.com", "android.com"); + checkFindSendToAccount(0, "def@android.com", "@android.com"); + checkFindSendToAccount(0, "abc@gmail.com", "gmail.com"); + checkFindSendToAccount(0, "abc@gmail.com", "@gmail.com"); + } + + /** + * Same as {@link #findSendToAccount_noWorkProfile()}, but with work profile, with mixed + * domains. + */ + @Test + public void findSendToAccount_withWorkProfile_mixedDomains() { + when(mTestContext.userManager.getUserProfiles()).thenReturn( + list(UserHandle.of(UserHandle.USER_SYSTEM), UserHandle.of(10))); + + // Work profile has no accounts + when(mTestContext.accountManager.getAccountsAsUser(eq(10))).thenReturn( + array(account("xyz@android.com"))); + + // No accounts. + when(mTestContext.accountManager.getAccountsAsUser(eq(UserHandle.USER_SYSTEM))).thenReturn( + array()); + + checkFindSendToAccount(10, "xyz@android.com", null); + checkFindSendToAccount(10, "xyz@android.com", ""); + checkFindSendToAccount(10, "xyz@android.com", "android.com"); + checkFindSendToAccount(10, "xyz@android.com", "@android.com"); + + // 1 account + when(mTestContext.accountManager.getAccountsAsUser(eq(UserHandle.USER_SYSTEM))).thenReturn( + array(account("abc@gmail.com"))); + + checkFindSendToAccount(0, "abc@gmail.com", null); + checkFindSendToAccount(0, "abc@gmail.com", ""); + checkFindSendToAccount(10, "xyz@android.com", "android.com"); + checkFindSendToAccount(10, "xyz@android.com", "@android.com"); + checkFindSendToAccount(0, "abc@gmail.com", "gmail.com"); + checkFindSendToAccount(0, "abc@gmail.com", "@gmail.com"); + + // more accounts. + when(mTestContext.accountManager.getAccountsAsUser(eq(UserHandle.USER_SYSTEM))).thenReturn( + array(account("abc@gmail.com"), account("def@gmail.com"))); + + checkFindSendToAccount(0, "abc@gmail.com", null); + checkFindSendToAccount(0, "abc@gmail.com", ""); + checkFindSendToAccount(10, "xyz@android.com", "android.com"); + checkFindSendToAccount(10, "xyz@android.com", "@android.com"); + checkFindSendToAccount(0, "abc@gmail.com", "gmail.com"); + checkFindSendToAccount(0, "abc@gmail.com", "@gmail.com"); + } +}