Follow through when --force-dump-upload flag is set.

Follow through when --force-dump-upload flag is set.

When --force-dump-upload is set for testing, it needs to bypass all
the user preferences as it is used in automation. Otherwise some
crashes do not result in minidumps being uploaded despite having all
the right command line flags.

BUG=599269
Committed: https://crrev.com/d8bd715d1e0d45c4739ca8e49fece0ac27ced48a
Cr-Commit-Position: refs/heads/master@{#384896}

[Peter Wen, 2016-03-30 23:28:33]

wnwen@chromium.org changed reviewers:
+ yfriedman@chromium.org

[Peter Wen, 2016-03-30 23:28:33]

Hey Yaron,

This additional bypass is primarily for Android bots where they currently get
stuck and do not upload minidumps due to not setting the right preferences.

Full context in http://b/27548943#comment77

Thanks,

Peter

[Yaron, 2016-03-31 17:13:24]

yfriedman@chromium.org changed reviewers:
+ mimee@chromium.org

[Yaron, 2016-03-31 17:13:25]

Looks like this was added in https://codereview.chromium.org/1451153002

I don't think it was intentional so this seems reasonable. I just wonder if we
should be putting it into the caller instead?

https://codereview.chromium.org/1844173002/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/1844173002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/PrivacyPreferencesManager.java:394:
if (CommandLine.getInstance().hasSwitch(ChromeSwitches.FORCE_CRASH_DUMP_UPLOAD))
{
I wonder if this should be in the caller based on comments on L388

[mimee, 2016-03-31 17:31:26]

On 2016/03/31 17:13:25, Yaron wrote:
> Looks like this was added in https://codereview.chromium.org/1451153002
> 
> I don't think it was intentional so this seems reasonable. I just wonder if we
> should be putting it into the caller instead?
> 
>
https://codereview.chromium.org/1844173002/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/1844173002/diff/1/chrome/android/java/src/org...
>
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/PrivacyPreferencesManager.java:394:
> if
(CommandLine.getInstance().hasSwitch(ChromeSwitches.FORCE_CRASH_DUMP_UPLOAD))
> {
> I wonder if this should be in the caller based on comments on L388

Yes, definitely differentiate between 'user permitted' and 'test forced'.
You can do that by propagating the test status to the caller, thus logging
different
messages even if the behavior is similar (i.e. uploads reports).

The privacy pref manager needs loads of cleanup to make more sense, but the
work was de-prioritized back when I was working on it. For that reason also,
I suggest not making it even more tangled (the naming in the class is confusing
as it).

[Peter Wen, 2016-03-31 18:30:17]

PTAL. Switched to modifying caller instead. 🚌

https://codereview.chromium.org/1844173002/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/1844173002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/PrivacyPreferencesManager.java:394:
if (CommandLine.getInstance().hasSwitch(ChromeSwitches.FORCE_CRASH_DUMP_UPLOAD))
{
On 2016/03/31 17:13:25, Yaron wrote:
> I wonder if this should be in the caller based on comments on L388

Done.

[mimee, 2016-03-31 19:44:35]

Description was changed from

==========
Follow through when --force-dump-upload flag is set.

When --force-dump-upload is set for testing, it needs to bypass all
the user preferences as it is used in automation. Otherwise some
crashes do not result in minidumps being uploaded despite having all
the right command line flags.

BUG=599269
==========

to

==========
Follow through when --force-dump-upload flag is set.

When --force-dump-upload is set for testing, it needs to bypass all
the user preferences as it is used in automation. Otherwise some
crashes do not result in minidumps being uploaded despite having all
the right command line flags.

BUG=599269
==========

[mimee, 2016-03-31 19:44:35]

mimee@chromium.org changed reviewers:
+ jchinlee@chromium.org

[Yaron, 2016-03-31 19:46:46]

lgtm

[Peter Wen, 2016-04-04 12:45:44]

The CQ bit was checked by wnwen@chromium.org

[commit-bot: I haz the power, 2016-04-04 12:45:57]

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

[commit-bot: I haz the power, 2016-04-04 13:40:12]

Description was changed from

==========
Follow through when --force-dump-upload flag is set.

When --force-dump-upload is set for testing, it needs to bypass all
the user preferences as it is used in automation. Otherwise some
crashes do not result in minidumps being uploaded despite having all
the right command line flags.

BUG=599269
==========

to

==========
Follow through when --force-dump-upload flag is set.

When --force-dump-upload is set for testing, it needs to bypass all
the user preferences as it is used in automation. Otherwise some
crashes do not result in minidumps being uploaded despite having all
the right command line flags.

BUG=599269
==========

[commit-bot: I haz the power, 2016-04-04 13:40:12]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-04-04 13:41:59]

Description was changed from

==========
Follow through when --force-dump-upload flag is set.

When --force-dump-upload is set for testing, it needs to bypass all
the user preferences as it is used in automation. Otherwise some
crashes do not result in minidumps being uploaded despite having all
the right command line flags.

BUG=599269
==========

to

==========
Follow through when --force-dump-upload flag is set.

When --force-dump-upload is set for testing, it needs to bypass all
the user preferences as it is used in automation. Otherwise some
crashes do not result in minidumps being uploaded despite having all
the right command line flags.

BUG=599269

Committed: https://crrev.com/d8bd715d1e0d45c4739ca8e49fece0ac27ced48a
Cr-Commit-Position: refs/heads/master@{#384896}
==========

[commit-bot: I haz the power, 2016-04-04 13:42:00]

Patchset 2 (id:??) landed as
https://crrev.com/d8bd715d1e0d45c4739ca8e49fece0ac27ced48a
Cr-Commit-Position: refs/heads/master@{#384896}