Bug 79918 – [GTK] Add WebKitWebPage::console-message-sent signal to Web Extensions API

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Carlos Garcia Campos]

It will be useful to get information about javascript exceptions an other console messages. See also bug #75543.

[Carlos Garcia Campos]

Created attachment 129460 [details]
Patch

[Carlos Garcia Campos]

Created attachment 134591 [details]
New patch

I've updated the patch to move the WebKitConsoleMessage boxed type implementation to its own file, following the same approach than WebKitJavaScriptResult and WebKitScriptDialog boxed types.

[WebKit Review Bot]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Philippe Normand]

Comment on attachment 134591 [details]
New patch

Attachment 134591 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/12227001

[Carlos Garcia Campos]

Patch doesn't build because it depends on bug #79482

[Joone Hur]

Created attachment 174024 [details]
rebased patch

[Carlos Garcia Campos]

Created attachment 174123 [details]
Updated patch

We don't use the C API internally anymore.

[WebKit Review Bot]

Attachment 174123 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1262:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1263:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1264:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1265:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1266:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1267:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1268:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1269:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WARNING: File exempt from style guide. Skipping: "Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h"
WARNING: File exempt from style guide. Skipping: "Source/WebKit2/UIProcess/API/gtk/WebKitConsoleMessage.h"
WARNING: File exempt from style guide. Skipping: "Source/WebKit2/UIProcess/API/gtk/webkit2.h"
Total errors found: 8 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[kov's GTK+ EWS bot]

Comment on attachment 174123 [details]
Updated patch

Attachment 174123 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/14832365

[Gustavo Noronha (kov)]

Comment on attachment 174123 [details]
Updated patch

View in context: https://bugs.webkit.org/attachment.cgi?id=174123&action=review

> Source/WebKit2/UIProcess/API/gtk/WebKitConsoleMessage.h:39
> + * @WEBKIT_CONSOLE_MESSAGE_SOURCE_NETWORK: Message produced by WebSockets.

Wondering if it would make sense to use WEBSOCKETS instead of NETWORK here, but I guess using NETWORK makes this more future-proof, as in, it could be used for other features that deal with network connections, even though that would mean a slight change in behaviour, meaning it would no longer mean websockets.

> Source/WebKit2/UIProcess/API/gtk/WebKitUIClient.cpp:161
> +static void addMessageToConsole(WKPageRef page, WKMessageSourceType source, WKMessageLevelType level, WKStringRef message, unsigned lineNumber, WKStringRef sourceID, const void* clientInfo)

The build fails, apparently we should use WebCore::MessageSource, WebCore::MessageType, and WebCore::MessageLevel here.

[Carlos Garcia Campos]

Build fails because this depends on another patch, that unfortunately was rejected and the bug closed as wont fix. We'll have to use injected bundle to expose this api.

[Carlos Garcia Campos]

I'm giving this another try, this time moving the API to the web extensions. This way we avoid the IPC traffic, users can handle the messages in their web extension or send them to the UI process. I think it would be better to use console-message-sent, instead of console-message, since this signal is just a notification, not an action that the user can handle or prevent.

[Carlos Garcia Campos]

Created attachment 260576 [details]
Patch

[Csaba Osztrogonác]

Comment on attachment 174123 [details]
Updated patch

Cleared Gustavo Noronha Silva's review+ from obsolete attachment 174123 [details] so that this bug does not appear in http://webkit.org/pending-commit.

[Carlos Garcia Campos]

Created attachment 263140 [details]
Updated patch

Fixed the ( used in HTTP error message, I noticed I was using ) ) instead of ( )

[Carlos Garcia Campos]

ping reviewers, Gustavo?

[Gustavo Noronha (kov)]

Comment on attachment 263140 [details]
Updated patch

View in context: https://bugs.webkit.org/attachment.cgi?id=263140&action=review

> Source/WebKit2/ChangeLog:34
> +        (didReceiveResponseForResource): Generate a console message in
> +        case of HTTP failure >= 400 for consistency with the inspector.

Nice touch =)

[Carlos Garcia Campos]

Committed r193620: <http://trac.webkit.org/changeset/193620>

[Michael Catanzaro]

It broke distcheck:

/home/mcatanzaro/src/WebKit/WebKitBuild/GNOME/Documentation/webkit2gtk-4.0/webkit2gtk-docs.sgml:65: element include: XInclude error : could not load /home/mcatanzaro/src/WebKit/WebKitBuild/GNOME/Documentation/webkit2gtk-4.0/xml/WebKitConsoleMessage, and no fallback was found

[Michael Catanzaro]

Committed r194280: <http://trac.webkit.org/changeset/194280>