| Summary: | Introduce new message sources for logging. | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Dmitry Zvorygin <zvorygin> | ||||||||||||
| Component: | WebCore Misc. | Assignee: | Nobody <webkit-unassigned> | ||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||
| Severity: | Normal | CC: | buildbot, mkwst, pfeldman, rniwa, timothy, webkit.review.bot | ||||||||||||
| Priority: | P2 | ||||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||
| Hardware: | Unspecified | ||||||||||||||
| OS: | Unspecified | ||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Dmitry Zvorygin
2013-02-27 04:04:01 PST
Created attachment 190488 [details]
Patch
Mike, how do you like it? Comment on attachment 190488 [details] Patch Attachment 190488 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-commit-queue.appspot.com/results/16769785 Comment on attachment 190488 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=190488&action=review This is a great start, thank you! I like the direction... a few comments inline. > Source/WebCore/Modules/webdatabase/DatabaseBase.cpp:49 > + m_scriptExecutionContext->addConsoleMessage(OfflineMessageSource, ErrorMessageLevel, message); Why "Offline" here? I'd suggest something like "Storage", which would also allow us to change IDB messages. > Source/WebCore/html/HTMLFormElement.cpp:264 > + document()->addConsoleMessage(JSMessageSource, ErrorMessageLevel, message); Let's leave this as RenderingMessageSource, and probably change the message in a separate patch, as it's confusing. This error occurs when form validation would focus on a required element that, for whatever reason, isn't focusable. LayoutTests/fast/forms/interactive-validation-prevented.html has an example. > Source/WebCore/html/HTMLIFrameElement.cpp:93 > + document()->addConsoleMessage(SecurityMessageSource, ErrorMessageLevel, "Error while parsing the 'sandbox' attribute: " + invalidTokens); This seems more like a rendering/parsing error than a security error. The sandbox isn't blocking anything, it's not being parsed correctly. > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:5314 > + document->addConsoleMessage(RenderingMessageSource, WarningMessageLevel, message); It might make sense to give WebGL errors a source of their own, if we're going to be enabling filtering on them later. > Source/WebCore/loader/MixedContentChecker.cpp:102 > // FIXME: Why does this message not have a source URL or a line number? webkit.org/b/97979 Since you're here, please kill this FIXME. We fixed it when adding stack traces to messages globally. > Source/WebCore/svg/SVGDocumentExtensions.cpp:138 > + document->addConsoleMessage(RenderingMessageSource, level, message); SVG might be another reason (alongside WebGL from above) to add a GraphicsMessageSource or something similar. Comment on attachment 190488 [details] Patch Attachment 190488 [details] did not pass mac-ews (mac): Output: http://webkit-commit-queue.appspot.com/results/16823159 Comment on attachment 190488 [details] Patch Attachment 190488 [details] did not pass mac-ews (mac): Output: http://webkit-commit-queue.appspot.com/results/16672367 Comment on attachment 190488 [details]
Patch
r- as per Mike's comments.
Comment on attachment 190488 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=190488&action=review >> Source/WebCore/Modules/webdatabase/DatabaseBase.cpp:49 >> + m_scriptExecutionContext->addConsoleMessage(OfflineMessageSource, ErrorMessageLevel, message); > > Why "Offline" here? I'd suggest something like "Storage", which would also allow us to change IDB messages. Done. >> Source/WebCore/html/HTMLFormElement.cpp:264 >> + document()->addConsoleMessage(JSMessageSource, ErrorMessageLevel, message); > > Let's leave this as RenderingMessageSource, and probably change the message in a separate patch, as it's confusing. This error occurs when form validation would focus on a required element that, for whatever reason, isn't focusable. LayoutTests/fast/forms/interactive-validation-prevented.html has an example. Ok. >> Source/WebCore/html/HTMLIFrameElement.cpp:93 >> + document()->addConsoleMessage(SecurityMessageSource, ErrorMessageLevel, "Error while parsing the 'sandbox' attribute: " + invalidTokens); > > This seems more like a rendering/parsing error than a security error. The sandbox isn't blocking anything, it's not being parsed correctly. Ok. >> Source/WebCore/html/canvas/WebGLRenderingContext.cpp:5314 >> + document->addConsoleMessage(RenderingMessageSource, WarningMessageLevel, message); > > It might make sense to give WebGL errors a source of their own, if we're going to be enabling filtering on them later. Currently we aren't going to have separate webgl filter. >> Source/WebCore/loader/MixedContentChecker.cpp:102 >> // FIXME: Why does this message not have a source URL or a line number? webkit.org/b/97979 > > Since you're here, please kill this FIXME. We fixed it when adding stack traces to messages globally. Done. >> Source/WebCore/svg/SVGDocumentExtensions.cpp:138 >> + document->addConsoleMessage(RenderingMessageSource, level, message); > > SVG might be another reason (alongside WebGL from above) to add a GraphicsMessageSource or something similar. Maybe later, if somebody provides well reviewed list of required message sources. Created attachment 191500 [details]
Patch
Comment on attachment 191500 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=191500&action=review > Source/WebCore/page/Console.cpp:104 > + sourceString = "OFFLINE"; Copy paste. Created attachment 191503 [details]
Patch
Comment on attachment 191503 [details] Patch Rejecting attachment 191503 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=webkit-commit-queue.appspot.com', '--bot-id=gce-cq-02', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 191503, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue Last 500 characters of output: t/git/webkit-commit-queue/Source/WebKit/chromium/v8 --revision 13779 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' 54>At revision 13779. ________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' ________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' Updating webkit projects from gyp files... Full output: http://webkit-commit-queue.appspot.com/results/17017009 Comment on attachment 191503 [details] Patch Attachment 191503 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-commit-queue.appspot.com/results/17060012 Comment on attachment 191503 [details] Patch Attachment 191503 [details] did not pass mac-ews (mac): Output: http://webkit-commit-queue.appspot.com/results/16991216 Comment on attachment 191503 [details] Patch Attachment 191503 [details] did not pass mac-ews (mac): Output: http://webkit-commit-queue.appspot.com/results/16998112 Comment on attachment 191503 [details] Patch Attachment 191503 [details] did not pass mac-ews (mac): Output: http://webkit-commit-queue.appspot.com/results/17036140 Created attachment 191708 [details]
Patch
Comment on attachment 191708 [details] Patch Rejecting attachment 191708 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=webkit-commit-queue.appspot.com', '--bot-id=gce-cq-04', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 191708, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue Last 500 characters of output: t/git/webkit-commit-queue/Source/WebKit/chromium/v8 --revision 13779 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' 54>At revision 13779. ________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' ________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' Updating webkit projects from gyp files... Full output: http://webkit-commit-queue.appspot.com/results/17079161 Created attachment 191782 [details]
Patch
Comment on attachment 191782 [details] Patch Clearing flags on attachment: 191782 Committed r144949: <http://trac.webkit.org/changeset/144949> All reviewed patches have been landed. Closing bug. |