Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(29)

Issue 2514343002: Don't silently eat Inspector.TargetCrashed (Closed)

Created:
4 years, 1 month ago by alex clarke (OOO till 29th)
Modified:
4 years, 1 month ago
Reviewers:
Sami
CC:
chromium-reviews, pfeldman, devtools-reviews_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Don't silently eat Inspector.TargetCrashed For the browser test and headless_shell the devtools event is used to detect renderer crashes. We retain the C++ observer since it returns the termination reason (not avaliable over devtools) BUG=546953 Committed: https://crrev.com/a0adb8b5bd579433f57903c18f9764d69c57de1c Cr-Commit-Position: refs/heads/master@{#433903}

Patch Set 1 #

Total comments: 4

Patch Set 2 : Added a comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+20 lines, -15 lines) Patch
M headless/app/headless_shell.cc View 1 5 chunks +6 lines, -5 lines 0 comments Download
M headless/lib/browser/headless_devtools_client_impl.cc View 1 chunk +2 lines, -7 lines 0 comments Download
M headless/lib/headless_devtools_client_browsertest.cc View 2 chunks +12 lines, -3 lines 0 comments Download

Messages

Total messages: 13 (4 generated)
alex clarke (OOO till 29th)
4 years, 1 month ago (2016-11-21 11:41:44 UTC) #2
Sami
Thanks! Would you mind adding a TODO somewhere for adding the termination reason to the ...
4 years, 1 month ago (2016-11-21 14:59:28 UTC) #3
alex clarke (OOO till 29th)
https://codereview.chromium.org/2514343002/diff/1/headless/app/headless_shell.cc File headless/app/headless_shell.cc (right): https://codereview.chromium.org/2514343002/diff/1/headless/app/headless_shell.cc#newcode171 headless/app/headless_shell.cc:171: Shutdown(); On 2016/11/21 14:59:28, Sami wrote: > Should we ...
4 years, 1 month ago (2016-11-22 10:48:55 UTC) #4
Sami
https://codereview.chromium.org/2514343002/diff/1/headless/app/headless_shell.cc File headless/app/headless_shell.cc (right): https://codereview.chromium.org/2514343002/diff/1/headless/app/headless_shell.cc#newcode171 headless/app/headless_shell.cc:171: Shutdown(); On 2016/11/22 10:48:55, alex clarke wrote: > On ...
4 years, 1 month ago (2016-11-22 11:23:11 UTC) #5
alex clarke (OOO till 29th)
https://codereview.chromium.org/2514343002/diff/1/headless/app/headless_shell.cc File headless/app/headless_shell.cc (right): https://codereview.chromium.org/2514343002/diff/1/headless/app/headless_shell.cc#newcode171 headless/app/headless_shell.cc:171: Shutdown(); On 2016/11/22 11:23:11, Sami wrote: > On 2016/11/22 ...
4 years, 1 month ago (2016-11-22 15:30:58 UTC) #6
Sami
Ah, thanks. lgtm.
4 years, 1 month ago (2016-11-22 17:19:54 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2514343002/20001
4 years, 1 month ago (2016-11-22 17:21:11 UTC) #9
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 1 month ago (2016-11-22 17:30:57 UTC) #11
commit-bot: I haz the power
4 years, 1 month ago (2016-11-22 17:37:55 UTC) #13
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/a0adb8b5bd579433f57903c18f9764d69c57de1c
Cr-Commit-Position: refs/heads/master@{#433903}

Powered by Google App Engine
This is Rietveld 408576698