AU: Reduce instances of signalling the crash reporter.

AU: Reduce instances of signalling the crash reporter.

We recently started notifying the crash reporter whenever we attempted
to report an error code to the update server. We now throttle that
back a little bit: we only notify for official builds when we were
successful in reporting the error to the update server.

TEST=unittest; tested on device
BUG=10805
Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=2008e4c

[petkov, 2011-01-12 01:10:22]

http://codereview.chromium.org/6154003/diff/1/omaha_request_action.cc
File omaha_request_action.cc (left):

http://codereview.chromium.org/6154003/diff/1/omaha_request_action.cc#oldcode347
omaha_request_action.cc:347: completer.set_code(kActionCodeSuccess);
Here you could:

if (event->result ==
OmahaEvent::kResultError && !successful && IsOfficialBuild()) {
  utils::ScheduleCrashReporterUpload();
}

See other comment why this might be a better implementation.

http://codereview.chromium.org/6154003/diff/1/omaha_request_action.cc
File omaha_request_action.cc (right):

http://codereview.chromium.org/6154003/diff/1/omaha_request_action.cc#newcode356
omaha_request_action.cc:356: // Events are best effort transactions, but it's
still okay to fail them,
This is somewhat dangerous. The code runs for both real error events and
"normal" events -- e.g., the successful kTypeUpdateComplete at the end of the
attempter action list.

If, for example, you lose Internet connection during the post-install action,
the update will technically succeed but the Event will not go through and the
update attempter will say that the whole update has failed. Maybe we should
extend the comment to make that clear.

A safer way to implement this might be to move the ScheduleCrashReporterUpload
call here. Basically, check for IsEvent first (original condition order), then
inside the if do the upload only if it's a real error event (if event->result ==
OmahaEvent::kResultError) and !successful and IsOfficialBuild.

See the other comment for a possible implementation.

[petkov, 2011-01-12 01:17:07]

http://codereview.chromium.org/6154003/diff/1/omaha_request_action.cc
File omaha_request_action.cc (left):

http://codereview.chromium.org/6154003/diff/1/omaha_request_action.cc#oldcode347
omaha_request_action.cc:347: completer.set_code(kActionCodeSuccess);
On 2011/01/12 01:10:22, petkov wrote:
> Here you could:
> 
> if (event->result ==
> OmahaEvent::kResultError && !successful && IsOfficialBuild()) {
>   utils::ScheduleCrashReporterUpload();
> }
> 
> See other comment why this might be a better implementation.

Of course I meant:

if (event->result == OmahaEvent::kResultError && successful &&
IsOfficialBuild()) ...

[adlr, 2011-01-12 03:02:00]

okay, made the change you suggest. PTAL. thanks!

[petkov, 2011-01-12 17:51:46]

LGTM. Thanks!