| Summary: | Fix the !ENABLE(FILTERS) && !ENABLE(CSS_FILTERS) build after r167497 | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Tibor Mészáros <mtiborinf> | ||||||
| Component: | WebKit Misc. | Assignee: | Nobody <webkit-unassigned> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | commit-queue, darin, d-r, esprehn+autocc, fpizlo, galpeter, ggaren, gyuyoung.kim, ossy, sam | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 131797 | ||||||||
| Attachments: |
|
||||||||
|
Description
Tibor Mészáros
2014-07-07 03:48:47 PDT
Created attachment 234484 [details]
Patch
Fix for the bug
Comment on attachment 234484 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=234484&action=review I’d like to hear what some JavaScript bindings expert has to say about this. > Source/WebCore/platform/graphics/ImageBuffer.h:39 > +#include <runtime/JSCInlines.h> > +#include <runtime/TypedArrayInlines.h> This is really unfortunate; includes that are only needed for the JavaScript bindings really don’t belong in the DOM implementation itself. However, there may be no other solution to this problem. > Source/WebCore/platform/graphics/ImageBuffer.h:40 > #include <runtime/Uint8ClampedArray.h> This already-existing include is equally bad. Comment on attachment 234484 [details]
Patch
Can we put these includes into the two affected .cpp files rather than into the header?
(In reply to comment #3) > (From update of attachment 234484 [details]) > Can we put these includes into the two affected .cpp files rather than into the header? I will try to do what you wished. If I got some result, i will upload a new patch. Comment on attachment 234484 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=234484&action=review This bug was already fixed by https://bugs.webkit.org/show_bug.cgi?id=130394, but r167497 removed the fix. The goal of this bug is to revert this incorrect removal which caused the build failure mentioned in the description. >> Source/WebCore/platform/graphics/ImageBuffer.h:39 >> +#include <runtime/TypedArrayInlines.h> > > This is really unfortunate; includes that are only needed for the JavaScript bindings really don’t belong in the DOM implementation itself. However, there may be no other solution to this problem. There are many includes of this headers everywhere, I don't think if we can avoid them: $ grep -R "runtime/TypedArrayInlines.h" . ./WebCore/Modules/webaudio/AudioBuffer.cpp:#include <runtime/TypedArrayInlines.h> ./WebCore/testing/MockCDM.cpp:#include <runtime/TypedArrayInlines.h> ./WebCore/platform/graphics/filters/FilterEffect.cpp:#include <runtime/TypedArrayInlines.h> ./WebCore/platform/graphics/filters/FEGaussianBlur.cpp:#include <runtime/TypedArrayInlines.h> ./WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:#import <runtime/TypedArrayInlines.h> ./WebCore/html/canvas/WebGLRenderingContext.cpp:#include <runtime/TypedArrayInlines.h> ./WebCore/bindings/js/SerializedScriptValue.cpp:#include <runtime/TypedArrayInlines.h> ./WebCore/bindings/js/JSDOMBinding.h:#include <runtime/TypedArrayInlines.h> ./WebCore/bindings/js/JSWebGLRenderingContextCustom.cpp:#include <runtime/TypedArrayInlines.h> ./WebCore/bindings/js/JSCryptoAlgorithmBuilder.cpp:#include <runtime/TypedArrayInlines.h> $ grep -R "runtime/JSCInlines.h" . ./WebCore/inspector/WebConsoleAgent.cpp:#include <runtime/JSCInlines.h> ./WebCore/inspector/InspectorDOMAgent.cpp:#include <runtime/JSCInlines.h> ./WebCore/xml/XMLHttpRequest.cpp:#include <runtime/JSCInlines.h> ./WebCore/bridge/jsc/BridgeJSC.h:#include <runtime/JSCInlines.h> ./WebCore/bridge/c/c_utility.h:#include <runtime/JSCInlines.h> ./WebCore/Modules/webaudio/AudioBuffer.cpp:#include <runtime/JSCInlines.h> ./WebCore/Modules/mediasource/SourceBuffer.cpp:#include <runtime/JSCInlines.h> ./WebCore/testing/MockCDM.cpp:#include <runtime/JSCInlines.h> ./WebCore/testing/Internals.cpp:#include <runtime/JSCInlines.h> ./WebCore/platform/SerializedPlatformRepresentation.h:#include <runtime/JSCInlines.h> ./WebCore/platform/graphics/filters/FilterEffect.cpp:#include <runtime/JSCInlines.h> ./WebCore/platform/graphics/filters/FEGaussianBlur.cpp:#include <runtime/JSCInlines.h> ./WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:#import <runtime/JSCInlines.h> ./WebCore/html/HTMLCanvasElement.cpp:#include <runtime/JSCInlines.h> ./WebCore/html/track/DataCue.h:#include <runtime/JSCInlines.h> ./WebCore/html/canvas/WebGLRenderingContext.cpp:#include <runtime/JSCInlines.h> ./WebCore/html/HTMLImageLoader.cpp:#include <runtime/JSCInlines.h> ./WebCore/bindings/js/JSNodeFilterCondition.h:#include <runtime/JSCInlines.h> ./WebCore/bindings/js/SerializedScriptValue.cpp:#include <runtime/JSCInlines.h> ./WebCore/bindings/js/JSCryptoKeySerializationJWK.cpp:#include <runtime/JSCInlines.h> ./WebCore/bindings/js/JSCustomXPathNSResolver.h:#include <runtime/JSCInlines.h> ./WebCore/bindings/js/JSDictionary.h:#include <runtime/JSCInlines.h> ./WebCore/bindings/js/WebCoreTypedArrayController.cpp:#include <runtime/JSCInlines.h> ./WebCore/bindings/js/JSMessagePortCustom.h:#include <runtime/JSCInlines.h> ./WebCore/bindings/js/JSCryptoAlgorithmBuilder.cpp:#include <runtime/JSCInlines.h> ./WebCore/dom/PopStateEvent.cpp:#include <runtime/JSCInlines.h> ./WebCore/dom/CustomEvent.cpp:#include <runtime/JSCInlines.h> ./WebCore/dom/MessageEvent.cpp:#include <runtime/JSCInlines.h> ./WebCore/dom/Node.cpp:#include <runtime/JSCInlines.h> ./WebKit2/Shared/linux/WebMemorySamplerLinux.cpp:#include <runtime/JSCInlines.h> ./WebKit2/WebProcess/WebPage/WebPage.cpp:#include <runtime/JSCInlines.h> ./WebKit2/UIProcess/WebContext.cpp:#include <runtime/JSCInlines.h> >> Source/WebCore/platform/graphics/ImageBuffer.h:40 >> #include <runtime/Uint8ClampedArray.h> > > This already-existing include is equally bad. PassRefPtr<Uint8ClampedArray> is used in line 105 and 106, we really need this include: http://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/ImageBuffer.h#L105 It didn't cause build failures before, because it was built into filters objects. But I don't think if it is good if ImageBuffer depends on something built to a filter file. $ grep -R "runtime/Uint8ClampedArray.h" . ./WebCore/platform/graphics/filters/FilterEffect.cpp:#include <runtime/Uint8ClampedArray.h> ./WebCore/platform/graphics/filters/FEComponentTransfer.cpp:#include <runtime/Uint8ClampedArray.h> ./WebCore/platform/graphics/filters/FEGaussianBlur.cpp:#include <runtime/Uint8ClampedArray.h> ./WebCore/platform/graphics/filters/FEConvolveMatrix.cpp:#include <runtime/Uint8ClampedArray.h> ./WebCore/platform/graphics/filters/FilterEffect.h:#include <runtime/Uint8ClampedArray.h> ./WebCore/platform/graphics/filters/FEBlend.cpp:#include <runtime/Uint8ClampedArray.h> ./WebCore/platform/graphics/filters/FEMorphology.cpp:#include <runtime/Uint8ClampedArray.h> ./WebCore/platform/graphics/filters/FEColorMatrix.cpp:#include <runtime/Uint8ClampedArray.h> ./WebCore/platform/graphics/filters/FEComposite.cpp:#include <runtime/Uint8ClampedArray.h> ./WebCore/platform/graphics/filters/FEDropShadow.cpp:#include <runtime/Uint8ClampedArray.h> ./WebCore/platform/graphics/filters/FELighting.h:#include <runtime/Uint8ClampedArray.h> ./WebCore/platform/graphics/filters/FEDisplacementMap.cpp:#include <runtime/Uint8ClampedArray.h> ./WebCore/platform/graphics/filters/FETurbulence.cpp:#include <runtime/Uint8ClampedArray.h> ./WebCore/platform/graphics/cg/ImageBufferDataCG.h:#include <runtime/Uint8ClampedArray.h> ./WebCore/platform/graphics/ImageBuffer.h:#include <runtime/Uint8ClampedArray.h> ./WebCore/html/ImageData.h:#include <runtime/Uint8ClampedArray.h> cc-ing Filip too, he landed a huge patch related to this inlines files - https://trac.webkit.org/changeset/163844 Created attachment 234854 [details]
Patch v2
This patch will add the two include into the affected .cpp files.
Comment on attachment 234854 [details] Patch v2 Clearing flags on attachment: 234854 Committed r171083: <http://trac.webkit.org/changeset/171083> All reviewed patches have been landed. Closing bug. |