Bug 96500

Summary: [WK2] Improve log channel decision routine on WebKit
Product: WebKit Reporter: KwangYong Choi <ky0.choi>
Component: WebKit2Assignee: KwangYong Choi <ky0.choi>
Status: RESOLVED INVALID    
Severity: Normal CC: abecsi, ap, cmarcelo, menard, rakuco, webkit.review.bot, zoltan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 96478    
Attachments:
Description Flags
Patch ap: review-

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.