Bug 105009 - [EFL][DRT] Isolates GTK specific code in DumpRenderTree/gtk
Summary: [EFL][DRT] Isolates GTK specific code in DumpRenderTree/gtk
Status: RESOLVED DUPLICATE of bug 105007
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Linux
: P2 Normal
Assignee: Krzysztof Czech
URL:
Keywords:
Depends on: 107440
Blocks: 105007
  Show dependency treegraph
 
Reported: 2012-12-14 03:12 PST by Krzysztof Czech
Modified: 2013-03-01 02:50 PST (History)
8 users (show)

See Also:


Attachments
[EFL][DRT] Isolates GTK specific code in DumpRenderTree/gtk (6.06 KB, patch)
2012-12-14 03:18 PST, Krzysztof Czech
no flags Details | Formatted Diff | Diff
[EFL][DRT] Isolates GTK specific code in DumpRenderTree/gtk (6.08 KB, patch)
2012-12-14 03:27 PST, Krzysztof Czech
mrobinson: review-
Details | Formatted Diff | Diff
[EFL][DRT] Isolates GTK specific code in DumpRenderTree/gtk (3.34 KB, patch)
2013-01-16 05:09 PST, Krzysztof Czech
no flags Details | Formatted Diff | Diff
[EFL][DRT] Isolates GTK specific code in DumpRenderTree/gtk (3.40 KB, patch)
2013-01-21 06:04 PST, Krzysztof Czech
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Krzysztof Czech 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.
Comment 1 Krzysztof Czech 2012-12-14 03:18:11 PST
Created attachment 179460 [details]
[EFL][DRT] Isolates GTK specific code in DumpRenderTree/gtk
Comment 2 WebKit Review Bot 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.
Comment 3 Krzysztof Czech 2012-12-14 03:27:26 PST
Created attachment 179462 [details]
[EFL][DRT] Isolates GTK specific code in DumpRenderTree/gtk
Comment 4 Mario Sanchez Prada 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
Comment 5 Martin Robinson 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.
Comment 6 Krzysztof Czech 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.
Comment 7 Martin Robinson 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.
Comment 8 Krzysztof Czech 2013-01-16 05:09:11 PST
Created attachment 182961 [details]
[EFL][DRT] Isolates GTK specific code in DumpRenderTree/gtk
Comment 9 Krzysztof Czech 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 ?.
Comment 10 Martin Robinson 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.
Comment 11 Krzysztof Czech 2013-01-21 06:04:03 PST
Created attachment 183769 [details]
[EFL][DRT] Isolates GTK specific code in DumpRenderTree/gtk
Comment 12 Krzysztof Czech 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.
Comment 13 Krzysztof Czech 2013-01-25 07:41:33 PST

*** This bug has been marked as a duplicate of bug 105007 ***
Comment 14 Eric Seidel (no email) 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).