Bug 96500 - [WK2] Improve log channel decision routine on WebKit
Summary: [WK2] Improve log channel decision routine on WebKit
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: KwangYong Choi
URL:
Keywords:
Depends on:
Blocks: 96478
  Show dependency treegraph
 
Reported: 2012-09-12 05:20 PDT by KwangYong Choi
Modified: 2012-09-12 23:39 PDT (History)
7 users (show)

See Also:


Attachments
Patch (7.17 KB, patch)
2012-09-12 05:36 PDT, KwangYong Choi
ap: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description KwangYong Choi 2012-09-12 05:20:18 PDT
initializeLogChannel() on GTK and QT on WebKit2 is not effective. It's not necessary to call initializeLogChannel() for every log channels.
Comment 1 KwangYong Choi 2012-09-12 05:36:20 PDT
Created attachment 163603 [details]
Patch
Comment 2 Alexey Proskuryakov 2012-09-12 13:32:44 PDT
Comment on attachment 163603 [details]
Patch

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

This

> Source/WebKit2/Platform/mac/Logging.mac.mm:54
> +void platformInitializeLogChannels()
> +{
> +    initializeLogChannel(&LogContextMenu);
> +    initializeLogChannel(&LogIconDatabase);
> +    initializeLogChannel(&LogKeyHandling);
> +    initializeLogChannel(&LogSessionState);
> +    initializeLogChannel(&LogTextInput);
> +    initializeLogChannel(&LogView);

As a result of this patch, each platform is responsible for knowing about all log channels, not just about how to initialize them. This is not maintainable - adding a logging channel should not require changing platform specific code.
Comment 3 KwangYong Choi 2012-09-12 18:37:18 PDT
Comment on attachment 163603 [details]
Patch

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

>> Source/WebKit2/Platform/mac/Logging.mac.mm:54
>> +    initializeLogChannel(&LogView);
> 
> As a result of this patch, each platform is responsible for knowing about all log channels, not just about how to initialize them. This is not maintainable - adding a logging channel should not require changing platform specific code.

The reason why I am not changing the routine of Mac port is because its routine in WebCore is same. And I'm not familiar with Objective-C.

GTK and QT will find the enabled log channels based on environment variable. It need not to be executed if there is no environment variable.
Comment 4 Alexey Proskuryakov 2012-09-12 20:28:07 PDT
This issue needs to be addressed in a way that doesn't introduce the design problem I pointed out. Perhaps Gtk and Qt could cache some results to avoid recalculating them unnecessarily. 

Also, I'm not sure why this is a problem to begin with, as this code is debug only.
Comment 5 KwangYong Choi 2012-09-12 23:39:06 PDT
(In reply to comment #4)
> This issue needs to be addressed in a way that doesn't introduce the design problem I pointed out. Perhaps Gtk and Qt could cache some results to avoid recalculating them unnecessarily. 
> 
> Also, I'm not sure why this is a problem to begin with, as this code is debug only.

I just wanted to add this feature to EFL port. And I think, it can be modified to WebCore style implementation before I add it to EFL port.

I will found out how to add this feature to EFL without changing previous implementation.

Thank you for review.