Bug 116122

Summary: [Mac] Add a testing shim for secure event input functions
Product: WebKit Reporter: Alexey Proskuryakov <ap>
Component: Tools / TestsAssignee: Alexey Proskuryakov <ap>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dpranke, glenn, ossy, zan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 116121    
Attachments:
Description Flags
proposed patch mrowe: review+

Description Alexey Proskuryakov 2013-05-14 12:58:14 PDT
Secure event input is set per user session. When running tests, we should not change it, or depend on it.
Comment 1 Alexey Proskuryakov 2013-05-14 13:43:19 PDT
Created attachment 201755 [details]
proposed patch
Comment 2 Mark Rowe (bdash) 2013-05-14 14:21:40 PDT
Comment on attachment 201755 [details]
proposed patch

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?
Comment 3 Alexey Proskuryakov 2013-05-14 15:08:55 PDT
Committed <http://trac.webkit.org/r150089>.
Comment 4 Csaba Osztrogonác 2013-05-15 03:56:32 PDT
(In reply to comment #3)
> Committed <http://trac.webkit.org/r150089>.

It broke a webkitpy unit test on the bots:

  Traceback (most recent call last):
    File "/ramdisk/qt-linux-release/build/Tools/Scripts/webkitpy/port/mac_unittest.py", line 119, in test_setup_environ_for_server
      self.assertEqual(env['DYLD_INSERT_LIBRARIES'], '/usr/lib/libgmalloc.dylib')
  AssertionError: '/usr/lib/libgmalloc.dylib:/mock-build/libWebCoreTestShim.dylib' != '/usr/lib/libgmalloc.dylib'
  
[617/1456] webkitpy.common.checkout.checkout_unittest.CommitMessageForThisCommitTest.test_commit_message_for_this_commit (+11)         

Could you fix it, please?
Comment 5 Alexey Proskuryakov 2013-05-15 10:00:57 PDT
Zan just fixed in <http://trac.webkit.org/changeset/150116>. Sorry for the breakage.