Bug 105009

Summary: [EFL][DRT] Isolates GTK specific code in DumpRenderTree/gtk
Product: WebKit Reporter: Krzysztof Czech <k.czech>
Component: WebKit EFLAssignee: Krzysztof Czech <k.czech>
Status: RESOLVED DUPLICATE    
Severity: Normal CC: apinheiro, g.czajkowski, jdiggs, laszlo.gombos, lucas.de.marchi, mario, mrobinson, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Linux   
Bug Depends on: 107440    
Bug Blocks: 105007    
Attachments:
Description Flags
[EFL][DRT] Isolates GTK specific code in DumpRenderTree/gtk
none
[EFL][DRT] Isolates GTK specific code in DumpRenderTree/gtk
mrobinson: review-
[EFL][DRT] Isolates GTK specific code in DumpRenderTree/gtk
none
[EFL][DRT] Isolates GTK specific code in DumpRenderTree/gtk none

Krzysztof Czech
Reported 2012-12-14 03:12:32 PST
Isolates some specific GTK code. It's a part of 105007 bug with sharing DumpRenderTree's implementation among other ports.
Attachments
[EFL][DRT] Isolates GTK specific code in DumpRenderTree/gtk (6.06 KB, patch)
2012-12-14 03:18 PST, Krzysztof Czech 		no flags
DetailsFormatted DiffDiff
[EFL][DRT] Isolates GTK specific code in DumpRenderTree/gtk (6.08 KB, patch)
2012-12-14 03:27 PST, Krzysztof Czech 		mrobinson: review-
DetailsFormatted DiffDiff
[EFL][DRT] Isolates GTK specific code in DumpRenderTree/gtk (3.34 KB, patch)
2013-01-16 05:09 PST, Krzysztof Czech 		no flags
DetailsFormatted DiffDiff
[EFL][DRT] Isolates GTK specific code in DumpRenderTree/gtk (3.40 KB, patch)
2013-01-21 06:04 PST, Krzysztof Czech 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Krzysztof Czech
Comment 1 2012-12-14 03:18:11 PST
Created attachment 179460 [details] [EFL][DRT] Isolates GTK specific code in DumpRenderTree/gtk
WebKit Review Bot
Comment 2 2012-12-14 03:22:07 PST
Attachment 179460 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Tools/ChangeLog', u'Tools/DumpRenderTree/g..." exit_code: 1 Tools/ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Krzysztof Czech
Comment 3 2012-12-14 03:27:26 PST
Created attachment 179462 [details] [EFL][DRT] Isolates GTK specific code in DumpRenderTree/gtk
Mario Sanchez Prada
Comment 4 2012-12-14 04:27:34 PST
Comment on attachment 179462 [details] [EFL][DRT] Isolates GTK specific code in DumpRenderTree/gtk Looks good to me
Martin Robinson
Comment 5 2012-12-16 00:46:48 PST
Comment on attachment 179462 [details] [EFL][DRT] Isolates GTK specific code in DumpRenderTree/gtk Instead of first splicing a file in the directory with #ifdefs I think it makes sense to move shared code to a shared directory, while keeping platform-specific methods or functions in the GTK+ directory. This is the structure that many classes in WebCore use. It is based on the fact that the implementation of various class methods can be spread across compilation units.
Krzysztof Czech
Comment 6 2012-12-17 03:05:42 PST
I think you are right about keeping platform specific methods in GTK+ directory. Basically those methods where I used PLATFORM(GTK) ifdefs will be also shared. The only difference is call to DumpRenderTreeSupportGtk/Efl specific method. I have already started implementing those missing bits from DumpRenderTreeSupporEfl.
Martin Robinson
Comment 7 2012-12-17 03:18:22 PST
(In reply to comment #6) > I think you are right about keeping platform specific methods in GTK+ directory. > Basically those methods where I used PLATFORM(GTK) ifdefs will be also shared. The only difference is call to DumpRenderTreeSupportGtk/Efl specific method. > I have already started implementing those missing bits from DumpRenderTreeSupporEfl. Instead of sharing methods with #ifdefs, I'm suggesting creating new methods that hide the platform-specific behavior.
Krzysztof Czech
Comment 8 2013-01-16 05:09:11 PST
Created attachment 182961 [details] [EFL][DRT] Isolates GTK specific code in DumpRenderTree/gtk
Krzysztof Czech
Comment 9 2013-01-16 05:13:12 PST
(In reply to comment #7) > (In reply to comment #6) > > I think you are right about keeping platform specific methods in GTK+ directory. > > Basically those methods where I used PLATFORM(GTK) ifdefs will be also shared. The only difference is call to DumpRenderTreeSupportGtk/Efl specific method. > > I have already started implementing those missing bits from DumpRenderTreeSupporEfl. > > Instead of sharing methods with #ifdefs, I'm suggesting creating new methods that hide the platform-specific behavior. I applied you suggestions. I tried to minimize usage of #ifdefs. Used them only in AccessibilityCallbacks.cpp. What do you think ?.
Martin Robinson
Comment 10 2013-01-16 06:19:49 PST
Comment on attachment 182961 [details] [EFL][DRT] Isolates GTK specific code in DumpRenderTree/gtk It's hard to understand this change by itself. Perhaps it would be better to include the addition of the EFL bits here as well.
Krzysztof Czech
Comment 11 2013-01-21 06:04:03 PST
Created attachment 183769 [details] [EFL][DRT] Isolates GTK specific code in DumpRenderTree/gtk
Krzysztof Czech
Comment 12 2013-01-21 06:07:25 PST
I included additional EFL bits. > (From update of attachment 182961 [details]) > It's hard to understand this change by itself. Perhaps it would be better to include the addition of the EFL bits here as well.
Krzysztof Czech
Comment 13 2013-01-25 07:41:33 PST
*** This bug has been marked as a duplicate of bug 105007 ***
Eric Seidel (no email)
Comment 14 2013-03-01 02:50:39 PST
Comment on attachment 183769 [details] [EFL][DRT] Isolates GTK specific code in DumpRenderTree/gtk Cleared review? from attachment 183769 [details] so that this bug does not appear in http://webkit.org/pending-review. If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).
Note You need to log in before you can comment on or make changes to this bug.