Bug 20736 - [GTK] GtkWebKit incompatible with rgba colormaps
: [GTK] GtkWebKit incompatible with rgba colormaps
Status: RESOLVED FIXED
: WebKit
WebKit Gtk
: 528+ (Nightly build)
: Other Linux
: P2 Normal
Assigned To:
: http://www.slello.com/tmp/gtkwebkit-r...
: Gtk
:
:
  Show dependency treegraph
 
Reported: 2008-09-08 17:08 PST by
Modified: 2010-01-17 09:55 PST (History)


Attachments
Example (4.35 KB, application/octet-stream)
2008-09-08 17:09 PST, Dan S
no flags Details
Assertion Failures (7.14 KB, text/plain)
2008-09-08 17:13 PST, Dan S
no flags Details
Screenshot (232.78 KB, image/png)
2008-09-08 17:15 PST, Dan S
no flags Details
Patch for this issue (113.86 KB, patch)
2009-12-20 05:32 PST, Martin Robinson
no flags Review Patch | Details | Formatted Diff | Diff
Fix for this issue with style updates (114.35 KB, patch)
2009-12-20 05:57 PST, Martin Robinson
no flags Review Patch | Details | Formatted Diff | Diff
Patch for this issue (rev2) (115.36 KB, patch)
2009-12-20 15:08 PST, Martin Robinson
gns: review-
gns: commit‑queue-
Review Patch | Details | Formatted Diff | Diff
Patch for this issue with style fixes (115.39 KB, patch)
2009-12-30 09:13 PST, Martin Robinson
no flags Review Patch | Details | Formatted Diff | Diff
Updated patch (112.55 KB, patch)
2010-01-07 19:21 PST, Martin Robinson
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2008-09-08 17:08:27 PST
On systems with compiz running trying to make a program that utilises partial transparency in the same window as webkit causes assertion failures.

Specifically in a GtkWindow screen changed handler, if one calls something like the following.

GdkScreen   * screen = gtk_widget_get_screen(widget);
GdkColormap * colormap = gdk_screen_get_rgba_colormap(screen);
if (!colormap) colormap = gdk_screen_get_rgb_colormap(screen);
gtk_widget_set_colormap(widget, colormap);

You would trigger the problem. If the user doesn't have compiz or simulates that by providing a rgb colourmap instead of an rgba colourmap the issue does not exist.

I have modified the example gtk program to show this.

http://www.slello.com/tmp/gtkwebkit-rgba.zip

Thanks,
Dan
------- Comment #1 From 2008-09-08 17:09:13 PST -------
Created an attachment (id=23282) [details]
Example
------- Comment #2 From 2008-09-08 17:13:16 PST -------
Created an attachment (id=23283) [details]
Assertion Failures
------- Comment #3 From 2008-09-08 17:15:27 PST -------
Created an attachment (id=23284) [details]
Screenshot
------- Comment #4 From 2009-12-20 05:32:02 PST -------
Created an attachment (id=45271) [details]
Patch for this issue

I've attached a patch for this issue.
------- Comment #5 From 2009-12-20 05:37:16 PST -------
Attachment 45271 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
Last 5120 characters of output:
 tooltipWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:91:  Declaration has space between * and variable name in GtkWidget* menuBarWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:92:  Declaration has space between * and variable name in GtkWidget* menuBarItemWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:93:  Declaration has space between * and variable name in GtkWidget* menuPopupWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:94:  Declaration has space between * and variable name in GtkWidget* menuItemWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:95:  Declaration has space between * and variable name in GtkWidget* imageMenuItemWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:96:  Declaration has space between * and variable name in GtkWidget* checkMenuItemWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:97:  Declaration has space between * and variable name in GtkWidget* treeViewWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:98:  Declaration has space between * and variable name in GtkTreeViewColumn* middleTreeViewColumn  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:99:  Declaration has space between * and variable name in GtkWidget* treeHeaderCellWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:100:  Declaration has space between * and variable name in GtkWidget* treeHeaderSortArrowWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:101:  Declaration has space between * and variable name in GtkWidget* expanderWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:102:  Declaration has space between * and variable name in GtkWidget* toolbarSeparatorWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:103:  Declaration has space between * and variable name in GtkWidget* menuSeparatorWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:104:  Declaration has space between * and variable name in GtkWidget* hpanedWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:105:  Declaration has space between * and variable name in GtkWidget* vpanedWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:106:  Declaration has space between * and variable name in GtkWidget* scrolledWindowWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:118:  Declaration has space between * and variable name in GdkColormap* colormap  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:281:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/platform/gtk/gtk2drawing.c:291:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/platform/gtk/gtk2drawing.c:295:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/platform/gtk/gtk2drawing.c:460:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
WebCore/platform/gtk/gtk2drawing.c:461:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
WebCore/platform/gtk/gtk2drawing.c:725:  Missing space before ( in if(  [whitespace/parens] [5]
WebCore/platform/gtk/gtk2drawing.c:942:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/gtk/gtk2drawing.c:955:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/gtk/gtk2drawing.c:1423:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/platform/gtk/gtk2drawing.c:1464:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/platform/gtk/gtk2drawing.c:1803:  separator_width is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/gtk/gtk2drawing.c:2601:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/gtk/gtk2drawing.c:3046:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/platform/gtk/gtk2drawing.c:3061:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/gtk/RenderThemeGtk.cpp:171:  An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
WebCore/platform/gtk/RenderThemeGtk.cpp:376:  Missing space after ,  [whitespace/comma] [3]
WebCore/platform/gtk/RenderThemeGtk.cpp:397:  Missing space after ,  [whitespace/comma] [3]
WebCore/platform/gtk/RenderThemeGtk.cpp:456:  Missing space after ,  [whitespace/comma] [3]
WebCore/platform/gtk/RenderThemeGtk.h:34:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 65
------- Comment #6 From 2009-12-20 05:57:50 PST -------
Created an attachment (id=45272) [details]
Fix for this issue with style updates

I've updated the patch fixing the style issues in RenderThemeGtk.*, but not gtk2drawing.c because it originates from outside the project.
------- Comment #7 From 2009-12-20 06:02:50 PST -------
Attachment 45272 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
Last 5120 characters of output:
pace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:87:  Declaration has space between * and variable name in GtkWidget* statusbarWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:88:  Declaration has space between * and variable name in GtkWidget* progresWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:89:  Declaration has space between * and variable name in GtkWidget* tabWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:90:  Declaration has space between * and variable name in GtkWidget* tooltipWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:91:  Declaration has space between * and variable name in GtkWidget* menuBarWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:92:  Declaration has space between * and variable name in GtkWidget* menuBarItemWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:93:  Declaration has space between * and variable name in GtkWidget* menuPopupWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:94:  Declaration has space between * and variable name in GtkWidget* menuItemWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:95:  Declaration has space between * and variable name in GtkWidget* imageMenuItemWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:96:  Declaration has space between * and variable name in GtkWidget* checkMenuItemWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:97:  Declaration has space between * and variable name in GtkWidget* treeViewWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:98:  Declaration has space between * and variable name in GtkTreeViewColumn* middleTreeViewColumn  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:99:  Declaration has space between * and variable name in GtkWidget* treeHeaderCellWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:100:  Declaration has space between * and variable name in GtkWidget* treeHeaderSortArrowWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:101:  Declaration has space between * and variable name in GtkWidget* expanderWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:102:  Declaration has space between * and variable name in GtkWidget* toolbarSeparatorWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:103:  Declaration has space between * and variable name in GtkWidget* menuSeparatorWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:104:  Declaration has space between * and variable name in GtkWidget* hpanedWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:105:  Declaration has space between * and variable name in GtkWidget* vpanedWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:106:  Declaration has space between * and variable name in GtkWidget* scrolledWindowWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:118:  Declaration has space between * and variable name in GdkColormap* colormap  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:281:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/platform/gtk/gtk2drawing.c:291:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/platform/gtk/gtk2drawing.c:295:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/platform/gtk/gtk2drawing.c:460:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
WebCore/platform/gtk/gtk2drawing.c:461:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
WebCore/platform/gtk/gtk2drawing.c:725:  Missing space before ( in if(  [whitespace/parens] [5]
WebCore/platform/gtk/gtk2drawing.c:942:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/gtk/gtk2drawing.c:955:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/gtk/gtk2drawing.c:1423:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/platform/gtk/gtk2drawing.c:1464:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/platform/gtk/gtk2drawing.c:1803:  separator_width is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/gtk/gtk2drawing.c:2601:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/gtk/gtk2drawing.c:3046:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/platform/gtk/gtk2drawing.c:3061:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 60
------- Comment #8 From 2009-12-20 15:08:52 PST -------
Created an attachment (id=45297) [details]
Patch for this issue (rev2)

After talking with Xan, kov, and Company, I've made some improvement to my original patch. In particular, memory allocated per colormap is free at exit now.
------- Comment #9 From 2009-12-20 15:10:11 PST -------
Attachment 45297 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
Last 5120 characters of output:
ame in GtkWidget* progresWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:88:  Declaration has space between * and variable name in GtkWidget* tabWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:89:  Declaration has space between * and variable name in GtkWidget* tooltipWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:90:  Declaration has space between * and variable name in GtkWidget* menuBarWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:91:  Declaration has space between * and variable name in GtkWidget* menuBarItemWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:92:  Declaration has space between * and variable name in GtkWidget* menuPopupWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:93:  Declaration has space between * and variable name in GtkWidget* menuItemWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:94:  Declaration has space between * and variable name in GtkWidget* imageMenuItemWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:95:  Declaration has space between * and variable name in GtkWidget* checkMenuItemWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:96:  Declaration has space between * and variable name in GtkWidget* treeViewWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:97:  Declaration has space between * and variable name in GtkTreeViewColumn* middleTreeViewColumn  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:98:  Declaration has space between * and variable name in GtkWidget* treeHeaderCellWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:99:  Declaration has space between * and variable name in GtkWidget* treeHeaderSortArrowWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:100:  Declaration has space between * and variable name in GtkWidget* expanderWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:101:  Declaration has space between * and variable name in GtkWidget* toolbarSeparatorWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:102:  Declaration has space between * and variable name in GtkWidget* menuSeparatorWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:103:  Declaration has space between * and variable name in GtkWidget* hpanedWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:104:  Declaration has space between * and variable name in GtkWidget* vpanedWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:105:  Declaration has space between * and variable name in GtkWidget* scrolledWindowWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:118:  Declaration has space between * and variable name in GdkColormap* colormap  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:126:  One line control clauses should not use braces.  [whitespace/braces] [4]
WebCore/platform/gtk/gtk2drawing.c:277:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/platform/gtk/gtk2drawing.c:287:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/platform/gtk/gtk2drawing.c:291:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/platform/gtk/gtk2drawing.c:456:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
WebCore/platform/gtk/gtk2drawing.c:457:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
WebCore/platform/gtk/gtk2drawing.c:721:  Missing space before ( in if(  [whitespace/parens] [5]
WebCore/platform/gtk/gtk2drawing.c:939:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/gtk/gtk2drawing.c:952:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/gtk/gtk2drawing.c:1420:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/platform/gtk/gtk2drawing.c:1461:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/platform/gtk/gtk2drawing.c:1800:  separator_width is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/gtk/gtk2drawing.c:2598:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/gtk/gtk2drawing.c:3043:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/platform/gtk/gtk2drawing.c:3058:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/gtk/gtk2drawing.c:3307:  Declaration has space between * and variable name in GtkThemeParts* part  [whitespace/declaration] [3]
Total errors found: 64
------- Comment #10 From 2009-12-20 17:28:25 PST -------
Are these style errors real or false positives?  If they're real, we should fix them.  If they're false positives, we need to teach the stylebot not to go nuts.
------- Comment #11 From 2009-12-21 07:52:53 PST -------
> Are these style errors real or false positives?  If they're real, we should fix
> them.  If they're false positives, we need to teach the stylebot not to go
> nuts.

Hey Adam, gtk2drawing.c and gtkdrawing.h are originally from Mozilla. I didn't touch the style of these files because of the guidelines at: http://trac.webkit.org/wiki/HackingGtk about them. Let me know if you'd prefer that I do something different.
------- Comment #12 From 2009-12-21 11:37:09 PST -------
There's a thread on webkit-dev about this topic at the moment.  It might be worth contributing your thoughts.  We should either fix the style violations in those files or teach the style checker not to complain.  I'm not sure which is the better approach, but having the style checker spew false alarms isn't useful.
------- Comment #13 From 2009-12-29 10:17:47 PST -------
(From update of attachment 45297 [details])
Wicked!

22         No new tests. (OOPS!)

We should explain here why this isn't testable right now.

5353 #include <string.h>
 54 #include <stdlib.h>

Poorly sorted

 108 static GHashTable *gPartsTable = NULL;
 109 static GtkThemeParts *gParts = NULL;

Bad placement for *'s

 227 gint
 228 moz_gtk_use_parts_for_drawable(GdkDrawable* parts);

gint should be in the same line, in the header file.
------- Comment #14 From 2009-12-29 10:25:12 PST -------
(In reply to comment #13)

> We should explain here why this isn't testable right now.
> Poorly sorted

Thanks for the review! I should have a fix for these first two issues soon.

>  108 static GHashTable *gPartsTable = NULL;
>  109 static GtkThemeParts *gParts = NULL;
> 
> Bad placement for *'s
> 
>  227 gint
>  228 moz_gtk_use_parts_for_drawable(GdkDrawable* parts);
> 
> gint should be in the same line, in the header file.

Since these files originate from Mozilla, I think using the original style in this case will make it easier for the patch to move back upstream if there is interest. I'm not sure if any resolution came from the discussion of this issue on the mailing list.
------- Comment #15 From 2009-12-30 09:13:56 PST -------
Created an attachment (id=45666) [details]
Patch for this issue with style fixes

I've fixed the style and changelog issues from the previous review.
------- Comment #16 From 2009-12-30 09:15:45 PST -------
Attachment 45666 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
Last 3072 characters of output:
atform/gtk/gtk2drawing.c:101:  Declaration has space between * and variable name in GtkWidget* toolbarSeparatorWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:102:  Declaration has space between * and variable name in GtkWidget* menuSeparatorWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:103:  Declaration has space between * and variable name in GtkWidget* hpanedWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:104:  Declaration has space between * and variable name in GtkWidget* vpanedWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:105:  Declaration has space between * and variable name in GtkWidget* scrolledWindowWidget  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:118:  Declaration has space between * and variable name in GdkColormap* colormap  [whitespace/declaration] [3]
WebCore/platform/gtk/gtk2drawing.c:126:  One line control clauses should not use braces.  [whitespace/braces] [4]
WebCore/platform/gtk/gtk2drawing.c:277:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/platform/gtk/gtk2drawing.c:287:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/platform/gtk/gtk2drawing.c:291:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/platform/gtk/gtk2drawing.c:456:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
WebCore/platform/gtk/gtk2drawing.c:457:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
WebCore/platform/gtk/gtk2drawing.c:721:  Missing space before ( in if(  [whitespace/parens] [5]
WebCore/platform/gtk/gtk2drawing.c:939:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/gtk/gtk2drawing.c:952:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/gtk/gtk2drawing.c:1420:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/platform/gtk/gtk2drawing.c:1461:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/platform/gtk/gtk2drawing.c:1800:  separator_width is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/gtk/gtk2drawing.c:2598:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/gtk/gtk2drawing.c:3043:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/platform/gtk/gtk2drawing.c:3058:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/gtk/gtk2drawing.c:3307:  Declaration has space between * and variable name in GtkThemeParts* part  [whitespace/declaration] [3]
Total errors found: 63
------- Comment #17 From 2010-01-06 10:35:44 PST -------
2 things from looking at the patch:
1) GtkThemeParts are allocated with g_slice_new0() but cleared with g_free() (in the hashtable).
2) I don't like using the hash table. Couldn't we have an API like:
/* call once per page and store in RenderThemeGtk */
GtkThemeParts *gtk_theme_parts_new (GdkDrawable *);
/* call instead of moz_gtk_use_parts_for_drawable */
void gtk_theme_parts_use (GtkThemeParts *);
/* obvious */
gtk_theme_parts_free (GtkThemeParts *);
That gets rid of stupid globals.
It does create new widgets for every RenderTheme. I don't think it matters though.
------- Comment #18 From 2010-01-06 11:20:34 PST -------
I think you should not use Gtk or gtk as structure and function prefixes respectively. I have experienced confusion with people trying to debug functions named with a foreign API prefix and it's best to avoid that.
------- Comment #19 From 2010-01-07 19:21:23 PST -------
Created an attachment (id=46109) [details]
Updated patch
------- Comment #20 From 2010-01-07 19:23:02 PST -------
Attachment 46109 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/platform/gtk/gtkdrawing.h:304:  moz_gtk_destroy_theme_parts_widgets is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/gtk/gtk2drawing.c:208:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/platform/gtk/gtk2drawing.c:218:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/platform/gtk/gtk2drawing.c:222:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/platform/gtk/gtk2drawing.c:387:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
WebCore/platform/gtk/gtk2drawing.c:388:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
WebCore/platform/gtk/gtk2drawing.c:652:  Missing space before ( in if(  [whitespace/parens] [5]
WebCore/platform/gtk/gtk2drawing.c:866:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/gtk/gtk2drawing.c:879:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/gtk/gtk2drawing.c:1347:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/platform/gtk/gtk2drawing.c:1388:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/platform/gtk/gtk2drawing.c:1727:  separator_width is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/gtk/gtk2drawing.c:2525:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/gtk/gtk2drawing.c:2970:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/platform/gtk/gtk2drawing.c:2985:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/gtk/gtk2drawing.c:3235:  moz_gtk_destroy_theme_parts_widgets is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 16
------- Comment #21 From 2010-01-17 09:22:39 PST -------
(In reply to comment #20)
> Attachment 46109 [details] [details] did not pass style-queue:
> 
> Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1

Reported these as https://bugs.webkit.org/show_bug.cgi?id=33771.
------- Comment #22 From 2010-01-17 09:26:28 PST -------
(From update of attachment 46109 [details])
Looks good to me.
------- Comment #23 From 2010-01-17 09:54:57 PST -------
(From update of attachment 46109 [details])
Clearing flags on attachment: 46109

Committed r53372: <http://trac.webkit.org/changeset/53372>
------- Comment #24 From 2010-01-17 09:55:06 PST -------
All reviewed patches have been landed.  Closing bug.