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

Issue 23007021: Report Javascript Runtime Errors to the Error Console (Closed)

Created:
7 years, 4 months ago by Devlin
Modified:
7 years, 3 months ago
CC:
chromium-reviews, extensions-reviews_chromium.org, joi+watch-content_chromium.org, darin-cc_chromium.org, chromium-apps-reviews_chromium.org, miu+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@dc_ec_feldman
Visibility:
Public.

Description

Report Javascript Runtime Errors to the Error Console TBR=brettw@chromium.org (moving DEPS file from extensions/common/matcher to extensions/common). BUG=21734 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=220753

Patch Set 1 : #

Patch Set 2 : #

Total comments: 19

Patch Set 3 : Yoyo's #

Total comments: 3

Patch Set 4 : Comment Condensed #

Total comments: 2

Patch Set 5 : Joi's #

Total comments: 6

Patch Set 6 : Moved StackFrame to content/public/ #

Patch Set 7 : Move StackFrame from content/, complexity still in renderer #

Patch Set 8 : #

Total comments: 4

Patch Set 9 : #

Patch Set 10 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+891 lines, -165 lines) Patch
A chrome/browser/extensions/error_console/error_console_browsertest.cc View 1 2 3 4 5 6 1 chunk +353 lines, -0 lines 0 comments Download
M chrome/browser/extensions/error_console/error_console_unittest.cc View 1 2 3 4 5 6 2 chunks +19 lines, -16 lines 0 comments Download
M chrome/browser/extensions/extension_host.h View 1 2 3 4 5 6 2 chunks +6 lines, -0 lines 0 comments Download
M chrome/browser/extensions/extension_host.cc View 1 2 3 4 5 6 5 chunks +25 lines, -0 lines 0 comments Download
M chrome/browser/extensions/tab_helper.h View 1 2 3 4 5 6 2 chunks +5 lines, -0 lines 0 comments Download
M chrome/browser/extensions/tab_helper.cc View 1 2 3 4 5 6 6 chunks +25 lines, -1 line 0 comments Download
M chrome/chrome_common.gypi View 1 2 3 4 5 6 7 8 9 2 chunks +4 lines, -0 lines 0 comments Download
M chrome/chrome_tests.gypi View 1 2 3 4 5 6 7 8 9 1 chunk +1 line, -0 lines 0 comments Download
M chrome/common/render_messages.h View 1 2 3 4 5 6 3 chunks +16 lines, -0 lines 0 comments Download
M chrome/renderer/chrome_content_renderer_client.cc View 1 2 2 chunks +2 lines, -1 line 0 comments Download
M chrome/renderer/chrome_render_view_observer.h View 1 2 3 4 5 6 7 8 2 chunks +6 lines, -0 lines 0 comments Download
M chrome/renderer/chrome_render_view_observer.cc View 1 2 3 4 5 6 7 8 8 chunks +83 lines, -4 lines 0 comments Download
A + chrome/test/data/extensions/error_console/browser_action_runtime_error/browser_action.js View 1 2 3 4 5 6 1 chunk +3 lines, -2 lines 0 comments Download
A chrome/test/data/extensions/error_console/browser_action_runtime_error/manifest.json View 1 chunk +15 lines, -0 lines 0 comments Download
A + chrome/test/data/extensions/error_console/content_script_log_and_runtime_error/content_script.js View 1 chunk +6 lines, -7 lines 0 comments Download
A chrome/test/data/extensions/error_console/content_script_log_and_runtime_error/manifest.json View 1 chunk +15 lines, -0 lines 0 comments Download
M content/browser/renderer_host/render_view_host_delegate.h View 1 2 3 4 5 6 1 chunk +1 line, -2 lines 0 comments Download
M content/browser/renderer_host/render_view_host_delegate.cc View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download
M content/browser/renderer_host/render_view_host_impl.h View 1 2 3 4 5 6 7 8 9 1 chunk +1 line, -2 lines 0 comments Download
M content/browser/renderer_host/render_view_host_impl.cc View 1 2 3 4 5 6 7 8 9 1 chunk +3 lines, -5 lines 0 comments Download
M content/browser/web_contents/web_contents_impl.h View 1 2 3 4 5 6 1 chunk +1 line, -2 lines 0 comments Download
M content/browser/web_contents/web_contents_impl.cc View 1 2 3 4 5 6 7 8 9 1 chunk +1 line, -2 lines 0 comments Download
M content/common/view_messages.h View 1 2 3 4 5 6 1 chunk +2 lines, -3 lines 0 comments Download
M content/public/renderer/render_view_observer.h View 1 2 3 4 5 6 7 8 1 chunk +12 lines, -0 lines 0 comments Download
M content/renderer/render_view_impl.cc View 1 2 3 4 5 6 7 8 9 1 chunk +12 lines, -2 lines 0 comments Download
M extensions/browser/extension_error.h View 1 2 3 4 5 6 2 chunks +11 lines, -34 lines 0 comments Download
M extensions/browser/extension_error.cc View 1 2 3 4 5 6 4 chunks +28 lines, -69 lines 0 comments Download
A + extensions/common/DEPS View 1 2 3 4 5 6 7 0 chunks +-1 lines, --1 lines 0 comments Download
M extensions/common/constants.h View 1 2 1 chunk +3 lines, -3 lines 0 comments Download
A extensions/common/extension_urls.h View 1 2 1 chunk +20 lines, -0 lines 0 comments Download
A + extensions/common/extension_urls.cc View 1 2 1 chunk +6 lines, -7 lines 0 comments Download
D extensions/common/matcher/DEPS View 1 2 3 4 5 6 7 1 chunk +0 lines, -3 lines 0 comments Download
A extensions/common/stack_frame.h View 1 2 3 4 5 6 7 8 1 chunk +41 lines, -0 lines 0 comments Download
A extensions/common/stack_frame.cc View 1 2 3 4 5 6 7 1 chunk +80 lines, -0 lines 0 comments Download
A extensions/common/stack_frame_unittest.cc View 1 2 3 4 5 6 7 1 chunk +85 lines, -0 lines 0 comments Download

Messages

Total messages: 35 (0 generated)
Cris Neckar
7 years, 4 months ago (2013-08-21 22:49:25 UTC) #1
Devlin
Yoyo, please take a look when you get a chance. Also, there's a little overlap ...
7 years, 4 months ago (2013-08-23 19:47:07 UTC) #2
Devlin
https://codereview.chromium.org/23007021/diff/12001/content/browser/web_contents/web_contents_impl.cc File content/browser/web_contents/web_contents_impl.cc (right): https://codereview.chromium.org/23007021/diff/12001/content/browser/web_contents/web_contents_impl.cc#newcode3430 content/browser/web_contents/web_contents_impl.cc:3430: FOR_EACH_OBSERVER(WebContentsObserver, Cris - RenderViewHostImpl receives the IPC message, and ...
7 years, 4 months ago (2013-08-23 20:06:50 UTC) #3
Cris Neckar
On 2013/08/23 20:06:50, D Cronin wrote: > https://codereview.chromium.org/23007021/diff/12001/content/browser/web_contents/web_contents_impl.cc > File content/browser/web_contents/web_contents_impl.cc (right): > > https://codereview.chromium.org/23007021/diff/12001/content/browser/web_contents/web_contents_impl.cc#newcode3430 ...
7 years, 4 months ago (2013-08-23 20:08:14 UTC) #4
Yoyo Zhou
mostly LGTM https://chromiumcodereview.appspot.com/23007021/diff/12001/chrome/browser/extensions/error_console/error_console_browsertest.cc File chrome/browser/extensions/error_console/error_console_browsertest.cc (right): https://chromiumcodereview.appspot.com/23007021/diff/12001/chrome/browser/extensions/error_console/error_console_browsertest.cc#newcode43 chrome/browser/extensions/error_console/error_console_browsertest.cc:43: string16 (*const U8To16)(const base::StringPiece&) = &base::UTF8ToUTF16; This ...
7 years, 4 months ago (2013-08-23 22:54:54 UTC) #5
Devlin
https://codereview.chromium.org/23007021/diff/12001/chrome/browser/extensions/error_console/error_console_browsertest.cc File chrome/browser/extensions/error_console/error_console_browsertest.cc (right): https://codereview.chromium.org/23007021/diff/12001/chrome/browser/extensions/error_console/error_console_browsertest.cc#newcode43 chrome/browser/extensions/error_console/error_console_browsertest.cc:43: string16 (*const U8To16)(const base::StringPiece&) = &base::UTF8ToUTF16; On 2013/08/23 22:54:54, ...
7 years, 4 months ago (2013-08-23 23:44:30 UTC) #6
Yoyo Zhou
LGTM https://codereview.chromium.org/23007021/diff/12001/chrome/browser/extensions/extension_host.cc File chrome/browser/extensions/extension_host.cc (right): https://codereview.chromium.org/23007021/diff/12001/chrome/browser/extensions/extension_host.cc#newcode434 chrome/browser/extensions/extension_host.cc:434: if (IsSourceFromAnExtension(source)) { On 2013/08/23 23:44:30, D Cronin ...
7 years, 4 months ago (2013-08-23 23:55:04 UTC) #7
Devlin
https://codereview.chromium.org/23007021/diff/22001/extensions/browser/extension_error.cc File extensions/browser/extension_error.cc (right): https://codereview.chromium.org/23007021/diff/22001/extensions/browser/extension_error.cc#newcode181 extensions/browser/extension_error.cc:181: // our own code up until the error catch). ...
7 years, 4 months ago (2013-08-24 00:00:32 UTC) #8
Devlin
+sky@ - chrome/renderer/chrome_content_renderer_client.cc, content/browser/web_contents/web_contents_impl.cc +joi@ - content/public/browser/web_contents_observer.h
7 years, 4 months ago (2013-08-24 00:03:01 UTC) #9
Yoyo Zhou
https://codereview.chromium.org/23007021/diff/22001/extensions/browser/extension_error.cc File extensions/browser/extension_error.cc (right): https://codereview.chromium.org/23007021/diff/22001/extensions/browser/extension_error.cc#newcode181 extensions/browser/extension_error.cc:181: // our own code up until the error catch). ...
7 years, 4 months ago (2013-08-24 00:03:17 UTC) #10
Jói
https://codereview.chromium.org/23007021/diff/41001/content/public/browser/web_contents_observer.h File content/public/browser/web_contents_observer.h (right): https://codereview.chromium.org/23007021/diff/41001/content/public/browser/web_contents_observer.h#newcode271 content/public/browser/web_contents_observer.h:271: virtual void OnMessageAddedToConsole(const base::string16& source, Perhaps the semantics of ...
7 years, 4 months ago (2013-08-25 16:36:43 UTC) #11
Devlin
https://codereview.chromium.org/23007021/diff/41001/content/public/browser/web_contents_observer.h File content/public/browser/web_contents_observer.h (right): https://codereview.chromium.org/23007021/diff/41001/content/public/browser/web_contents_observer.h#newcode271 content/public/browser/web_contents_observer.h:271: virtual void OnMessageAddedToConsole(const base::string16& source, On 2013/08/25 16:36:43, Jói ...
7 years, 3 months ago (2013-08-26 16:32:34 UTC) #12
Jói
LGTM, thanks.
7 years, 3 months ago (2013-08-26 18:26:52 UTC) #13
Cris Neckar
https://codereview.chromium.org/23007021/diff/48001/extensions/browser/extension_error.cc File extensions/browser/extension_error.cc (right): https://codereview.chromium.org/23007021/diff/48001/extensions/browser/extension_error.cc#newcode112 extensions/browser/extension_error.cc:112: if (!re2::RE2::FullMatch(text, I don't much like that we are ...
7 years, 3 months ago (2013-08-27 19:10:46 UTC) #14
Devlin
https://codereview.chromium.org/23007021/diff/48001/extensions/browser/extension_error.cc File extensions/browser/extension_error.cc (right): https://codereview.chromium.org/23007021/diff/48001/extensions/browser/extension_error.cc#newcode112 extensions/browser/extension_error.cc:112: if (!re2::RE2::FullMatch(text, On 2013/08/27 19:10:47, Cris Neckar wrote: > ...
7 years, 3 months ago (2013-08-27 19:52:47 UTC) #15
Cris Neckar
On 2013/08/27 19:52:47, D Cronin wrote: > https://codereview.chromium.org/23007021/diff/48001/extensions/browser/extension_error.cc > File extensions/browser/extension_error.cc (right): > > https://codereview.chromium.org/23007021/diff/48001/extensions/browser/extension_error.cc#newcode112 ...
7 years, 3 months ago (2013-08-27 19:54:57 UTC) #16
Devlin
https://codereview.chromium.org/23007021/diff/48001/extensions/browser/extension_error.cc File extensions/browser/extension_error.cc (right): https://codereview.chromium.org/23007021/diff/48001/extensions/browser/extension_error.cc#newcode112 extensions/browser/extension_error.cc:112: if (!re2::RE2::FullMatch(text, On 2013/08/27 19:10:47, Cris Neckar wrote: > ...
7 years, 3 months ago (2013-08-27 23:51:33 UTC) #17
Jói
> https://codereview.chromium.org/23007021/diff/58001/content/common/stack_frame_unittest.cc#newcode1 > content/common/stack_frame_unittest.cc:1: // Copyright 2013 The Chromium > Authors. All rights reserved. > ...
7 years, 3 months ago (2013-08-28 07:37:48 UTC) #18
Cris Neckar
On 2013/08/28 07:37:48, Jói wrote: > > > https://codereview.chromium.org/23007021/diff/58001/content/common/stack_frame_unittest.cc#newcode1 > > content/common/stack_frame_unittest.cc:1: // Copyright 2013 ...
7 years, 3 months ago (2013-08-28 18:02:56 UTC) #19
Devlin
+jam, since we're now adding a (small) new class to content/public/.
7 years, 3 months ago (2013-08-28 18:32:44 UTC) #20
jam
On 2013/08/28 18:32:44, D Cronin wrote: > +jam, since we're now adding a (small) new ...
7 years, 3 months ago (2013-08-28 19:34:04 UTC) #21
jam
On 2013/08/28 19:34:04, jam wrote: > On 2013/08/28 18:32:44, D Cronin wrote: > > +jam, ...
7 years, 3 months ago (2013-08-28 19:43:33 UTC) #22
Cris Neckar
On 2013/08/28 19:43:33, jam wrote: > On 2013/08/28 19:34:04, jam wrote: > > On 2013/08/28 ...
7 years, 3 months ago (2013-08-28 19:53:35 UTC) #23
Devlin
On 2013/08/28 19:53:35, Cris Neckar wrote: > On 2013/08/28 19:43:33, jam wrote: > > On ...
7 years, 3 months ago (2013-08-28 20:18:48 UTC) #24
jam
On 2013/08/28 20:18:48, D Cronin wrote: > On 2013/08/28 19:53:35, Cris Neckar wrote: > > ...
7 years, 3 months ago (2013-08-29 05:23:59 UTC) #25
Yoyo Zhou
On 2013/08/29 05:23:59, jam wrote: > On 2013/08/28 20:18:48, D Cronin wrote: > > [1] ...
7 years, 3 months ago (2013-08-29 14:08:09 UTC) #26
jam
https://codereview.chromium.org/23007021/diff/48001/extensions/browser/extension_error.cc File extensions/browser/extension_error.cc (right): https://codereview.chromium.org/23007021/diff/48001/extensions/browser/extension_error.cc#newcode112 extensions/browser/extension_error.cc:112: if (!re2::RE2::FullMatch(text, On 2013/08/27 19:10:47, Cris Neckar wrote: > ...
7 years, 3 months ago (2013-08-29 16:10:31 UTC) #27
jschuh
On 2013/08/29 16:10:31, jam wrote: > https://codereview.chromium.org/23007021/diff/48001/extensions/browser/extension_error.cc > File extensions/browser/extension_error.cc (right): > > https://codereview.chromium.org/23007021/diff/48001/extensions/browser/extension_error.cc#newcode112 > ...
7 years, 3 months ago (2013-08-29 17:22:36 UTC) #28
Devlin
On 2013/08/29 17:22:36, Justin Schuh wrote: > On 2013/08/29 16:10:31, jam wrote: > > > ...
7 years, 3 months ago (2013-08-29 19:09:11 UTC) #29
jam
https://codereview.chromium.org/23007021/diff/82001/content/public/renderer/render_view_observer.h File content/public/renderer/render_view_observer.h (right): https://codereview.chromium.org/23007021/diff/82001/content/public/renderer/render_view_observer.h#newcode93 content/public/renderer/render_view_observer.h:93: virtual void OnDetailedConsoleMessageAdded(const base::string16& message, this doesn't match this ...
7 years, 3 months ago (2013-08-30 16:04:38 UTC) #30
Devlin
https://codereview.chromium.org/23007021/diff/82001/content/public/renderer/render_view_observer.h File content/public/renderer/render_view_observer.h (right): https://codereview.chromium.org/23007021/diff/82001/content/public/renderer/render_view_observer.h#newcode93 content/public/renderer/render_view_observer.h:93: virtual void OnDetailedConsoleMessageAdded(const base::string16& message, On 2013/08/30 16:04:39, jam ...
7 years, 3 months ago (2013-08-30 16:33:37 UTC) #31
sky
LGTM
7 years, 3 months ago (2013-08-30 19:47:51 UTC) #32
jam
lgtm
7 years, 3 months ago (2013-08-30 20:48:29 UTC) #33
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rdevlin.cronin@chromium.org/23007021/86001
7 years, 3 months ago (2013-08-30 23:15:44 UTC) #34
commit-bot: I haz the power
7 years, 3 months ago (2013-09-01 23:05:14 UTC) #35
Message was sent while issue was closed.
Change committed as 220753

Powered by Google App Engine
This is Rietveld 408576698