Consume EXCEPTION_INVALID_HANDLE on sandbox process shutdown.

Consume EXCEPTION_INVALID_HANDLE on sandbox process shutdown.

BUG=452613
Committed: https://crrev.com/9428d09618bb28e6161f9d4d5159bdc47ba037a8
Cr-Commit-Position: refs/heads/master@{#318204}

[Will Harris, 2015-02-22 03:12:10]

wfh@chromium.org changed reviewers:
+ cpu@chromium.org

[Will Harris, 2015-02-22 05:14:01]

PTAL - but this needs a breakpad roll to run on the bots.

[Will Harris, 2015-02-23 19:04:13]

On 2015/02/22 05:14:01, Will Harris wrote:
> PTAL - but this needs a breakpad roll to run on the bots.

siggi suggests doing this exception eating just for Win 10.  WDYT?

[Sigurður Ásgeirsson, 2015-02-23 19:52:57]

My worry is that this'll potentially occlude future regressions - 's all.

On Mon Feb 23 2015 at 2:04:14 PM <wfh@chromium.org> wrote:

> On 2015/02/22 05:14:01, Will Harris wrote:
> > PTAL - but this needs a breakpad roll to run on the bots.
>
> siggi suggests doing this exception eating just for Win 10.  WDYT?
>
> https://codereview.chromium.org/944283002/
>

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

[cpu_(ooo_6.6-7.5), 2015-02-23 21:09:39]

I feel that I don't care if it is sanboxed or not instead I care that is W10.

other than that looks good.

[Will Harris, 2015-02-24 20:18:03]

On 2015/02/23 21:09:39, cpu wrote:
> I feel that I don't care if it is sanboxed or not instead I care that is W10.
> 
> other than that looks good.

Made it only Windows 10.  crbug.com/456193 to check before Win10 goes RTM
whether we can remove this.

[rvargas (doing something else), 2015-02-25 02:02:56]

rvargas@chromium.org changed reviewers:
+ rvargas@chromium.org

[rvargas (doing something else), 2015-02-25 02:02:57]

https://codereview.chromium.org/944283002/diff/20001/chrome/app/client_util.cc
File chrome/app/client_util.cc (right):

https://codereview.chromium.org/944283002/diff/20001/chrome/app/client_util.c...
chrome/app/client_util.cc:255: breakpad::ConsumeInvalidHandleExceptions();
Drive by:

I don't know what this code does (don't see it anywhere), but all exceptions are
not equal. Some require extra work when handling them.

I believe that invalid exception is on the non-trivial side.
See: https://code.google.com/p/chromium/issues/detail?id=131699#c25

Maybe it's already taken care of...

[Will Harris, 2015-02-25 02:15:18]

On 2015/02/25 02:02:57, rvargas wrote:
> https://codereview.chromium.org/944283002/diff/20001/chrome/app/client_util.cc
> File chrome/app/client_util.cc (right):
> 
>
https://codereview.chromium.org/944283002/diff/20001/chrome/app/client_util.c...
> chrome/app/client_util.cc:255: breakpad::ConsumeInvalidHandleExceptions();
> Drive by:
> 
> I don't know what this code does (don't see it anywhere), but all exceptions
are
> not equal. Some require extra work when handling them.
> 
> I believe that invalid exception is on the non-trivial side.
> See: https://code.google.com/p/chromium/issues/detail?id=131699#c25
> 
> Maybe it's already taken care of...

the code is here in breakpad:

https://chromium.googlesource.com/external/google-breakpad/src/+/master/clien...

and I've tested that it successfully catches and eats the
INVALID_HANDLE_EXCEPTION exceptions generated on shutdown.

[rvargas (doing something else), 2015-02-25 03:51:24]

On 2015/02/25 02:15:18, Will Harris wrote:
> On 2015/02/25 02:02:57, rvargas wrote:
> >
https://codereview.chromium.org/944283002/diff/20001/chrome/app/client_util.cc
> > File chrome/app/client_util.cc (right):
> > 
> >
>
https://codereview.chromium.org/944283002/diff/20001/chrome/app/client_util.c...
> > chrome/app/client_util.cc:255: breakpad::ConsumeInvalidHandleExceptions();
> > Drive by:
> > 
> > I don't know what this code does (don't see it anywhere), but all exceptions
> are
> > not equal. Some require extra work when handling them.
> > 
> > I believe that invalid exception is on the non-trivial side.
> > See: https://code.google.com/p/chromium/issues/detail?id=131699#c25
> > 
> > Maybe it's already taken care of...
> 
> the code is here in breakpad:
> 
>
https://chromium.googlesource.com/external/google-breakpad/src/+/master/clien...
> 
> and I've tested that it successfully catches and eats the
> INVALID_HANDLE_EXCEPTION exceptions generated on shutdown.

OK. Alexey was pretty sure about that (but I'm sure he never tested Win 10 :P ).
Maybe it was on some specific OSes (I sort of recall something about different
behavior at least for x86/64). I don't think we should add functionality that
may break sometimes but if you tested it I'm good.

[cpu_(ooo_6.6-7.5), 2015-02-25 22:54:30]

lgtm

[Will Harris, 2015-02-26 02:18:14]

The CQ bit was checked by wfh@chromium.org

[commit-bot: I haz the power, 2015-02-26 02:19:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/944283002/40001

[commit-bot: I haz the power, 2015-02-26 07:12:32]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-02-26 07:13:36]

Patchset 3 (id:??) landed as
https://crrev.com/9428d09618bb28e6161f9d4d5159bdc47ba037a8
Cr-Commit-Position: refs/heads/master@{#318204}