Schedule retry for user-permitted uploads.

Schedule retry for user-permitted uploads.

Previously uploads that are user-permitted but not network-permitted
get ignored when it comes to retry scheduling. This fix makes them
eligible for retry.

* Use isUploadUserPermitted just to check user preference, so that
uploads that only failed this check may be retried in the future.

* Delete crash dump if user does not permit uploads. This makes sure
that uploads returning UPLOAD_DISABLED do not get retried, ever.

Also deleted obsolete code and fixed a TAG prefix.

BUG=553488
R=acleung@chromium.org

Committed: https://crrev.com/95b02f0eabcd1a4d661f63a6fa38d4d30945f0fd
Cr-Commit-Position: refs/heads/master@{#360147}

[mimee1, 2015-11-16 23:00:40]

mimee@google.com changed reviewers:
+ dtrainor@google.com, gayane@chromium.org, jchinlee@chromium.org,
mimee@google.com

[David Trainor- moved to gerrit, 2015-11-17 06:48:13]

Description was changed from

==========
Schedule retry for user-permitted uploads.

Previously uploads that are user-permitted but not network-permitted
get ignored when it comes to retry scheduling. This fix makes them
eligible for retry.

* Use isUploadUserPermitted just to check user preference, so that
uploads that only failed this check may be retried in the future.

* Delete crash dump if user does not permit uploads. This makes sure
that uploads returning UPLOAD_DISABLED do not get retried, ever.

Also deleted obsolete code and fixed a TAG prefix.

BUG=553488
R=acleung@chromium.org
==========

to

==========
Schedule retry for user-permitted uploads.

Previously uploads that are user-permitted but not network-permitted
get ignored when it comes to retry scheduling. This fix makes them
eligible for retry.

* Use isUploadUserPermitted just to check user preference, so that
uploads that only failed this check may be retried in the future.

* Delete crash dump if user does not permit uploads. This makes sure
that uploads returning UPLOAD_DISABLED do not get retried, ever.

Also deleted obsolete code and fixed a TAG prefix.

BUG=553488
R=acleung@chromium.org
==========

[David Trainor- moved to gerrit, 2015-11-17 06:48:13]

dtrainor@chromium.org changed reviewers:
+ dtrainor@chromium.org
- dtrainor@google.com

[David Trainor- moved to gerrit, 2015-11-17 06:52:27]

lgtm, but wait for others :).

https://codereview.chromium.org/1451153002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/PrivacyPreferencesManager.java
(right):

https://codereview.chromium.org/1451153002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/PrivacyPreferencesManager.java:258:
return false;
put this on the above line?

https://codereview.chromium.org/1451153002/diff/1/chrome/android/javatests/sr...
File
chrome/android/javatests/src/org/chromium/chrome/browser/crash/MinidumpUploadCallableTest.java
(right):

https://codereview.chromium.org/1451153002/diff/1/chrome/android/javatests/sr...
chrome/android/javatests/src/org/chromium/chrome/browser/crash/MinidumpUploadCallableTest.java:124:
boolean isLimited) {
I think this should just be +8 from the M in MockCrash...

[gayane -on leave until 09-2017, 2015-11-17 14:46:04]

LGTM

https://codereview.chromium.org/1451153002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/PrivacyPreferencesManager.java
(right):

https://codereview.chromium.org/1451153002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/PrivacyPreferencesManager.java:262:
if (isMobileNetworkCapable()) {
From this point on its the same logic as isUploadCrashDumpEnabled().
You could reuse that function if you want.

[mimee, 2015-11-17 16:52:54]

New patch. PTAL

https://codereview.chromium.org/1451153002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/PrivacyPreferencesManager.java
(right):

https://codereview.chromium.org/1451153002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/PrivacyPreferencesManager.java:258:
return false;
On 2015/11/17 06:52:27, David Trainor wrote:
> put this on the above line?

Done.

https://codereview.chromium.org/1451153002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/PrivacyPreferencesManager.java:262:
if (isMobileNetworkCapable()) {
On 2015/11/17 14:46:04, gayane wrote:
> From this point on its the same logic as isUploadCrashDumpEnabled().
> You could reuse that function if you want.

I have a refactor coming in soon after this one. isUploadCrashDumpEnabled()
currently has the wrong docs, so I'd need to change a bit more. I will keep this
fix one small to merge and clean up all together later. Is that okay?

[gayane -on leave until 09-2017, 2015-11-17 19:04:50]

https://codereview.chromium.org/1451153002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/PrivacyPreferencesManager.java
(right):

https://codereview.chromium.org/1451153002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/PrivacyPreferencesManager.java:262:
if (isMobileNetworkCapable()) {
On 2015/11/17 16:52:53, mimee wrote:
> On 2015/11/17 14:46:04, gayane wrote:
> > From this point on its the same logic as isUploadCrashDumpEnabled().
> > You could reuse that function if you want.
> 
> I have a refactor coming in soon after this one. isUploadCrashDumpEnabled()
> currently has the wrong docs, so I'd need to change a bit more. I will keep
this
> fix one small to merge and clean up all together later. Is that okay?

SGTM

[mimee, 2015-11-17 19:46:15]

The CQ bit was checked by mimee@chromium.org

[mimee, 2015-11-17 19:46:16]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtrainor@chromium.org, gayane@chromium.org
Link to the patchset: https://codereview.chromium.org/1451153002/#ps60001
(title: "Undo introduction of obsolete code")

[commit-bot: I haz the power, 2015-11-17 19:46:27]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1451153002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1451153002/60001

[commit-bot: I haz the power, 2015-11-17 20:26:10]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-11-17 20:27:14]

Patchset 4 (id:??) landed as
https://crrev.com/95b02f0eabcd1a4d661f63a6fa38d4d30945f0fd
Cr-Commit-Position: refs/heads/master@{#360147}