Bug 74972 – Enable the [Supplemental] IDL on Gtk

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Kentaro Hara]

We have enabled the [Supplemental] IDL on Chromium and AppleWebKit, and are planning to enable it on all platforms (Meta bug 72138). In this bug, we enable it on Gtk.

[Kentaro Hara]

Created attachment 120112 [details]
WIP patch to see if build passes

[Martin Robinson]

Comment on attachment 120112 [details]
WIP patch to see if build passes

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

> GNUmakefile.am:54
> +BINDING_IDLS :=

Would prefer something a bit more descriptive. Would dom_binding_idls be more accurate?

[Kentaro Hara]

(In reply to comment #2)
> (From update of attachment 120112 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=120112&action=review
> 
> > GNUmakefile.am:54
> > +BINDING_IDLS :=
> 
> Would prefer something a bit more descriptive. Would dom_binding_idls be more accurate?

Sure, I'll fix it. Since this patch is too big, I'll land it step by step after I confirm that it passes all builds.

[Collabora GTK+ EWS bot]

Comment on attachment 120112 [details]
WIP patch to see if build passes

Attachment 120112 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10991600

[Kentaro Hara]

Created attachment 120160 [details]
Patch

[Collabora GTK+ EWS bot]

Comment on attachment 120160 [details]
Patch

Attachment 120160 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10999061

[Kentaro Hara]

Created attachment 120249 [details]
WIP patch to see if build succeeds

[WebKit Review Bot]

Attachment 120249 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9

Updating OpenSource
From git://git.webkit.org/WebKit
   e4a13f1..3779062  master     -> origin/master
Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ...
Currently at 103476 = e4a13f19647c218b1fc1ce0ecc69c5ed16a4d7f4
r103477 = 3779062662ed4d6798ce8e6d2adb880cc2b2704a
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Applying: Inform the scrolling coordinator when scrollbar layers come and go
Using index info to reconstruct a base tree...
<stdin>:474806: trailing whitespace.
        [Chromium] DatabaseTrackerChromium: iterating DatabaseSet races with Database disposal on worker thread 
<stdin>:474827: trailing whitespace.
        Nothing to test, just removing redundant code. Correct behavior tested by 
<stdin>:475346: trailing whitespace.
    
warning: 3 lines add whitespace errors.
Falling back to patching base and 3-way merge...
warning: too many files (created: 167534 deleted: 3), skipping inexact rename detection
Auto-merging LayoutTests/ChangeLog
CONFLICT (content): Merge conflict in LayoutTests/ChangeLog
Auto-merging LayoutTests/platform/chromium/test_expectations.txt
CONFLICT (content): Merge conflict in LayoutTests/platform/chromium/test_expectations.txt
Auto-merging Source/WebCore/ChangeLog
CONFLICT (content): Merge conflict in Source/WebCore/ChangeLog
Auto-merging Source/WebCore/page/ScrollingCoordinator.h
CONFLICT (content): Merge conflict in Source/WebCore/page/ScrollingCoordinator.h
Auto-merging Source/WebCore/page/mac/ScrollingCoordinatorMac.mm
CONFLICT (content): Merge conflict in Source/WebCore/page/mac/ScrollingCoordinatorMac.mm
Auto-merging Source/WebCore/platform/ScrollView.cpp
Auto-merging Source/WebCore/rendering/RenderLayerCompositor.cpp
CONFLICT (content): Merge conflict in Source/WebCore/rendering/RenderLayerCompositor.cpp
Auto-merging Source/WebKit2/ChangeLog
CONFLICT (content): Merge conflict in Source/WebKit2/ChangeLog
Auto-merging Tools/ChangeLog
CONFLICT (content): Merge conflict in Tools/ChangeLog
Auto-merging Tools/Scripts/build-webkit
Auto-merging Tools/Scripts/webkitdirs.pm
CONFLICT (content): Merge conflict in Tools/Scripts/webkitdirs.pm
Failed to merge in the changes.
Patch failed at 0001 Inform the scrolling coordinator when scrollbar layers come and go

When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To restore the original branch and stop rebasing run "git rebase --abort".

rebase refs/remotes/origin/master: command returned error: 1

Died at Tools/Scripts/update-webkit line 158.


If any of these errors are false positives, please file a bug against check-webkit-style.

[Kentaro Hara]

Created attachment 120250 [details]
WIP patch to see if build succeeds

[WebKit Review Bot]

Attachment 120250 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9

Updating OpenSource
First, rewinding head to replay your work on top of it...
Applying: Inform the scrolling coordinator when scrollbar layers come and go
Using index info to reconstruct a base tree...
<stdin>:474806: trailing whitespace.
        [Chromium] DatabaseTrackerChromium: iterating DatabaseSet races with Database disposal on worker thread 
<stdin>:474827: trailing whitespace.
        Nothing to test, just removing redundant code. Correct behavior tested by 
<stdin>:475346: trailing whitespace.
    
warning: 3 lines add whitespace errors.
Falling back to patching base and 3-way merge...
warning: too many files (created: 167534 deleted: 3), skipping inexact rename detection
Auto-merging LayoutTests/ChangeLog
CONFLICT (content): Merge conflict in LayoutTests/ChangeLog
Auto-merging LayoutTests/platform/chromium/test_expectations.txt
CONFLICT (content): Merge conflict in LayoutTests/platform/chromium/test_expectations.txt
Auto-merging Source/WebCore/ChangeLog
CONFLICT (content): Merge conflict in Source/WebCore/ChangeLog
Auto-merging Source/WebCore/page/ScrollingCoordinator.h
CONFLICT (content): Merge conflict in Source/WebCore/page/ScrollingCoordinator.h
Auto-merging Source/WebCore/page/mac/ScrollingCoordinatorMac.mm
CONFLICT (content): Merge conflict in Source/WebCore/page/mac/ScrollingCoordinatorMac.mm
Auto-merging Source/WebCore/platform/ScrollView.cpp
Auto-merging Source/WebCore/rendering/RenderLayerCompositor.cpp
CONFLICT (content): Merge conflict in Source/WebCore/rendering/RenderLayerCompositor.cpp
Auto-merging Source/WebKit2/ChangeLog
CONFLICT (content): Merge conflict in Source/WebKit2/ChangeLog
Auto-merging Tools/ChangeLog
CONFLICT (content): Merge conflict in Tools/ChangeLog
Auto-merging Tools/Scripts/build-webkit
Auto-merging Tools/Scripts/webkitdirs.pm
CONFLICT (content): Merge conflict in Tools/Scripts/webkitdirs.pm
Failed to merge in the changes.
Patch failed at 0001 Inform the scrolling coordinator when scrollbar layers come and go

When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To restore the original branch and stop rebasing run "git rebase --abort".

rebase refs/remotes/origin/master: command returned error: 1

Died at Tools/Scripts/update-webkit line 158.


If any of these errors are false positives, please file a bug against check-webkit-style.

[Gustavo Noronha (kov)]

Comment on attachment 120250 [details]
WIP patch to see if build succeeds

Attachment 120250 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10996336

[Kentaro Hara]

Created attachment 120554 [details]
WIP patch to see if build succeeds

[Adam Barth]

It builds!

[Kentaro Hara]

[Summary]

- It builds, but the last patch is a first step for the ideal approach.
- I would like to commit the last patch, and then refactor GNUmakefile.list.am for the ideal approach.

[Details]

Current GNUmakefile.list.am:

    webcore_built_sources += \
        DerivedSources/WebCore/JSA.cpp \
        DerivedSources/WebCore/JSA.h \
        DerivedSources/WebCore/JSB.cpp \
        DerivedSources/WebCore/JSB.h \
        DerivedSources/WebCore/JSC.cpp \
        DerivedSources/WebCore/JSC.h \
        ....


The last patch:

    webcore_built_sources += \
        DerivedSources/WebCore/JSA.cpp \
        DerivedSources/WebCore/JSA.h \
        DerivedSources/WebCore/JSB.cpp \
        DerivedSources/WebCore/JSB.h \
        DerivedSources/WebCore/JSC.cpp \
        DerivedSources/WebCore/JSC.h \
        ....

    dom_binding_idls += \
        WebCore/aaa/A.idl \
        WebCore/bbb/B.idl \
        WebCore/ccc/C.idl \
        ....


The ideal approach (which I have tried but failed in the previous patches): 

    dom_binding_idls += \
        WebCore/aaa/A.idl \
        WebCore/bbb/B.idl \
        WebCore/ccc/C.idl \
        ....

    webcore_built_sources += $(patsubst %, DerivedSources/WebCore/JS%.cpp, $(basename $(notdir $(dom_binding_idls))))
    webcore_built_sources += $(patsubst %, DerivedSources/WebCore/JS%.h, $(basename $(notdir $(dom_binding_idls))))


As you can see, the last patch is verbose since we need to describe *.idl, JS*.h and JS*.cpp in GNUmakefile.list.am. I have tried to build the ideal approach in my local GTK environment for one week, but the build is still failing, as described below. Anyway, I think that the last patch is a first step for the ideal approach. We can remove verbose JS*.h and JS*.cpp from GNUmakefile.list.am incrementally after committing it.


[Build failure]

Although all compiles succeed, the link fails: http://queues.webkit.org/results/10996336

[I] This succeeds (i.e. move one file to dom_binding_idls):

    webcore_built_sources += \
        DerivedSources/WebCore/JSB.cpp \
        DerivedSources/WebCore/JSB.h \
        DerivedSources/WebCore/JSC.cpp \
        DerivedSources/WebCore/JSC.h \
        ....

    dom_binding_idls += \
        WebCore/aaa/A.idl

    webcore_built_sources += $(patsubst %, DerivedSources/WebCore/JS%.cpp, $(basename $(notdir $(dom_binding_idls))))
    webcore_built_sources += $(patsubst %, DerivedSources/WebCore/JS%.h, $(basename $(notdir $(dom_binding_idls))))


[II] This also succeeds (i.e. move two files to dom_binding_idls):

    webcore_built_sources += \
        DerivedSources/WebCore/JSC.cpp \
        DerivedSources/WebCore/JSC.h \
        ....

    dom_binding_idls += \
        WebCore/aaa/A.idl \
        WebCore/bbb/B.idl

    webcore_built_sources += $(patsubst %, DerivedSources/WebCore/JS%.cpp, $(basename $(notdir $(dom_binding_idls))))
    webcore_built_sources += $(patsubst %, DerivedSources/WebCore/JS%.h, $(basename $(notdir $(dom_binding_idls))))

[III] But this fails in the link process (i.e. move all files to dom_binding_idls):

    webcore_built_sources +=

    dom_binding_idls += \
        WebCore/aaa/A.idl \
        WebCore/bbb/B.idl \
        WebCore/ccc/C.idl \
        ....

    webcore_built_sources += $(patsubst %, DerivedSources/WebCore/JS%.cpp, $(basename $(notdir $(dom_binding_idls))))
    webcore_built_sources += $(patsubst %, DerivedSources/WebCore/JS%.h, $(basename $(notdir $(dom_binding_idls))))


In [I], [II] and [III], I confirmed that the set of generated JS*.h and JS*.cpp and their contents are exactly same. Although http://queues.webkit.org/results/10996336 indicates that JS*.h and JS*.cpp are compiled but not linked (I don't know why), interestingly, this link failure happens for (not all but) a part of JS*.h and JS*.cpp deterministically. I investigated the common points of IDLs from which those failing JS*.h and JS*.cpp are generated, but could not find anything particular. I am not sure but this might be a problem of the link order.

### Another difficulty is that it takes two hours for each clobber build in my local GTK environemnt:-)

[Adam Barth]

How bizarre.  The approach in your patch is how a number of the other build systems work, so it seems like a fine intermediate state.

[WebKit Review Bot]

Comment on attachment 120554 [details]
WIP patch to see if build succeeds

Clearing flags on attachment: 120554

Committed r103729: <http://trac.webkit.org/changeset/103729>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.