Bug 78433 – [GTK] Mutation Observers build is broken

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Philippe Normand]

Some files are missing in the Makefile.


../../Source/WebCore/bindings/js/JSMutationCallbackCustom.cpp:35:32: error: JSMutationCallback.h: No such file or directory
../../Source/WebCore/bindings/js/JSWebKitMutationObserverCustom.cpp:35:38: error: JSWebKitMutationObserver.h: No such file or directory

[Philippe Normand]

I disabled it for GTK in r107504. Please next time before enabling a feature on all ports make sure it builds there or watch for the bots fallout.

[Philippe Normand]

Gustavo, can you please trigger clean builds in your EWS and bot?
64-bit bots are rolling but I need to clean my EWS :/

[ChangSeok Oh]

Hello. I think I have a proper patch to fix this.
Would you mind if I submit this? :)

[Philippe Normand]

(In reply to comment #3)
> Hello. I think I have a proper patch to fix this.
> Would you mind if I submit this? :)

Please go ahead!
Don't forget to unskip the tests that should pass after this feature is enabled :)

[ChangSeok Oh]

Created attachment 126775 [details]
Patch

[Martin Robinson]

Comment on attachment 126775 [details]
Patch

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

> configure.ac:656
> +                             [enable DOM mutation observer support [default=yes]]),
> +              [],[enable_mutation_observers="yes"])

You probably shouldn't make this change. It's one thing to turn a new feature on for the bots, it another to have everyone who compiles WebKitGTK+ using this new feature.

[Philippe Normand]

Comment on attachment 126775 [details]
Patch

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

>> configure.ac:656
>> +              [],[enable_mutation_observers="yes"])
> 
> You probably shouldn't make this change. It's one thing to turn a new feature on for the bots, it another to have everyone who compiles WebKitGTK+ using this new feature.

I agree, the initial patch that switched the feature on in build-webkit didn't do so in configure.ac

[ChangSeok Oh]

Comment on attachment 126775 [details]
Patch

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

>>> configure.ac:656
>>> +              [],[enable_mutation_observers="yes"])
>> 
>> You probably shouldn't make this change. It's one thing to turn a new feature on for the bots, it another to have everyone who compiles WebKitGTK+ using this new feature.
> 
> I agree, the initial patch that switched the feature on in build-webkit didn't do so in configure.ac

O.K Let me update immediately :)

[ChangSeok Oh]

Created attachment 126778 [details]
Patch

[Martin Robinson]

Comment on attachment 126778 [details]
Patch

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

> LayoutTests/platform/gtk/Skipped:1619
> +# Needed FileSystem API enabled at the same time.
> +fast/mutation/filesystem-callback-delivery.html

Do you think you can move this test up with the other block tests about the FileSystem API?

[Philippe Normand]

(In reply to comment #10)
> (From update of attachment 126778 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=126778&action=review
> 
> > LayoutTests/platform/gtk/Skipped:1619
> > +# Needed FileSystem API enabled at the same time.
> > +fast/mutation/filesystem-callback-delivery.html
> 
> Do you think you can move this test up with the other block tests about the FileSystem API?

Can we also wait EWS before landing this?

[Philippe Normand]

(In reply to comment #11)
> (In reply to comment #10)
> > (From update of attachment 126778 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=126778&action=review
> > 
> > > LayoutTests/platform/gtk/Skipped:1619
> > > +# Needed FileSystem API enabled at the same time.
> > > +fast/mutation/filesystem-callback-delivery.html
> > 
> > Do you think you can move this test up with the other block tests about the FileSystem API?
> 
> Can we also wait EWS before landing this?

Green, go!

[ChangSeok Oh]

Comment on attachment 126778 [details]
Patch

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

>>> LayoutTests/platform/gtk/Skipped:1619
>>> +fast/mutation/filesystem-callback-delivery.html
>> 
>> Do you think you can move this test up with the other block tests about the FileSystem API?
> 
> Can we also wait EWS before landing this?

I tried to build with FileSystem API but  unhappily there is also an issue like following.
>make: *** No rule to make target `../../Source/WebCore/fileapi/WebKitFlags.idl', needed by `DerivedSources/WebCore/supplemental_dependency.tmp'
So I opened http://webkit.org/b/78479 for it.

[ChangSeok Oh]

Created attachment 126781 [details]
Patch

[Adam Klein]

(In reply to comment #1)
> I disabled it for GTK in r107504. Please next time before enabling a feature on all ports make sure it builds there or watch for the bots fallout.

Sorry for the breakage, my patch was passing on EWS, which I thought would be sufficient for GTK.  For future reference, is there some difference between GTK EWS and the build bot?

[WebKit Review Bot]

Comment on attachment 126781 [details]
Patch

Clearing flags on attachment: 126781

Committed r107662: <http://trac.webkit.org/changeset/107662>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Philippe Normand]

(In reply to comment #17)
> All reviewed patches have been landed.  Closing bug.

Is a clean build needed?

  CXXLD  Programs/WebKitTestRunner
../../Source/WebCore/dom/Element.cpp:2045: error: undefined reference to 'WebCore::MutationRecord::createAttributes(WTF::PassRefPtr<WebCore::Node>, WebCore::QualifiedName const&, WTF::AtomicString const&)'
../../Source/WebCore/dom/Element.cpp:2045: error: undefined reference to 'WebCore::MutationObserverInterestGroup::enqueueMutationRecord(WTF::PassRefPtr<WebCore::MutationRecord>)'
../../Source/WebCore/dom/MutationObserverInterestGroup.h:70: error: undefined reference to 'WebCore::MutationObserverInterestGroup::createIfNeeded(WebCore::Node*, WebCore::WebKitMutationObserver::MutationType, WTF::AtomicString const&, unsigned char)'
../../Source/JavaScriptCore/wtf/OwnPtrCommon.h:54: error: undefined reference to 'WebCore::MutationObserverRegistration::~MutationObserverRegistration()'
../../Source/WebCore/dom/Node.cpp:2624: error: undefined reference to 'WebCore::MutationObserverRegistration::shouldReceiveMutationFrom(WebCore::Node*, WebCore::WebKitMutationObserver::MutationType, WTF::AtomicString const&)'
../../Source/WebCore/dom/Node.cpp:2637: error: undefined reference to 'WebCore::MutationObserverRegistration::shouldReceiveMutationFrom(WebCore::Node*, WebCore::WebKitMutationObserver::MutationType, WTF::AtomicString const&)'
../../Source/WebCore/dom/Node.cpp:2662: error: undefined reference to 'WebCore::MutationObserverRegistration::create(WTF::PassRefPtr<WebCore::WebKitMutationObserver>, WebCore::Node*)'
../../Source/WebCore/dom/Node.cpp:2708: error: undefined reference to 'WebCore::MutationObserverRegistration::observedSubtreeNodeWillDetach(WTF::PassRefPtr<WebCore::Node>)'
../../Source/WebCore/dom/Node.cpp:2713: error: undefined reference to 'WebCore::MutationObserverRegistration::observedSubtreeNodeWillDetach(WTF::PassRefPtr<WebCore::Node>)'
../../Source/WebCore/dom/ChildListMutationScope.h:52: error: undefined reference to 'WebCore::ChildListMutationScope::MutationAccumulationRouter::instance()'
../../Source/WebCore/dom/ChildListMutationScope.h:52: error: undefined reference to 'WebCore::ChildListMutationScope::MutationAccumulationRouter::incrementScopingLevel(WebCore::Node*)'
../../Source/WebCore/dom/ChildListMutationScope.h:58: error: undefined reference to 'WebCore::ChildListMutationScope::MutationAccumulationRouter::instance()'
../../Source/WebCore/dom/ChildListMutationScope.h:58: error: undefined reference to 'WebCore::ChildListMutationScope::MutationAccumulationRouter::decrementScopingLevel(WebCore::Node*)'
../../Source/JavaScriptCore/wtf/RefCounted.h:183: error: undefined reference to 'WebCore::WebKitMutationObserver::~WebKitMutationObserver()'
../../Source/WebCore/bindings/js/JSMainThreadExecState.cpp:37: error: undefined reference to 'WebCore::WebKitMutationObserver::deliverAllMutations()'
../../Source/WebCore/css/StylePropertySet.cpp:113: error: undefined reference to 'WebCore::MutationObserverInterestGroup::isOldValueRequested()'
../../Source/WebCore/css/StylePropertySet.cpp:114: error: undefined reference to 'WebCore::MutationRecord::createAttributes(WTF::PassRefPtr<WebCore::Node>, WebCore::QualifiedName const&, WTF::AtomicString const&)'
../../Source/WebCore/css/StylePropertySet.cpp:126: error: undefined reference to 'WebCore::MutationObserverInterestGroup::enqueueMutationRecord(WTF::PassRefPtr<WebCore::MutationRecord>)'
../../Source/WebCore/dom/CharacterData.cpp:196: error: undefined reference to 'WebCore::MutationRecord::createCharacterData(WTF::PassRefPtr<WebCore::Node>, WTF::String const&)'
../../Source/WebCore/dom/CharacterData.cpp:196: error: undefined reference to 'WebCore::MutationObserverInterestGroup::enqueueMutationRecord(WTF::PassRefPtr<WebCore::MutationRecord>)'
../../Source/WebCore/dom/MutationObserverInterestGroup.h:62: error: undefined reference to 'WebCore::MutationObserverInterestGroup::createIfNeeded(WebCore::Node*, WebCore::WebKitMutationObserver::MutationType, WTF::AtomicString const&, unsigned char)'
../../Source/WebCore/dom/ChildListMutationScope.h:64: error: undefined reference to 'WebCore::ChildListMutationScope::MutationAccumulationRouter::instance()'
../../Source/WebCore/dom/ChildListMutationScope.h:64: error: undefined reference to 'WebCore::ChildListMutationScope::MutationAccumulationRouter::childAdded(WebCore::Node*, WebCore::Node*)'
../../Source/WebCore/dom/ChildListMutationScope.h:70: error: undefined reference to 'WebCore::ChildListMutationScope::MutationAccumulationRouter::instance()'
../../Source/WebCore/dom/ChildListMutationScope.h:70: error: undefined reference to 'WebCore::ChildListMutationScope::MutationAccumulationRouter::willRemoveChild(WebCore::Node*, WebCore::Node*)'
DerivedSources/WebCore/JSWebKitMutationObserver.cpp:81: error: undefined reference to 'WebCore::JSWebKitMutationObserverConstructor::constructJSWebKitMutationObserver(JSC::ExecState*)'
DerivedSources/WebCore/JSWebKitMutationObserver.cpp:176: error: undefined reference to 'WebCore::JSWebKitMutationObserver::observe(JSC::ExecState*)'
DerivedSources/WebCore/JSWebKitMutationObserver.cpp:187: error: undefined reference to 'WebCore::WebKitMutationObserver::disconnect()'
collect2: ld returned 1 exit status

[Philippe Normand]

(In reply to comment #15)
> (In reply to comment #1)
> > I disabled it for GTK in r107504. Please next time before enabling a feature on all ports make sure it builds there or watch for the bots fallout.
> 
> Sorry for the breakage, my patch was passing on EWS, which I thought would be sufficient for GTK.  For future reference, is there some difference between GTK EWS and the build bot?

Not really. Since build-webkit locally installs most of the build dependencies before compiling WebKit it should be fine.

Sometimes though, bots require a clean build after some Makefile(s) update(s). I don't know what happens with that patch, it broke the build again even though the EWS was green.... :(

[Philippe Normand]

(In reply to comment #19)
> (In reply to comment #15)
> > (In reply to comment #1)
> > > I disabled it for GTK in r107504. Please next time before enabling a feature on all ports make sure it builds there or watch for the bots fallout.
> > 
> > Sorry for the breakage, my patch was passing on EWS, which I thought would be sufficient for GTK.  For future reference, is there some difference between GTK EWS and the build bot?
> 
> Not really. Since build-webkit locally installs most of the build dependencies before compiling WebKit it should be fine.
> 
> Sometimes though, bots require a clean build after some Makefile(s) update(s). I don't know what happens with that patch, it broke the build again even though the EWS was green.... :(

A clean build is needed. 64-bit bots should be fine now. Gustavo, 32-bit clean-build-request :P