From fcca68dfb137c061952d23e1873e995e6bcf172d Mon Sep 17 00:00:00 2001 From: Felipe Leme Date: Tue, 12 Apr 2016 14:00:07 -0700 Subject: [PATCH] Revert warning logic so it has a "don't show again" message. BUG: 28140003 Change-Id: I93e7b1494a0f4c5ca080fbe9dd94dc2168092ffa --- packages/Shell/res/layout/confirm_repeat.xml | 2 +- packages/Shell/res/values/strings.xml | 6 +-- .../src/com/android/shell/BugreportPrefs.java | 16 +++--- .../android/shell/BugreportProgressService.java | 5 +- .../android/shell/BugreportWarningActivity.java | 4 +- .../com/android/shell/BugreportReceiverTest.java | 61 ++++++++++++++++------ 6 files changed, 64 insertions(+), 30 deletions(-) diff --git a/packages/Shell/res/layout/confirm_repeat.xml b/packages/Shell/res/layout/confirm_repeat.xml index d12f467db254..ad90af186c05 100644 --- a/packages/Shell/res/layout/confirm_repeat.xml +++ b/packages/Shell/res/layout/confirm_repeat.xml @@ -38,5 +38,5 @@ android:id="@android:id/checkbox" android:layout_width="match_parent" android:layout_height="wrap_content" - android:text="@string/bugreport_confirm_repeat" /> + android:text="@string/bugreport_confirm_dont_repeat" /> diff --git a/packages/Shell/res/values/strings.xml b/packages/Shell/res/values/strings.xml index 5d901896cc4a..8c0d374bf3a6 100644 --- a/packages/Shell/res/values/strings.xml +++ b/packages/Shell/res/values/strings.xml @@ -37,9 +37,9 @@ Tap to share your bug report without a screenshot or wait for the screenshot to finish - Bug reports contain data from the system\'s various log files, including personal and private information. Only share bug reports with apps and people you trust. - - Show this message next time + Bug reports contain data from the system\'s various log files, which may include data you consider sensitive (such as app-usage and location data). Only share bug reports with people and apps you trust. + + Don\'t show again Bug reports diff --git a/packages/Shell/src/com/android/shell/BugreportPrefs.java b/packages/Shell/src/com/android/shell/BugreportPrefs.java index 3748e89c0598..93690d48cd04 100644 --- a/packages/Shell/src/com/android/shell/BugreportPrefs.java +++ b/packages/Shell/src/com/android/shell/BugreportPrefs.java @@ -22,22 +22,24 @@ import android.content.SharedPreferences; /** * Preferences related to bug reports. */ -public class BugreportPrefs { - private static final String PREFS_BUGREPORT = "bugreports"; +final class BugreportPrefs { + static final String PREFS_BUGREPORT = "bugreports"; private static final String KEY_WARNING_STATE = "warning-state"; - public static final int STATE_UNKNOWN = 0; - public static final int STATE_SHOW = 1; - public static final int STATE_HIDE = 2; + static final int STATE_UNKNOWN = 0; + // Shows the warning dialog. + static final int STATE_SHOW = 1; + // Skips the warning dialog. + static final int STATE_HIDE = 2; - public static int getWarningState(Context context, int def) { + static int getWarningState(Context context, int def) { final SharedPreferences prefs = context.getSharedPreferences( PREFS_BUGREPORT, Context.MODE_PRIVATE); return prefs.getInt(KEY_WARNING_STATE, def); } - public static void setWarningState(Context context, int value) { + static void setWarningState(Context context, int value) { final SharedPreferences prefs = context.getSharedPreferences( PREFS_BUGREPORT, Context.MODE_PRIVATE); prefs.edit().putInt(KEY_WARNING_STATE, value).apply(); diff --git a/packages/Shell/src/com/android/shell/BugreportProgressService.java b/packages/Shell/src/com/android/shell/BugreportProgressService.java index 796dff52304d..c70e021329e6 100644 --- a/packages/Shell/src/com/android/shell/BugreportProgressService.java +++ b/packages/Shell/src/com/android/shell/BugreportProgressService.java @@ -17,7 +17,8 @@ package com.android.shell; import static android.os.Process.THREAD_PRIORITY_BACKGROUND; -import static com.android.shell.BugreportPrefs.STATE_SHOW; +import static com.android.shell.BugreportPrefs.STATE_HIDE; +import static com.android.shell.BugreportPrefs.STATE_UNKNOWN; import static com.android.shell.BugreportPrefs.getWarningState; import java.io.BufferedOutputStream; @@ -927,7 +928,7 @@ public class BugreportProgressService extends Service { final Intent notifIntent; // Send through warning dialog by default - if (getWarningState(mContext, STATE_SHOW) == STATE_SHOW) { + if (getWarningState(mContext, STATE_UNKNOWN) != STATE_HIDE) { notifIntent = buildWarningIntent(mContext, sendIntent); } else { notifIntent = sendIntent; diff --git a/packages/Shell/src/com/android/shell/BugreportWarningActivity.java b/packages/Shell/src/com/android/shell/BugreportWarningActivity.java index a1d879af2025..da33a654f910 100644 --- a/packages/Shell/src/com/android/shell/BugreportWarningActivity.java +++ b/packages/Shell/src/com/android/shell/BugreportWarningActivity.java @@ -59,7 +59,7 @@ public class BugreportWarningActivity extends AlertActivity ap.mNegativeButtonListener = this; mConfirmRepeat = (CheckBox) ap.mView.findViewById(android.R.id.checkbox); - mConfirmRepeat.setChecked(getWarningState(this, STATE_UNKNOWN) == STATE_SHOW); + mConfirmRepeat.setChecked(getWarningState(this, STATE_UNKNOWN) != STATE_SHOW); setupAlert(); } @@ -68,7 +68,7 @@ public class BugreportWarningActivity extends AlertActivity public void onClick(DialogInterface dialog, int which) { if (which == AlertDialog.BUTTON_POSITIVE) { // Remember confirm state, and launch target - setWarningState(this, mConfirmRepeat.isChecked() ? STATE_SHOW : STATE_HIDE); + setWarningState(this, mConfirmRepeat.isChecked() ? STATE_HIDE : STATE_SHOW); startActivity(mSendIntent); } diff --git a/packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java b/packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java index 3b5305527ba5..9df7254cc27a 100644 --- a/packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java +++ b/packages/Shell/tests/src/com/android/shell/BugreportReceiverTest.java @@ -17,7 +17,14 @@ package com.android.shell; import static android.test.MoreAsserts.assertContainsRegex; + import static com.android.shell.ActionSendMultipleConsumerActivity.UI_NAME; +import static com.android.shell.BugreportPrefs.PREFS_BUGREPORT; +import static com.android.shell.BugreportPrefs.STATE_HIDE; +import static com.android.shell.BugreportPrefs.STATE_SHOW; +import static com.android.shell.BugreportPrefs.STATE_UNKNOWN; +import static com.android.shell.BugreportPrefs.getWarningState; +import static com.android.shell.BugreportPrefs.setWarningState; import static com.android.shell.BugreportProgressService.EXTRA_BUGREPORT; import static com.android.shell.BugreportProgressService.EXTRA_ID; import static com.android.shell.BugreportProgressService.EXTRA_MAX; @@ -48,12 +55,12 @@ import java.util.zip.ZipInputStream; import java.util.zip.ZipOutputStream; import libcore.io.Streams; + import android.app.ActivityManager; import android.app.ActivityManager.RunningServiceInfo; import android.app.Instrumentation; import android.app.NotificationManager; import android.content.Context; -import android.content.ContextWrapper; import android.content.Intent; import android.net.Uri; import android.os.Bundle; @@ -62,7 +69,6 @@ import android.service.notification.StatusBarNotification; import android.support.test.uiautomator.UiDevice; import android.support.test.uiautomator.UiObject; import android.support.test.uiautomator.UiObjectNotFoundException; -import android.support.test.uiautomator.UiSelector; import android.test.InstrumentationTestCase; import android.test.suitebuilder.annotation.LargeTest; import android.text.TextUtils; @@ -70,7 +76,6 @@ import android.text.format.DateUtils; import android.util.Log; import com.android.shell.ActionSendMultipleConsumerActivity.CustomActionSendMultipleListener; -import com.android.shell.BugreportProgressService; /** * Integration tests for {@link BugreportReceiver}. @@ -171,7 +176,7 @@ public class BugreportReceiverTest extends InstrumentationTestCase { } mDescription = sb.toString(); - BugreportPrefs.setWarningState(mContext, BugreportPrefs.STATE_HIDE); + setWarningState(mContext, STATE_HIDE); } public void testProgress() throws Exception { @@ -503,9 +508,29 @@ public class BugreportReceiverTest extends InstrumentationTestCase { assertServiceNotRunning(); } - public void testBugreportFinished_withWarning() throws Exception { - // Explicitly shows the warning. - BugreportPrefs.setWarningState(mContext, BugreportPrefs.STATE_SHOW); + public void testBugreportFinished_withWarningFirstTime() throws Exception { + bugreportFinishedWithWarningTest(null); + } + + public void testBugreportFinished_withWarningUnknownState() throws Exception { + bugreportFinishedWithWarningTest(STATE_UNKNOWN); + } + + public void testBugreportFinished_withWarningShowAgain() throws Exception { + bugreportFinishedWithWarningTest(STATE_SHOW); + } + + private void bugreportFinishedWithWarningTest(Integer propertyState) throws Exception { + if (propertyState == null) { + // Clear properties + mContext.getSharedPreferences(PREFS_BUGREPORT, Context.MODE_PRIVATE) + .edit().clear().commit(); + // Sanity check... + assertEquals("Did not reset properties", STATE_UNKNOWN, + getWarningState(mContext, STATE_UNKNOWN)); + } else { + setWarningState(mContext, propertyState); + } // Send notification and click on share. sendBugreportFinished(NO_ID, mPlainTextPath, null); @@ -513,10 +538,16 @@ public class BugreportReceiverTest extends InstrumentationTestCase { // Handle the warning mUiBot.getVisibleObject(mContext.getString(R.string.bugreport_confirm)); - // TODO: get ok and showMessageAgain from the dialog reference above - UiObject showMessageAgain = - mUiBot.getVisibleObject(mContext.getString(R.string.bugreport_confirm_repeat)); - mUiBot.click(showMessageAgain, "show-message-again"); + // TODO: get ok and dontShowAgain from the dialog reference above + UiObject dontShowAgain = + mUiBot.getVisibleObject(mContext.getString(R.string.bugreport_confirm_dont_repeat)); + final boolean firstTime = propertyState == null || propertyState == STATE_UNKNOWN; + if (firstTime) { + assertTrue("Checkbox should be checked by default", dontShowAgain.isChecked()); + } else { + assertFalse("Checkbox should not be checked", dontShowAgain.isChecked()); + mUiBot.click(dontShowAgain, "dont-show-again"); + } UiObject ok = mUiBot.getVisibleObject(mContext.getString(com.android.internal.R.string.ok)); mUiBot.click(ok, "ok"); @@ -526,8 +557,8 @@ public class BugreportReceiverTest extends InstrumentationTestCase { assertActionSendMultiple(extras, BUGREPORT_CONTENT, NO_SCREENSHOT); // Make sure it's hidden now. - int newState = BugreportPrefs.getWarningState(mContext, BugreportPrefs.STATE_UNKNOWN); - assertEquals("Didn't change state", BugreportPrefs.STATE_HIDE, newState); + int newState = getWarningState(mContext, STATE_UNKNOWN); + assertEquals("Didn't change state", STATE_HIDE, newState); } public void testShareBugreportAfterServiceDies() throws Exception { @@ -889,8 +920,8 @@ public class BugreportReceiverTest extends InstrumentationTestCase { } private String getPath(String file) { - File rootDir = new ContextWrapper(mContext).getFilesDir(); - File dir = new File(rootDir, BUGREPORTS_DIR); + final File rootDir = mContext.getFilesDir(); + final File dir = new File(rootDir, BUGREPORTS_DIR); if (!dir.exists()) { Log.i(TAG, "Creating directory " + dir); assertTrue("Could not create directory " + dir, dir.mkdir()); -- 2.11.0