Bug 117543 – [GTK] Merge decamelizations fix ups in the GObject DOM bindings code generator

• Status: RESOLVED
• Resolution: FIXED
• Component: Bindings

[Diego Pino]

The function decamelize() cannot decamelize all strings the right way, so some hard coded fix ups are applied. At this moment, there are two functions that applied two different sets of fixups. There should be a single point to apply the same fix ups, which ensures that the result of decamelizing a string is always the same.

[Diego Pino]

Created attachment 204439 [details]
Patch

[Diego Pino]

Created attachment 205009 [details]
Patch

[Diego Pino]

I forgot to remove "No new tests (OOPS!)."

[Diego Pino]

Since this patch changes several parts of the code and it's hard to follow how those changes will affect the code generated, Xan suggested me to compare all the files under "WebKitBuild/Release/DerivedSources/webkitdom/" in branch master and a branch with this patch applied.

I wrote a script to do that (http://pastebin.com/T14kWZpF). The script revealed the following files were different:

   * WebKitBuild/Release/DerivedSources/webkitdom/WebKitDOMWebKitNamedFlow.cpp
   * WebKitBuild/Release/DerivedSources/webkitdom/WebKitDOMWebKitPoint.cpp
   * WebKitBuild/Release/DerivedSources/webkitdom/WebKitDOMHTMLIFrameElement.cpp
   * WebKitBuild/Release/DerivedSources/webkitdom/WebKitDOMXPathResult.cpp

The differences are due to an impromper decamelization of the variable "$nick" in master. Take for instance the file WebKitDOMHTMLIFrameElement.cpp, you can see code like this:

g_object_class_install_property(gobjectClass,
                                    PROP_ALIGN,
                                    g_param_spec_string("align", /* name */
                                                           "htmli_frame_element_align", /* short description */
                                                           "read-write  gchar* HTMLIFrameElement.align", /* longer - could do with some extra doc stuff here */
                                                           "", /* default */
                                                           WEBKIT_PARAM_READWRITE));

The string "htmli_frame_element_align" (value of the variable $nick), is not properly decamelized, it should be "html_iframe_element_align". This patch solves that unnoticed error too \o/

Besides "htmli", there are also an improper decamelization of the strings (as value of variable $nick) : 

   * "web_kit" should be "webkit"
   * "x_path" shouldb be "xpath"

[Carlos Garcia Campos]

Comment on attachment 205009 [details]
Patch

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

> Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm:137
> +        return FixUpDecamelize($s);

So, now that FixUpDecamelize is only called here, maybe we can add the implementation here?

[Diego Pino]

Created attachment 205912 [details]
Patch

[kov's GTK+ EWS bot]

Comment on attachment 205912 [details]
Patch

Attachment 205912 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/1016363

[Diego Pino]

Comment on attachment 205912 [details]
Patch

>Subversion Revision: 152267
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index f82d3f32bbc7d9dedd0656ab18ab746363fbceaa..e30525e640b5ab899858c707d8c692be4ac90fca 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,28 @@
>[phone]  Diego Pino Garcia  <[e-mail]>
>+
>+        [GTK] Merge decamelizations fix ups in the GObject DOM bindings code generator
>+        https://bugs.webkit.org/show_bug.cgi?id=117543
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Ensure that all the code that calls to decamelization() applies the
>+        same fix ups.
>+
>+        Now all functions that need to decamelize a string should simply
>+        call to decamelize(). This function calls to FixUpDecamelize to
>+        apply some fix ups.
>+
>+        * bindings/scripts/CodeGeneratorGObject.pm:
>+        (decamelize): decamelizes a string and applies fix ups
>+        (FixUpDecamelize): applies a series of fix ups to a decamelized string
>+        (GetParentGObjType): moved fix ups to FixUpDecamelize()
>+        (GenerateProperties): simply call to decamelize
>+        (GenerateHeader): simply call to decamelize
>+        (GetGReturnMacro): simply call to decamelize
>+        (GenerateFunction): simply call to decamelize
>+        (GenerateCFile): simply call to decamelize
>+        (GenerateEventTargetIface): simply call to decamelize
>+
> 2013-07-01  Tim Horton  <[e-mail]>
> 
>         Maximum scroll position can be negative in some cases
>diff --git a/Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm b/Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm
>index a52d47cc09c1370c4f4bbe721e40d604464b1959..061b13a27373bd54519b337349184dd20d7c7147 100644
>--- a/Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm
>+++ b/Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm
>@@ -134,18 +134,19 @@ sub decamelize
>                 $t .= $p3 ? $p1 ? "${p1}_$p2$p3" : "$p2$p3" : "$p1$p2";
>                 $t;
>         }ge;
>-        $s;
>-}
>-
>-sub FixUpDecamelizedName {
>-    my $classname = shift;
> 
>-    # FIXME: try to merge this somehow with the fixes in ClassNameToGobjectType
>-    $classname =~ s/x_path/xpath/;
>-    $classname =~ s/web_kit/webkit/;
>-    $classname =~ s/htmli_frame/html_iframe/;
>-
>-    return $classname;
>+        # Some strings are not correctly decamelized, apply fix ups
>+        for ($s) {
>+            s/domcss/dom_css/;
>+            s/domhtml/dom_html/;
>+            s/domdom/dom_dom/;
>+            s/domcdata/dom_cdata/;
>+            s/domui/dom_ui/;
>+            s/x_path/xpath/;
>+            s/web_kit/webkit/;
>+            s/htmli_frame/html_iframe/;
>+        }
>+        return $s;
> }
> 
> sub HumanReadableConditional {
>@@ -163,28 +164,11 @@ sub HumanReadableConditional {
>     return join(' ', @humanReadable);
> }
> 
>-sub ClassNameToGObjectType {
>-    my $className = shift;
>-    my $CLASS_NAME = uc(decamelize($className));
>-    # Fixup: with our prefix being 'WebKitDOM' decamelize can't get
>-    # WebKitDOMCSS and similar names right, so we have to fix it
>-    # manually.
>-    $CLASS_NAME =~ s/DOMCSS/DOM_CSS/;
>-    $CLASS_NAME =~ s/DOMHTML/DOM_HTML/;
>-    $CLASS_NAME =~ s/DOMDOM/DOM_DOM/;
>-    $CLASS_NAME =~ s/DOMCDATA/DOM_CDATA/;
>-    $CLASS_NAME =~ s/DOMX_PATH/DOM_XPATH/;
>-    $CLASS_NAME =~ s/DOM_WEB_KIT/DOM_WEBKIT/;
>-    $CLASS_NAME =~ s/DOMUI/DOM_UI/;
>-    $CLASS_NAME =~ s/HTMLI_FRAME/HTML_IFRAME/;
>-    return $CLASS_NAME;
>-}
>-
> sub GetParentGObjType {
>     my $interface = shift;
> 
>     return "WEBKIT_TYPE_DOM_OBJECT" if @{$interface->parents} eq 0;
>-    return "WEBKIT_TYPE_DOM_" . ClassNameToGObjectType($interface->parents(0));
>+    return "WEBKIT_TYPE_DOM_" . uc(decamelize($interface->parents(0)));
> }
> 
> sub GetClassName {
>@@ -604,8 +588,9 @@ EOF
> sub GenerateProperties {
>     my ($object, $interfaceName, $interface) = @_;
> 
>-    my $clsCaps = substr(ClassNameToGObjectType($className), 12);
>-    my $lowerCaseIfaceName = "webkit_dom_" . (FixUpDecamelizedName(decamelize($interfaceName)));
>+    my $decamelize = decamelize($interfaceName);
>+    my $clsCaps = uc($decamelize);
>+    my $lowerCaseIfaceName = "webkit_dom_$decamelize";
>     my $parentImplClassName = GetParentImplClassName($interface);
> 
>     my $conditionGuardStart = "";
>@@ -832,9 +817,9 @@ EOF
> 
>     push(@hBodyPre, $implContent);
> 
>-    my $decamelize = FixUpDecamelizedName(decamelize($interfaceName));
>+    my $decamelize = decamelize($interfaceName);
>     my $clsCaps = uc($decamelize);
>-    my $lowerCaseIfaceName = "webkit_dom_" . ($decamelize);
>+    my $lowerCaseIfaceName = "webkit_dom_$decamelize";
> 
>     $implContent = << "EOF";
> #define WEBKIT_TYPE_DOM_${clsCaps}            (${lowerCaseIfaceName}_get_type())
>@@ -867,7 +852,7 @@ sub GetGReturnMacro {
>     if ($paramIDLType eq "GError") {
>         $condition = "!$paramName || !*$paramName";
>     } elsif (IsGDOMClassType($paramIDLType)) {
>-        my $paramTypeCaps = uc(FixUpDecamelizedName(decamelize($paramIDLType)));
>+        my $paramTypeCaps = uc(decamelize($paramIDLType));
>         $condition = "WEBKIT_DOM_IS_${paramTypeCaps}($paramName)";
>         if (ParamCanBeNull($functionName, $paramName)) {
>             $condition = "!$paramName || $condition";
>@@ -902,7 +887,7 @@ sub ParamCanBeNull {
> sub GenerateFunction {
>     my ($object, $interfaceName, $function, $prefix, $parentNode) = @_;
> 
>-    my $decamelize = FixUpDecamelizedName(decamelize($interfaceName));
>+    my $decamelize = decamelize($interfaceName);
> 
>     if ($object eq "MediaQueryListListener") {
>         return;
>@@ -1299,8 +1284,9 @@ sub GenerateCFile {
> 
>     my $implContent = "";
> 
>-    my $clsCaps = uc(FixUpDecamelizedName(decamelize($interfaceName)));
>-    my $lowerCaseIfaceName = "webkit_dom_" . FixUpDecamelizedName(decamelize($interfaceName));
>+    my $decamelize = decamelize($interfaceName);
>+    my $clsCaps = uc($decamelize);
>+    my $lowerCaseIfaceName = "webkit_dom_$decamelize";
>     my $parentImplClassName = GetParentImplClassName($interface);
>     my $baseClassName = GetBaseClass($parentImplClassName);
> 
>@@ -1390,7 +1376,7 @@ sub GenerateEventTargetIface {
>     my $interface = shift;
> 
>     my $interfaceName = $interface->name;
>-    my $decamelize = FixUpDecamelizedName(decamelize($interfaceName));
>+    my $decamelize = decamelize($interfaceName);
>     my $conditionalString = $codeGenerator->GenerateConditionalString($interface);
>     my @conditionalWarn = GenerateConditionalWarning($interface);
>

[Diego Pino]

The last patch didn't apply because I used "//" to write a comment and Perl uses "#". I tried to modified the patch directly but don't know how to do it. I'm going to send the patch again.

[Diego Pino]

Created attachment 205932 [details]
Patch

[Carlos Garcia Campos]

Comment on attachment 205932 [details]
Patch

Thanks!

[WebKit Commit Bot]

Comment on attachment 205932 [details]
Patch

Rejecting attachment 205932 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-01', 'apply-attachment', '--no-update', '--non-interactive', 205932, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
lumes/Data/EWS/WebKit

Parsed 2 diffs from patch file(s).
patching file Source/WebCore/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm
Hunk #2 FAILED at 164.
1 out of 8 hunks FAILED -- saving rejects to file Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm.rej

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Carlos Garcia Campos']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Full output: http://webkit-queues.appspot.com/results/1130633

[Diego Pino]

Created attachment 207078 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 207078 [details]
Patch

Clearing flags on attachment: 207078

Committed r152898: <http://trac.webkit.org/changeset/152898>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.