Bug 110968 – Introduce new message sources for logging.

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Dmitry Zvorygin]

Introduce new message source for logging, and alter some message sources for existing log records, to make logging more fine-tuneable, and self-descriptive.

[Dmitry Zvorygin]

Created attachment 190488 [details]
Patch

[Pavel Feldman]

Mike, how do you like it?

[Build Bot]

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

[Mike West]

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.

[Build Bot]

Comment on attachment 190488 [details]
Patch

Attachment 190488 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/16823159

[Build Bot]

Comment on attachment 190488 [details]
Patch

Attachment 190488 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/16672367

[Pavel Feldman]

Comment on attachment 190488 [details]
Patch

r- as per Mike's comments.

[Dmitry Zvorygin]

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.

[Dmitry Zvorygin]

Created attachment 191500 [details]
Patch

[Pavel Feldman]

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.

[Dmitry Zvorygin]

Created attachment 191503 [details]
Patch

[WebKit Review Bot]

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

[Build Bot]

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

[Build Bot]

Comment on attachment 191503 [details]
Patch

Attachment 191503 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/16991216

[Build Bot]

Comment on attachment 191503 [details]
Patch

Attachment 191503 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/16998112

[Build Bot]

Comment on attachment 191503 [details]
Patch

Attachment 191503 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/17036140

[Dmitry Zvorygin]

Created attachment 191708 [details]
Patch

[WebKit Review Bot]

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

[Dmitry Zvorygin]

Created attachment 191782 [details]
Patch

[WebKit Review Bot]

Comment on attachment 191782 [details]
Patch

Clearing flags on attachment: 191782

Committed r144949: <http://trac.webkit.org/changeset/144949>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.