[chromium] use relative paths for includes in the TestRunner's public API
Created attachment 170068 [details] Patch
Comment on attachment 170068 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=170068&action=review > Tools/DumpRenderTree/chromium/TestRunner/public/WebEventSender.h:34 > -#include "WebDragOperation.h" > +#include "../../../../../Source/WebKit/chromium/public/WebDragOperation.h" Yuck. I think we should just add "Source" to the include path and be able to include these headers as: #include "WebKit/chromium/public/WebDragOperation.h" That would also solve our problem of including Platform/chromium/public headers in WebKit/chromium/public.
Comment on attachment 170068 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=170068&action=review > Tools/ChangeLog:8 > + When including the headers from chromium, we need to specify the full path. Gross! And not really true. What target is including these headers? It almost certainly has quite a few things on the include path already, one of which we could definitely use
Comment on attachment 170068 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=170068&action=review >> Tools/ChangeLog:8 >> + When including the headers from chromium, we need to specify the full path. > > Gross! And not really true. What target is including these headers? It almost certainly has quite a few things on the include path already, one of which we could definitely use content_shell_lib in chromium (i can post a patch, it's not yet landed) Note that the headers in e.g. Source/WebKit/chromium/public/platform have similar includes.
(In reply to comment #4) > (From update of attachment 170068 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=170068&action=review > > >> Tools/ChangeLog:8 > >> + When including the headers from chromium, we need to specify the full path. > > > > Gross! And not really true. What target is including these headers? It almost certainly has quite a few things on the include path already, one of which we could definitely use > > content_shell_lib in chromium (i can post a patch, it's not yet landed) > > Note that the headers in e.g. Source/WebKit/chromium/public/platform have similar includes. https://codereview.chromium.org/11245004/
Can you make the DumpRenderTree.gyp:TestRunner target put something reasonable on the include path of direct_dependent_settings?
Created attachment 170213 [details] Patch
(In reply to comment #6) > Can you make the DumpRenderTree.gyp:TestRunner target put something reasonable on the include path of direct_dependent_settings? that seems to be working
Comment on attachment 170213 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=170213&action=review > Tools/DumpRenderTree/DumpRenderTree.gyp/DumpRenderTree.gyp:112 > + ['inside_chromium_build==1', { > + 'direct_dependent_settings': { > + 'include_dirs': [ > + '../../../Source/WebKit/chromium/public', > + ], > + }, > + }], Is this better than adding ../../../Source ? I guess it depends how explicit we want to be about where these headers come from.
We eventually want to get rid of the "forwarding" headers in WebKit/chromium/public/platform. We should pick something here that can scale to that case too.
Created attachment 170255 [details] Patch
(In reply to comment #10) > We eventually want to get rid of the "forwarding" headers in WebKit/chromium/public/platform. We should pick something here that can scale to that case too. like this?
Comment on attachment 170255 [details] Patch I'm happy with that. @jamesr ? (Jochen, please feel free to land this patch if its blocking you. We can iterate based on jamesr's feedback.)
Comment on attachment 170255 [details] Patch Looks good to me!
Comment on attachment 170255 [details] Patch James ack'd via chat
Comment on attachment 170255 [details] Patch Clearing flags on attachment: 170255 Committed r132370: <http://trac.webkit.org/changeset/132370>
All reviewed patches have been landed. Closing bug.