Bug 76147

Summary: [Chromium] JSExportMacros.h should be visible.
Product: WebKit Reporter: Hajime Morrita <morrita>
Component: PlatformAssignee: Hajime Morrita <morrita>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, tony, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 72855    
Attachments:
Description Flags
Patch none

Hajime Morrita
Reported 2012-01-11 21:47:28 PST
JSExportMacros.h is hidden from Chromium port by the USE(JSC) gaurd. But WebCore uses Yarr which is a part of JSC and it includes JSExportMacros.h indirectly. So Chromium should be able to see it.
Attachments
Patch (2.19 KB, patch)
2012-01-11 22:18 PST, Hajime Morrita 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Hajime Morrita
Comment 1 2012-01-11 22:18:59 PST
Created attachment 122171 [details] Patch
Adam Barth
Comment 2 2012-01-11 22:47:01 PST
Ah, Yarr. /me hopes Tony will review this patch.
Hajime Morrita
Comment 3 2012-01-12 00:08:14 PST
(In reply to comment #2) > Ah, Yarr. /me hopes Tony will review this patch. OK, thanks for taking a look!
Tony Chang
Comment 4 2012-01-12 10:03:57 PST
Comment on attachment 122171 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=122171&action=review > Source/JavaScriptCore/config.h:37 > +// Chromium doesn't have runtime/ in its include paths. > +#include "runtime/JSExportMacros.h" Should we add runtime/ to the include path for yarr? Actually, it looks like it's already there for the yarr target. Should we add it to direct_dependent_settings?
Hajime Morrita
Comment 5 2012-01-12 19:35:06 PST
(In reply to comment #4) > (From update of attachment 122171 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=122171&action=review > > > Source/JavaScriptCore/config.h:37 > > +// Chromium doesn't have runtime/ in its include paths. > > +#include "runtime/JSExportMacros.h" > > Should we add runtime/ to the include path for yarr? Actually, it looks like it's already there for the yarr Whoa, I didn't know about direct_dependent_settings. I'll try it.
Hajime Morrita
Comment 6 2012-01-12 20:01:31 PST
> > Should we add runtime/ to the include path for yarr? Actually, it looks like it's already there for the yarr Whoa, I didn't know about direct_dependent_settings. I'll try it. I just tried direct_dependent_settings but noticed that JavaScriptCore/config.h is included from not only direct_dependent but also some more (like Webkit APi layer). Including runtime/ everywhere looks a bit too much polluting for me so I'll land this as is. Anyway, thanks for pointing it out. I hope I could find some cleaner way for this.
WebKit Review Bot
Comment 7 2012-01-12 21:13:49 PST
Comment on attachment 122171 [details] Patch Clearing flags on attachment: 122171 Committed r104897: <http://trac.webkit.org/changeset/104897>
WebKit Review Bot
Comment 8 2012-01-12 21:13:54 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.