[chromium] include webcore_test_support in the webkit target
Created attachment 169500 [details] Patch
Comment on attachment 169500 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=169500&action=review > Source/WebKit/chromium/WebKit.gyp:72 > + '../../WebCore/WebCore.gyp/WebCore.gyp:webcore_test_support', I don't understand. We don't want to link this code in production. That's why we have it as a separate dependency.
(In reply to comment #2) > (From update of attachment 169500 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=169500&action=review > > > Source/WebKit/chromium/WebKit.gyp:72 > > + '../../WebCore/WebCore.gyp/WebCore.gyp:webcore_test_support', > > I don't understand. We don't want to link this code in production. That's why we have it as a separate dependency. I suggested jochen move this. The idea is to expose this functionality to the content_shell-based DRT implementation. Is there a reason this is guarded in particular vs other testing APIs we expose through WebKit API?
I don't think we have a way to distinguish "WebKit.dll being used for production" from "WebKit.dll being used for content_shell based DRT". If layout tests rely on this functionality (which they do) and we insist on not making this available on WebKit.dll then there's not going to be any way to run layout tests with a content_shell based executable.
Comment on attachment 169500 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=169500&action=review >> Source/WebKit/chromium/WebKit.gyp:72 >> + '../../WebCore/WebCore.gyp/WebCore.gyp:webcore_test_support', > > I don't understand. We don't want to link this code in production. That's why we have it as a separate dependency. The problem with the separate dependency is that it's not always compiled. Also, why is it so bad to ship this code? It has to be explicitly enabled to do something
WebKit.dll is never used in production. We do link webcore_test_support into WebKit.dll today, but not into the targets that depend on WebKit in the non-component build. Does that mean we're going to link TestRunner.a into production builds as well? We might need to create a new top-level target that is ContentShell + TestRunner. Maybe we should call it WebKitTestRunner since it is the moral equivalent of http://trac.webkit.org/browser/trunk/Tools/WebKitTestRunner ? > Also, why is it so bad to ship this code? It has to be explicitly enabled to do something There's no reason to ship test-only code to production. It just bloats the binary and introduces security risks.
Created attachment 169517 [details] Patch
Comment on attachment 169517 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=169517&action=review We could alos make this a dependency of TestRunner once we build that as a separate lib. > Source/WebKit/chromium/WebKit.gyp:876 > + # WARNING: Do not view this particular case as a precedent for > + # including WebCore headers in DumpRenderTree project. This warning seems unneeded now.
Created attachment 169519 [details] Patch for landing
Comment on attachment 169519 [details] Patch for landing Clearing flags on attachment: 169519 Committed r131835: <http://trac.webkit.org/changeset/131835>
All reviewed patches have been landed. Closing bug.