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

Issue 17104003: Improvements to fatal JavaScript extension errors: (1) include the extension (Closed)

Created:
7 years, 6 months ago by not at google - send to devlin
Modified:
7 years, 6 months ago
Reviewers:
Jeffrey Yasskin
CC:
chromium-reviews, chromium-apps-reviews_chromium.org, extensions-reviews_chromium.org
Visibility:
Public.

Description

Improvements to fatal JavaScript extension errors: (1) include the extension ID if there is one, (2) only crash extension processes. The latter is a shame - it'll hide JS problems in the wild - but I'm not comfortable the way it is. R=jyasskin@chromium.org TBR=jochen@chromium.org NOTRY=true Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=208058

Patch Set 1 #

Patch Set 2 : . #

Total comments: 12

Patch Set 3 : jeffrey #

Total comments: 4

Patch Set 4 : . #

Patch Set 5 : channel restriction #

Patch Set 6 : channel restriction #

Unified diffs Side-by-side diffs Delta from patch set Stats (+96 lines, -50 lines) Patch
M chrome/renderer/extensions/chrome_v8_context_set.h View 1 2 1 chunk +6 lines, -2 lines 0 comments Download
M chrome/renderer/extensions/chrome_v8_context_set.cc View 1 2 3 4 5 1 chunk +7 lines, -4 lines 0 comments Download
M chrome/renderer/extensions/dispatcher.cc View 1 2 3 4 5 4 chunks +11 lines, -9 lines 0 comments Download
M chrome/renderer/extensions/module_system.cc View 1 2 3 4 5 13 chunks +67 lines, -23 lines 0 comments Download
M chrome/renderer/resources/extensions/ad_view.js View 1 2 3 4 5 1 chunk +5 lines, -6 lines 0 comments Download
D chrome/renderer/resources/extensions/ad_view_custom.js View 1 2 3 4 5 1 chunk +0 lines, -5 lines 0 comments Download
M chrome/renderer/resources/renderer_resources.grd View 1 2 3 4 5 1 chunk +0 lines, -1 line 0 comments Download

Messages

Total messages: 16 (0 generated)
not at google - send to devlin
FYI there is no BUG= because there are too many to choose from.
7 years, 6 months ago (2013-06-14 21:00:02 UTC) #1
Jeffrey Yasskin
https://codereview.chromium.org/17104003/diff/2001/chrome/renderer/extensions/chrome_v8_context_set.h File chrome/renderer/extensions/chrome_v8_context_set.h (right): https://codereview.chromium.org/17104003/diff/2001/chrome/renderer/extensions/chrome_v8_context_set.h#newcode54 chrome/renderer/extensions/chrome_v8_context_set.h:54: // Gets the ChromeV8Context correponding to v8::Context::GetCurrent(), or sp: ...
7 years, 6 months ago (2013-06-14 21:42:59 UTC) #2
not at google - send to devlin
https://codereview.chromium.org/17104003/diff/2001/chrome/renderer/extensions/chrome_v8_context_set.h File chrome/renderer/extensions/chrome_v8_context_set.h (right): https://codereview.chromium.org/17104003/diff/2001/chrome/renderer/extensions/chrome_v8_context_set.h#newcode54 chrome/renderer/extensions/chrome_v8_context_set.h:54: // Gets the ChromeV8Context correponding to v8::Context::GetCurrent(), or On ...
7 years, 6 months ago (2013-06-14 22:00:32 UTC) #3
Jeffrey Yasskin
lgtm, with the comments below. https://codereview.chromium.org/17104003/diff/2001/chrome/renderer/extensions/module_system.cc File chrome/renderer/extensions/module_system.cc (right): https://codereview.chromium.org/17104003/diff/2001/chrome/renderer/extensions/module_system.cc#newcode47 chrome/renderer/extensions/module_system.cc:47: if (is_extension_process) On 2013/06/14 ...
7 years, 6 months ago (2013-06-14 22:11:22 UTC) #4
not at google - send to devlin
https://codereview.chromium.org/17104003/diff/2001/chrome/renderer/extensions/module_system.cc File chrome/renderer/extensions/module_system.cc (right): https://codereview.chromium.org/17104003/diff/2001/chrome/renderer/extensions/module_system.cc#newcode47 chrome/renderer/extensions/module_system.cc:47: if (is_extension_process) On 2013/06/14 22:11:22, Jeffrey Yasskin wrote: > ...
7 years, 6 months ago (2013-06-14 22:27:44 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/kalman@chromium.org/17104003/20001
7 years, 6 months ago (2013-06-14 22:34:41 UTC) #6
commit-bot: I haz the power
Retried try job too often on linux_chromeos for step(s) browser_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chromeos&number=126139
7 years, 6 months ago (2013-06-15 01:14:24 UTC) #7
not at google - send to devlin
I made it crash web in dev channel - it's already like that anyway.
7 years, 6 months ago (2013-06-21 01:32:06 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/kalman@chromium.org/17104003/34001
7 years, 6 months ago (2013-06-21 01:33:45 UTC) #9
commit-bot: I haz the power
Retried try job too often on linux_aura for step(s) browser_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura&number=53215
7 years, 6 months ago (2013-06-21 04:08:14 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/kalman@chromium.org/17104003/34001
7 years, 6 months ago (2013-06-21 16:00:51 UTC) #11
commit-bot: I haz the power
Step "update" is always a major failure. Look at the try server FAQ for more ...
7 years, 6 months ago (2013-06-21 18:26:18 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/kalman@chromium.org/17104003/56003
7 years, 6 months ago (2013-06-22 04:41:41 UTC) #13
commit-bot: I haz the power
Retried try job too often on win_rel for step(s) browser_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&number=168197
7 years, 6 months ago (2013-06-22 06:22:02 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/kalman@chromium.org/17104003/56003
7 years, 6 months ago (2013-06-22 15:34:06 UTC) #15
commit-bot: I haz the power
7 years, 6 months ago (2013-06-22 15:34:19 UTC) #16
Message was sent while issue was closed.
Change committed as 208058

Powered by Google App Engine
This is Rietveld 408576698