[webkit-reviews] review granted: [Bug 136494] [iOS] Make iOS build when ENABLE_TOUCH_EVENT and ENABLE_IOS_TOUCH_EVENTS disabled : [Attachment 237576] Patch

[bugzilla-daemon at webkit.org]

Andy Estes <aestes at apple.com> has granted Daniel Bates <dbates at webkit.org>'s
request for review:
Bug 136494: [iOS] Make iOS build when ENABLE_TOUCH_EVENT and
ENABLE_IOS_TOUCH_EVENTS disabled
https://bugs.webkit.org/show_bug.cgi?id=136494

Attachment 237576: Patch
https://bugs.webkit.org/attachment.cgi?id=237576&action=review

------- Additional Comments from Andy Estes <aestes at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=237576&action=review


> Source/WebCore/bindings/js/JSDocumentCustom.cpp:38
> +#if ENABLE(TOUCH_EVENTS)
>  #include "JSTouch.h"
>  #include "JSTouchList.h"
> +#endif

We usually separate the conditional includes from the unconditional ones.

> Source/WebCore/platform/ios/ScrollAnimatorIOS.mm:33
> +#if ENABLE(TOUCH_EVENTS)
>  #include "PlatformTouchEventIOS.h"
> +#endif

Ditto.

> Source/WebCore/rendering/RenderLayer.cpp:183
> +#endif
> +#if ENABLE(IOS_TOUCH_EVENTS)
>      , m_registeredAsTouchEventListenerForScrolling(false)
> +#endif
> +#if PLATFORM(IOS)

I would just nest the ENABLE(IOS_TOUCH_EVENTS) inside the PLATFORM(IOS) block,
which would make this simpler.

> Source/WebKit/mac/MigrateHeaders.make:239
> +ifeq ($(findstring ENABLE_IOS_TOUCH_EVENTS, $(FEATURE_DEFINES)),
ENABLE_IOS_TOUCH_EVENTS)
> +all : \
> +    $(PRIVATE_HEADERS_DIR)/WebEventRegion.h
> +endif
> +

You should remove line 219.