Merge "Revert warning logic so it has a "don't show again" message." into nyc-dev

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" />
 </LinearLayout>

packages/Shell/res/values/strings.xml

@@ -35,9 +35,9 @@
     <string name="bugreport_finished_pending_screenshot_text" product="default">Tap to share your bug report without a screenshot or wait for the screenshot to finish</string>
 
     <!-- Body of dialog informing user about contents of a bugreport. [CHAR LIMIT=NONE] -->
-    <string name="bugreport_confirm">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.</string>
+    <string name="bugreport_confirm">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.</string>
-    <!-- Checkbox that indicates this dialog should be shown again when the next bugreport is taken. [CHAR LIMIT=50] -->
+    <!-- Checkbox that indicates this dialog should not be shown again when the next bugreport is taken. [CHAR LIMIT=50] -->
-    <string name="bugreport_confirm_repeat">Show this message next time</string>
+    <string name="bugreport_confirm_dont_repeat">Don\'t show again</string>
 
     <!-- Title for documents backend that offers bugreports. -->
     <string name="bugreport_storage_title">Bug reports</string>

packages/Shell/src/com/android/shell/BugreportPrefs.java

@@ -22,22 +22,24 @@ import android.content.SharedPreferences;
 /**
  * Preferences related to bug reports.
  */
-public class BugreportPrefs {
+final class BugreportPrefs {
-    private static final String PREFS_BUGREPORT = "bugreports";
+    static final String PREFS_BUGREPORT = "bugreports";
 
     private static final String KEY_WARNING_STATE = "warning-state";
 
-    public static final int STATE_UNKNOWN = 0;
+    static final int STATE_UNKNOWN = 0;
-    public static final int STATE_SHOW = 1;
+    // Shows the warning dialog.
-    public static final int STATE_HIDE = 2;
+    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();

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;

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);
         }
 

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}.
@@ -168,7 +173,7 @@ public class BugreportReceiverTest extends InstrumentationTestCase {
         }
         mDescription = sb.toString();
 
-        BugreportPrefs.setWarningState(mContext, BugreportPrefs.STATE_HIDE);
+        setWarningState(mContext, STATE_HIDE);
     }
 
     public void testProgress() throws Exception {
@@ -500,9 +505,29 @@ public class BugreportReceiverTest extends InstrumentationTestCase {
         assertServiceNotRunning();
     }
 
-    public void testBugreportFinished_withWarning() throws Exception {
+    public void testBugreportFinished_withWarningFirstTime() throws Exception {
-        // Explicitly shows the warning.
+        bugreportFinishedWithWarningTest(null);
-        BugreportPrefs.setWarningState(mContext, BugreportPrefs.STATE_SHOW);
+    }
+
+    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);
@@ -510,10 +535,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
+        // TODO: get ok and dontShowAgain from the dialog reference above
-        UiObject showMessageAgain =
+        UiObject dontShowAgain =
-                mUiBot.getVisibleObject(mContext.getString(R.string.bugreport_confirm_repeat));
+                mUiBot.getVisibleObject(mContext.getString(R.string.bugreport_confirm_dont_repeat));
-        mUiBot.click(showMessageAgain, "show-message-again");
+        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");
 
@@ -523,8 +554,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);
+        int newState = getWarningState(mContext, STATE_UNKNOWN);
-        assertEquals("Didn't change state", BugreportPrefs.STATE_HIDE, newState);
+        assertEquals("Didn't change state", STATE_HIDE, newState);
     }
 
     public void testShareBugreportAfterServiceDies() throws Exception {
@@ -876,8 +907,8 @@ public class BugreportReceiverTest extends InstrumentationTestCase {
     }
 
     private String getPath(String file) {
-        File rootDir = new ContextWrapper(mContext).getFilesDir();
+        final File rootDir = mContext.getFilesDir();
-        File dir = new File(rootDir, BUGREPORTS_DIR);
+        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());