Bug 24264 - GTK+ port needs proper logging
Summary: GTK+ port needs proper logging
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on:
Blocks:
 
Reported: 2009-02-28 13:59 PST by Gustavo Noronha (kov)
Modified: 2009-03-08 12:37 PDT (History)
1 user (show)

See Also:


Attachments
enables logging (6.17 KB, patch)
2009-02-28 14:00 PST, Gustavo Noronha (kov)
no flags Details | Formatted Diff | Diff
second try (6.47 KB, patch)
2009-03-02 08:48 PST, Gustavo Noronha (kov)
zecke: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gustavo Noronha (kov) 2009-02-28 13:59:10 PST
GTK+ currently logs only not implemented messages. Enabling logging of other log domains should make it much easier to figure out how things work and on debugging.
Comment 1 Gustavo Noronha (kov) 2009-02-28 14:00:16 PST
Created attachment 28126 [details]
enables logging
Comment 2 Holger Freyther 2009-03-01 07:10:18 PST
Comment on attachment 28126 [details]
enables logging


> +#include <glib.h>
> +#include <string.h>
> +
>  namespace WebCore {
>  
> +// Inspired by the code used by the Qt port
> +
> +static WTFLogChannel* getChannelFromName(const char* channelName)
> +{
> +    if (!strlen(channelName) >= 2)
> +        return 0;

aeh? if !strlen() >= 2? to be honest even if the semantic is like !(strlen() <= 2) you might be better of with just a if (strlen() < 3) return 0;




> +
> +    if (!g_ascii_strcasecmp(channelName, "BackForward")) return &LogBackForward;

You will have to turn these into a new line.



> -    // FIXME: Add a way for the user to specify which
> -    // logs he/she would like turned on.
> +    static bool haveInitializedLoggingChannels = false;
> +    if (haveInitializedLoggingChannels)
> +        return;
> +
> +    haveInitializedLoggingChannels = true;

okay, I think it has to be hasIniti...


;
> +
> +    // to disable logging notImplemented set the DISABLE_NI_WARNING
> +    // environment variable to 1
>      LogNotYetImplemented.state = WTFLogChannelOn;




> -    const char* webkit_debug = g_getenv("WEBKIT_DEBUG"); 
>      if (!soup_session_get_feature(session, SOUP_TYPE_LOGGER)
> -        && webkit_debug && !strcmp(webkit_debug, "network")) {
> +        && (LogNetwork.state == WTFLogChannelOn)) {
I think you can omit the () here..
>
Comment 3 Gustavo Noronha (kov) 2009-03-02 08:48:40 PST
Created attachment 28178 [details]
second try
Comment 4 Gustavo Noronha (kov) 2009-03-02 08:51:22 PST
(In reply to comment #2)
> (From update of attachment 28126 [details] [review])
> > +    haveInitializedLoggingChannels = true;
> 
> okay, I think it has to be hasIniti...

I changed this to loggingChannelsInitialized. Looking at the style guide I think maybe I should change it to 'didInitializeLoggingChannels' instead? I can make another change for a patch or when landing if you agree.
Comment 5 Holger Freyther 2009-03-07 05:04:14 PST
Comment on attachment 28178 [details]
second try

Do the variable rename you proposed, and it would be cool if you could unite Qt and Gtk+ in this respect, e.g. by using CString for getChannelFromName
Comment 6 Gustavo Noronha (kov) 2009-03-08 12:37:08 PDT
Landed as r41520. I will work on a patch to unite, or at least backport style fixes to the Qt implementation this week.