For Chrome builds, only upload crashes when the build is official.

For Chrome builds, only upload crashes when the build is official.

BUG=362783
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=271840

[Lei Zhang, 2014-05-20 02:16:48]

If there's a good reason to not do this, let me know.

[Mark Mentovai, 2014-05-20 03:31:19]

LGTM

https://codereview.chromium.org/292933002/diff/20001/chrome/app/chrome_breakp...
File chrome/app/chrome_breakpad_client.cc (right):

https://codereview.chromium.org/292933002/diff/20001/chrome/app/chrome_breakp...
chrome/app/chrome_breakpad_client.cc:337: // Convert #define to a variable so
that we can use if() rather than
This comment is becoming kind of inaccurate, and it’s also kind of pointless. I
think we can lose it now. Same thing in the other file.

[robertshield, 2014-05-20 03:34:29]

I had to add breakpad reporting to something recently and found it very useful
to be able to test that crash reporting works end-to-end with a local build.
This is a rather uncommon use case though, so if I'm the only dissenting voice,
please ignore me :-)  Having folk chase phantom bugs from dev builds seems much
worse.

Code LGTM.

Also +CC Siggi who has been looking into crash recently.

[Lei Zhang, 2014-05-20 03:45:15]

On 2014/05/20 03:34:29, robertshield wrote:
> I had to add breakpad reporting to something recently and found it very useful
> to be able to test that crash reporting works end-to-end with a local build.
> This is a rather uncommon use case though, so if I'm the only dissenting
voice,
> please ignore me :-)  Having folk chase phantom bugs from dev builds seems
much
> worse.

Yes, we got here because QA filed bugs for crashes that the crash processor
couldn't parse properly. Fortunately, it didn't take too long to figure out this
was someone's test build set to branding=Chrome (with type=Dev).

[Lei Zhang, 2014-05-20 03:48:40]

https://codereview.chromium.org/292933002/diff/20001/chrome/app/chrome_breakp...
File chrome/app/chrome_breakpad_client.cc (right):

https://codereview.chromium.org/292933002/diff/20001/chrome/app/chrome_breakp...
chrome/app/chrome_breakpad_client.cc:337: // Convert #define to a variable so
that we can use if() rather than
On 2014/05/20 03:31:19, Mark Mentovai wrote:
> This comment is becoming kind of inaccurate, and it’s also kind of pointless.
I
> think we can lose it now. Same thing in the other file.

Done.

[Mark Mentovai, 2014-05-20 12:57:50]

robertshield wrote:
> I had to add breakpad reporting to something recently and found it very useful
> to be able to test that crash reporting works end-to-end with a local build.

When someone really needs to test this, it’s probably easy for them to locally
tweak this file to turn uploads back on. Most developers are using the Chromium
branding and not the Google branding anyway, so that’s something they would have
needed to do before this change and after.

[Lei Zhang, 2014-05-20 21:01:38]

The CQ bit was checked by thestig@chromium.org

[commit-bot: I haz the power, 2014-05-20 21:05:25]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/thestig@chromium.org/292933002/40001

[commit-bot: I haz the power, 2014-05-21 03:21:34]

Change committed as 271840

[Sigurður Ásgeirsson, 2014-05-30 18:18:46]

Hey guys,

this makes it even harder to test crash reporting than it was. How about
reverting this, and tagging crashes from non-official builds with a special
key instead?

Siggi


On Mon, May 19, 2014 at 11:31 PM, <mark@chromium.org> wrote:

> LGTM
>
>
> https://codereview.chromium.org/292933002/diff/20001/
> chrome/app/chrome_breakpad_client.cc
> File chrome/app/chrome_breakpad_client.cc (right):
>
> https://codereview.chromium.org/292933002/diff/20001/
> chrome/app/chrome_breakpad_client.cc#newcode337
> chrome/app/chrome_breakpad_client.cc:337: // Convert #define to a
> variable so that we can use if() rather than
> This comment is becoming kind of inaccurate, and it's also kind of
> pointless. I think we can lose it now. Same thing in the other file.
>
> https://codereview.chromium.org/292933002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.