Bug 80030

Summary: [GObject bindings] Supplemental interfaces are not disabled with the "Conditional" attribute
Product: WebKit Reporter: Zan Dobersek <zan>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, haraken, japhet, mrobinson, pf, svillar, webkit.review.bot, zan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 81451, 82080    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch mrobinson: review+, mrobinson: commit-queue-

Description Zan Dobersek 2012-03-01 08:42:44 PST 
This is a spin-off from bug #79375 that focuses on the titular problem in the GObject bindings only.
Comment 1 Zan Dobersek 2012-03-01 09:01:43 PST 
(In reply to comment #41)
> (In reply to comment #34)
> > (In reply to comment #33)
> > 
> > > If including config.h somewhere along the way is not feasible or practical, maybe we can do an exception for GObject and just check #if defined(ENABLE_XXXX) && ENABLE_XXXX directly or define just the ENABLE macro somewhere appropriate?
> > 
> > One important consideration, which I hope does not complicate the matter terribly, is that the API should change as little as possible with different configuration flags. For instance, some distributions may compile WebKit with --disable-geolcation. Ideally, the GObject binding API should be the same in either case, even if the geolocation methods are stubs when disabled. The reason for this is that downstream applications need to be able to rely on certain API points being present to use them effectively.
> 
> Thing is, as I said in comment #8, that those API might have references to objects that are not generated. That's the case of geolocation for example. Maybe we could create empty objects but I'm sure that people will start to complain about non working stuff.

This is a valid concern. Generating bindings for a feature that's not supported in Gtk port, regardless of that feature being enabled, doesn't really make sense and can backfire in the shape of confused users. Generating bindings unconditionally for features that are fully supported and are enabled by default in the Gtk port is not as radical, but again, it is possible that some distributions may ship the library with geolocation (or any other feature) support disabled, causing problems.

An alternative, as already mentioned, is to create new API for feature detection, an equivalent to the ENABLE macro, and use that.
Comment 2 Zan Dobersek 2012-03-22 02:39:54 PDT 
I've tried to fix this in two different ways mentioned, here's what came out of each:

1) Creating stubs when the feature is not enabled

This seemed to work well, making changes mostly to the CodeGeneratorGObject script. Additionally, when a future is not enabled, warnings could be thrown, telling the feature is not enabled.

For compilation without geolocation support enabled, with this approach, it would also probably make sense to convert methods in WebKitGeolocationPolicyDecision to stubs when necessary.

Using this approach might work better if there's a decision to push this into the 1.8 branch. There's no new API additions.

2) Create and use ENABLE-like macro

This would require something like a WEBKIT_HAS_FEATURE macro to be added. This macro would then be used in public headers, while implementation files could use ENABLE macro.

One problem that immediately popped out was that when geolocation is disabled, the documentation could not be properly created - if the feature is disabled, the  gtkdoc tools complain of undefined references to this feature's methods. I don't know if this is a showstopper, but it would definitely cause inconvenience to distributions that would like to ship the library with the documented feature disabled.


I'd like some consensus on what's the right approach so I can start working on a fix.
Comment 3 Martin Robinson 2012-03-22 07:42:10 PDT 
(In reply to comment #2)

> 1) Creating stubs when the feature is not enabled
> 
> This seemed to work well, making changes mostly to the CodeGeneratorGObject script. Additionally, when a future is not enabled, warnings could be thrown, telling the feature is not enabled.
> 
> For compilation without geolocation support enabled, with this approach, it would also probably make sense to convert methods in WebKitGeolocationPolicyDecision to stubs when necessary.
> 
> Using this approach might work better if there's a decision to push this into the 1.8 branch. There's no new API additions.

Using stubs sounds like a great approach. We definitely want this for 1.8.\
Comment 4 Zan Dobersek 2012-03-23 07:27:35 PDT 
Created attachment 133481 [details]
Patch
Comment 5 Zan Dobersek 2012-03-23 07:33:13 PDT 
(In reply to comment #4)
> Created an attachment (id=133481) [details]
> Patch

Does some refactoring in the CodeGeneratorGObject.pm and creates proper stubs in case feature is not enabled at compile-time. Also fixes the build when geolocation support is disabled.

Forgot to test this with a WebKit2 build, will update the patch later if there will be need.

There are style issues with the generated test bindings, which were probably showing even before this patch.
Comment 6 Zan Dobersek 2012-03-23 07:45:04 PDT 
Created attachment 133485 [details]
Patch

Properly applies to the tree
Comment 7 WebKit Review Bot 2012-03-23 07:49:08 PDT 
Attachment 133485 [details] did not pass style-queue:

Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:77:  Extra space between return and WEBKIT_DOM_TEST_SERIALIZED_SCRIPT_VALUE_INTERFACE  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:222:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:227:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:250:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:255:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:260:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:265:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:283:  Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html).  [readability/pass_ptr] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:302:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:320:  Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html).  [readability/pass_ptr] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:337:  Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html).  [readability/pass_ptr] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:356:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:374:  Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html).  [readability/pass_ptr] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:98:  Extra space between return and WEBKIT_DOM_TEST_OBJ  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:184:  Non-label code inside switch statements should be indented.  [whitespace/indent] [4]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:327:  Non-label code inside switch statements should be indented.  [whitespace/indent] [4]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:581:  Declaration has space between type name and * in GObjectClass *gobjectClass  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMFloat64Array.cpp:75:  Extra space between return and WEBKIT_DOM_FLOAT64ARRAY  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMFloat64Array.cpp:158:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMFloat64Array.cpp:162:  LoFailed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
cal variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html).  [readability/pass_ptr] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestCallback.cpp:80:  Extra space between return and WEBKIT_DOM_TEST_CALLBACK  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestCallback.cpp:143:  Declaration has space between type name and * in GObjectClass *gobjectClass  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestCustomNamedGetter.cpp:70:  Extra space between return and WEBKIT_DOM_TEST_CUSTOM_NAMED_GETTER  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestEventConstructor.cpp:70:  Extra space between return and WEBKIT_DOM_TEST_EVENT_CONSTRUCTOR  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestEventTarget.cpp:78:  Extra space between return and WEBKIT_DOM_TEST_EVENT_TARGET  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestEventTarget.cpp:191:  Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html).  [readability/pass_ptr] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestEventTarget.cpp:204:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestInterface.cpp:79:  Extra space between return and WEBKIT_DOM_TEST_INTERFACE  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestInterface.cpp:256:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestInterface.cpp:261:  Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html).  [readability/pass_ptr] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestInterface.cpp:372:  Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html).  [readability/pass_ptr] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestInterface.cpp:397:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestNamedConstructor.cpp:70:  Extra space between return and WEBKIT_DOM_TEST_NAMED_CONSTRUCTOR  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestActiveDOMObject.cpp:72:  Extra space between return and WEBKIT_DOM_TEST_ACTIVE_DOM_OBJECT  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestActiveDOMObject.cpp:172:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestMediaQueryListListener.cpp:70:  Extra space between return and WEBKIT_DOM_TEST_MEDIA_QUERY_LIST_LISTENER  [whitespace/declaration] [3]
Total errors found: 36 in 30 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 Adam Barth 2012-03-23 09:20:31 PDT 
Comment on attachment 133485 [details]
Patch

It's slightly hard for me to follow all of the changes to the code generator, but your approach seems reasonable and the generated files do look better.  Thanks for the patch!
Comment 9 Xan Lopez 2012-03-23 10:23:50 PDT 
Comment on attachment 133485 [details]
Patch

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

This looks generally OK, but all the changes in the geolocation policy decision code seem pretty much unrelated? I think they should go in a different bug & patch, this is complicated enough to review as it is :) There also seems a couple of simple to fix style issues in the generated code that trigger warnings.

> Source/WebCore/ChangeLog:18
> +        but the root conditional feature is.

Yeah, this definitely makes sense.

> Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm:1081
> +                push(@cBody, "    return NULL;\n");

NULL here?

> Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm:1094
>                  push(@cBody, "    return NULL;\n");

And also here I suppose.

> Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm:1231
>                                                 "core-object", coreObject, NULL));

We might as well fix this double space while we are at it.
Comment 10 Zan Dobersek 2012-03-23 11:26:22 PDT 
Comment on attachment 133485 [details]
Patch

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

Very well, I'll put build fixes in WebKit part in bug #81451.

>> Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm:1081
>> +                push(@cBody, "    return NULL;\n");
> 
> NULL here?

You probably meant 0 ... ?

>> Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm:1094
>>                  push(@cBody, "    return NULL;\n");
> 
> And also here I suppose.

Ditto?

>> Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm:1231
>>                                                 "core-object", coreObject, NULL));
> 
> We might as well fix this double space while we are at it.

I'll also just put this into one line to avoid incorrect indentation.
Comment 11 Martin Robinson 2012-03-23 11:32:34 PDT 
(In reply to comment #10)
> (From update of attachment 133485 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=133485&action=review
> 
> Very well, I'll put build fixes in WebKit part in bug #81451.

I'm working on a patch that fixes all the style errors in the DOM bindings now. So, if you like, I can handle this.
Comment 12 Zan Dobersek 2012-03-23 11:51:01 PDT 
(In reply to comment #11)
> (In reply to comment #10)
> > (From update of attachment 133485 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=133485&action=review
> > 
> > Very well, I'll put build fixes in WebKit part in bug #81451.
> 
> I'm working on a patch that fixes all the style errors in the DOM bindings now. So, if you like, I can handle this.

Okay, that'd be great.

I'll be uploading a new patch with only changes to CodeGeneratorGObject.pm and rebaselines for the bindings tests.
Comment 13 Zan Dobersek 2012-03-23 11:52:52 PDT 
Created attachment 133527 [details]
Patch
Comment 14 WebKit Review Bot 2012-03-23 11:58:07 PDT 
Attachment 133527 [details] did not pass style-queue:

Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:77:  Extra space between return and WEBKIT_DOM_TEST_SERIALIZED_SCRIPT_VALUE_INTERFACE  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:222:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:227:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:250:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:255:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:260:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:265:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:283:  Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html).  [readability/pass_ptr] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:302:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:320:  Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html).  [readability/pass_ptr] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:337:  Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html).  [readability/pass_ptr] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:356:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:374:  Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html).  [readability/pass_ptr] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:98:  Extra space between return and WEBKIT_DOM_TEST_OBJ  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:185:  Non-label code inside switch statements should be indented.  [whitespace/indent] [4]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:328:  Non-label code inside switch statements should be indented.  [whitespace/indent] [4]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:587:  Declaration has space between type name and * in GObjectClass *gobjectClass  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMFloat64Array.cpp:75:  Extra space between return and WEBKIT_DOM_FLOAT64ARRAY  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMFloat64Array.cpp:158:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMFloat64Array.cpp:162:  LoFailed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
cal variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html).  [readability/pass_ptr] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestCallback.cpp:80:  Extra space between return and WEBKIT_DOM_TEST_CALLBACK  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestCallback.cpp:143:  Declaration has space between type name and * in GObjectClass *gobjectClass  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestCustomNamedGetter.cpp:70:  Extra space between return and WEBKIT_DOM_TEST_CUSTOM_NAMED_GETTER  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestEventConstructor.cpp:70:  Extra space between return and WEBKIT_DOM_TEST_EVENT_CONSTRUCTOR  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestEventTarget.cpp:78:  Extra space between return and WEBKIT_DOM_TEST_EVENT_TARGET  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestEventTarget.cpp:191:  Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html).  [readability/pass_ptr] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestEventTarget.cpp:204:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestInterface.cpp:79:  Extra space between return and WEBKIT_DOM_TEST_INTERFACE  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestInterface.cpp:256:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestInterface.cpp:261:  Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html).  [readability/pass_ptr] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestInterface.cpp:372:  Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html).  [readability/pass_ptr] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestInterface.cpp:397:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestNamedConstructor.cpp:70:  Extra space between return and WEBKIT_DOM_TEST_NAMED_CONSTRUCTOR  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestActiveDOMObject.cpp:72:  Extra space between return and WEBKIT_DOM_TEST_ACTIVE_DOM_OBJECT  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestActiveDOMObject.cpp:172:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestMediaQueryListListener.cpp:70:  Extra space between return and WEBKIT_DOM_TEST_MEDIA_QUERY_LIST_LISTENER  [whitespace/declaration] [3]
Total errors found: 36 in 25 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 15 Zan Dobersek 2012-03-23 12:00:39 PDT 
Comment on attachment 133527 [details]
Patch

Urgh, still requires the warn define. Uploading new patch in a moment.
Comment 16 Zan Dobersek 2012-03-23 12:39:48 PDT 
Created attachment 133536 [details]
Patch
Comment 17 WebKit Review Bot 2012-03-23 12:42:10 PDT 
Attachment 133536 [details] did not pass style-queue:

Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:77:  Extra space between return and WEBKIT_DOM_TEST_SERIALIZED_SCRIPT_VALUE_INTERFACE  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:222:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:227:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:250:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:255:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:260:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:265:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:283:  Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html).  [readability/pass_ptr] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:302:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:320:  Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html).  [readability/pass_ptr] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:337:  Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html).  [readability/pass_ptr] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:356:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:374:  Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html).  [readability/pass_ptr] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:98:  Extra space between return and WEBKIT_DOM_TEST_OBJ  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:185:  Non-label code inside switch statements should be indented.  [whitespace/indent] [4]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:328:  Non-label code inside switch statements should be indented.  [whitespace/indent] [4]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:587:  Declaration has space between type name and * in GObjectClass *gobjectClass  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMFloat64Array.cpp:75:  Extra space between return and WEBKIT_DOM_FLOAT64ARRAY  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMFloat64Array.cpp:158:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMFloat64Array.cpp:162:  LoFailed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
cal variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html).  [readability/pass_ptr] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestCallback.cpp:80:  Extra space between return and WEBKIT_DOM_TEST_CALLBACK  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestCallback.cpp:143:  Declaration has space between type name and * in GObjectClass *gobjectClass  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestCustomNamedGetter.cpp:70:  Extra space between return and WEBKIT_DOM_TEST_CUSTOM_NAMED_GETTER  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestEventConstructor.cpp:70:  Extra space between return and WEBKIT_DOM_TEST_EVENT_CONSTRUCTOR  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestEventTarget.cpp:78:  Extra space between return and WEBKIT_DOM_TEST_EVENT_TARGET  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestEventTarget.cpp:191:  Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html).  [readability/pass_ptr] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestEventTarget.cpp:204:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestInterface.cpp:79:  Extra space between return and WEBKIT_DOM_TEST_INTERFACE  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestInterface.cpp:256:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestInterface.cpp:261:  Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html).  [readability/pass_ptr] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestInterface.cpp:372:  Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html).  [readability/pass_ptr] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestInterface.cpp:397:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestNamedConstructor.cpp:70:  Extra space between return and WEBKIT_DOM_TEST_NAMED_CONSTRUCTOR  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestActiveDOMObject.cpp:72:  Extra space between return and WEBKIT_DOM_TEST_ACTIVE_DOM_OBJECT  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestActiveDOMObject.cpp:172:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestMediaQueryListListener.cpp:70:  Extra space between return and WEBKIT_DOM_TEST_MEDIA_QUERY_LIST_LISTENER  [whitespace/declaration] [3]
Total errors found: 36 in 27 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 18 Zan Dobersek 2012-03-23 12:43:53 PDT 
(In reply to comment #16)
> Created an attachment (id=133536) [details]
> Patch

This should be complete now. Style issues are covered by bug #82080.
Comment 19 Martin Robinson 2012-03-23 14:46:11 PDT 
Comment on attachment 133536 [details]
Patch

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

I'll land this one.

> Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm:117
> +sub HumanReadableConditional {

HumanReadableConditional -> humanReadbleConditional?

> Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm:362
> +sub GenerateConditionalWarn

GenerateConditionalWarn -> generateConditionalWarning

> Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm:368
> +    if ($conditional) {

This probably deserves an early return.
Comment 20 Martin Robinson 2012-03-23 15:10:44 PDT 
Committed r111914: <http://trac.webkit.org/changeset/111914>