[webkit-reviews] review granted: [Bug 116122] [Mac] Add a testing shim for secure event input functions : [Attachment 201755] proposed patch

[bugzilla-daemon at webkit.org]

Mark Rowe (bdash) <mrowe at apple.com> has granted Alexey Proskuryakov
<ap at webkit.org>'s request for review:
Bug 116122: [Mac] Add a testing shim for secure event input functions
https://bugs.webkit.org/show_bug.cgi?id=116122

Attachment 201755: proposed patch
https://bugs.webkit.org/attachment.cgi?id=201755&action=review

------- Additional Comments from Mark Rowe (bdash) <mrowe at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=201755&action=review


r=me, with several comments.

> Source/WebCore/Configurations/WebCoreTestShim.xcconfig:24
> +#include "WebCore.xcconfig"

What settings from WebCore.xcconfig does this need? Can they be moved to
Base.xcconfig instead so we don't have to reset a bunch of things below?

> Source/WebCore/Configurations/WebCoreTestShim.xcconfig:29
> +PRIVATE_HEADERS_FOLDER_PATH =
$(PRIVATE_HEADERS_FOLDER_PATH_$(CONFIGURATION));
> +PRIVATE_HEADERS_FOLDER_PATH_Debug = WebCoreTestShim;
> +PRIVATE_HEADERS_FOLDER_PATH_Release = $(PRIVATE_HEADERS_FOLDER_PATH_Debug);
> +PRIVATE_HEADERS_FOLDER_PATH_Production = /usr/local/include/WebCoreTestShim;


I wouldn't expect a shim to need to install headers. Can these simply be
removed?

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:27347
> +				PRODUCT_NAME = WebCoreTestShim;

This is set in the .xcconfig file so I think it's redundant to include in the
project too.

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:27355
> +				PRODUCT_NAME = WebCoreTestShim;

Ditto.

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:27363
> +				PRODUCT_NAME = WebCoreTestShim;

Ditto.

> Source/WebCore/platform/mac/DyldInterpose.h:26
> +#ifndef DyldInterpose_h

Ugh, seeing dyld with this capitalization rubs me the wrong way. Can we use
this opportunity to rename it to something without dyld in the name to avoid
this problem, like DynamicLinkerInterposing.h?

> Source/WebKit2/PluginProcess/mac/PluginProcessShim.mm:-29
> -#import "DYLDInterpose.h"

I wonder how this ever built on case-sensitive filesystems?