HeadlessWebContents:Observer to observe render process exit status

HeadlessWebContents:Observer to observe render process exit status

This means it's now possible to detect if the renderer crashes for
some reason.

BUG=546953
Committed: https://crrev.com/5777c83ead271a3866bd4aea8a9d8cae51aef2b0
Cr-Commit-Position: refs/heads/master@{#433231}

[alex clarke (OOO till 29th), 2016-11-18 14:24:13]

alexclarke@chromium.org changed reviewers:
+ altimin@chromium.org, eseckler@chromium.org

[altimin, 2016-11-18 14:36:45]

https://codereview.chromium.org/2509813006/diff/20001/headless/app/headless_s...
File headless/app/headless_shell.cc (right):

https://codereview.chromium.org/2509813006/diff/20001/headless/app/headless_s...
headless/app/headless_shell.cc:168: web_contents_->RemoveObserver(this);
Looks like we do not detail devtools observers here (see
HeadlessShell::Shutdown). Maybe we should be able to leverage
HeadlessShell::Shutdown here?

[alex clarke (OOO till 29th), 2016-11-18 14:59:38]

https://codereview.chromium.org/2509813006/diff/20001/headless/app/headless_s...
File headless/app/headless_shell.cc (right):

https://codereview.chromium.org/2509813006/diff/20001/headless/app/headless_s...
headless/app/headless_shell.cc:168: web_contents_->RemoveObserver(this);
On 2016/11/18 14:36:45, altimin wrote:
> Looks like we do not detail devtools observers here (see
> HeadlessShell::Shutdown). Maybe we should be able to leverage
> HeadlessShell::Shutdown here?

I had to figure out why it was crashing. That's (mostly) fixed, thers's some
other crash with no call stack in the browser shutdown.

../../build/linux/debian_wheezy_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../include/c++/4.6/debug/safe_iterator.261:
    error: attempt to increment a singular iterator.

[alex clarke (OOO till 29th), 2016-11-18 14:59:45]

The CQ bit was checked by alexclarke@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-18 15:00:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-18 15:07:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-18 15:07:59]

Dry run: This issue passed the CQ dry run.

[altimin, 2016-11-18 16:58:53]

lgtm

[alex clarke (OOO till 29th), 2016-11-18 16:59:26]

The CQ bit was checked by alexclarke@chromium.org

[commit-bot: I haz the power, 2016-11-18 17:00:04]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-18 17:42:47]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-11-18 17:45:32]

Description was changed from

==========
HeadlessWebContents:Observer to observe render process exit status

This means it's now possible to detect if the renderer crashes for
some reason.

BUG=546953
==========

to

==========
HeadlessWebContents:Observer to observe render process exit status

This means it's now possible to detect if the renderer crashes for
some reason.

BUG=546953

Committed: https://crrev.com/5777c83ead271a3866bd4aea8a9d8cae51aef2b0
Cr-Commit-Position: refs/heads/master@{#433231}
==========

[commit-bot: I haz the power, 2016-11-18 17:45:32]

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

[Sami, 2016-11-18 18:37:21]

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

[Sami, 2016-11-18 18:37:22]

https://codereview.chromium.org/2509813006/diff/40001/headless/lib/browser/he...
File headless/lib/browser/headless_devtools_client_impl.cc (right):

https://codereview.chromium.org/2509813006/diff/40001/headless/lib/browser/he...
headless/lib/browser/headless_devtools_client_impl.cc:140: if (method ==
"Inspector.targetCrashed") {
Shouldn't we still emit this event in Headless Shell / chrome --headless?

Does the C++ notification give us something this event doesn't? I'm wondering
whether we should instead extend this event to do what we need?

[alex clarke (OOO till 29th), 2016-11-21 10:44:01]

https://codereview.chromium.org/2509813006/diff/40001/headless/lib/browser/he...
File headless/lib/browser/headless_devtools_client_impl.cc (right):

https://codereview.chromium.org/2509813006/diff/40001/headless/lib/browser/he...
headless/lib/browser/headless_devtools_client_impl.cc:140: if (method ==
"Inspector.targetCrashed") {
On 2016/11/18 18:37:22, Sami (slow -- traveling) wrote:
> Shouldn't we still emit this event in Headless Shell / chrome --headless?
> 
> Does the C++ notification give us something this event doesn't? I'm wondering
> whether we should instead extend this event to do what we need?

Hmm so we do actually have an inspector domain - I hadn't appreciated that. 
Suggests that this bit of code is a mistake.

The C++ API does give us the base::TerminationStatus and platform specific exit
code.  Those might be useful still.