Bug 74972

Summary:

Enable the [Supplemental] IDL on Gtk

Product:

WebKit

Reporter:

Kentaro Hara <haraken>

Component:

WebCore Misc.

Assignee:

Kentaro Hara <haraken>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, dominicc, gustavo.noronha, gustavo, mrobinson, webkit.review.bot, xan.lopez

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

72138

Attachments:

Description	Flags
WIP patch to see if build passes	none
Patch	none
WIP patch to see if build succeeds	none
WIP patch to see if build succeeds	none
WIP patch to see if build succeeds	none

Description Kentaro Hara 2011-12-20 16:20:34 PST

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.

Comment 1 Kentaro Hara 2011-12-20 16:32:30 PST

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

Comment 2 Martin Robinson 2011-12-20 16:40:33 PST

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?

Comment 3 Kentaro Hara 2011-12-20 16:42:59 PST

(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.

Comment 4 Collabora GTK+ EWS bot 2011-12-21 01:30:12 PST

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

Comment 5 Kentaro Hara 2011-12-21 01:45:23 PST

Created attachment 120160 [details]
Patch

Comment 6 Collabora GTK+ EWS bot 2011-12-21 12:47:53 PST

Comment on attachment 120160 [details]
Patch

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

Comment 7 Kentaro Hara 2011-12-21 17:28:21 PST

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

Comment 8 WebKit Review Bot 2011-12-21 17:31:17 PST

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.

Comment 9 Kentaro Hara 2011-12-21 17:36:20 PST

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

Comment 10 WebKit Review Bot 2011-12-21 17:53:38 PST

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.

Comment 11 Gustavo Noronha (kov) 2011-12-21 23:06:41 PST

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

Comment 12 Kentaro Hara 2011-12-26 06:40:05 PST

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

Comment 13 Adam Barth 2011-12-26 11:43:58 PST

It builds!

Comment 14 Kentaro Hara 2011-12-26 16:06:39 PST

[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:-)

Comment 15 Adam Barth 2011-12-27 09:53:28 PST

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.

Comment 16 WebKit Review Bot 2011-12-27 14:32:02 PST

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>

Comment 17 WebKit Review Bot 2011-12-27 14:32:09 PST

All reviewed patches have been landed.  Closing bug.