Bug 24264

Summary: GTK+ port needs proper logging
Product: WebKit Reporter: Gustavo Noronha (kov) <gns>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: pochu27
Priority: P2 Keywords: Gtk
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
enables logging
none
second try zecke: review+

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.