Bug 76147 - [Chromium] JSExportMacros.h should be visible.
Summary: [Chromium] JSExportMacros.h should be visible.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Hajime Morrita
URL:
Keywords:
Depends on:
Blocks: 72855
  Show dependency treegraph
 
Reported: 2012-01-11 21:47 PST by Hajime Morrita
Modified: 2012-01-12 21:13 PST (History)
3 users (show)

See Also:


Attachments
Patch (2.19 KB, patch)
2012-01-11 22:18 PST, Hajime Morrita
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hajime Morrita 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.
Comment 1 Hajime Morrita 2012-01-11 22:18:59 PST
Created attachment 122171 [details]
Patch
Comment 2 Adam Barth 2012-01-11 22:47:01 PST
Ah, Yarr.  /me hopes Tony will review this patch.
Comment 3 Hajime Morrita 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!
Comment 4 Tony Chang 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?
Comment 5 Hajime Morrita 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.
Comment 6 Hajime Morrita 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.
Comment 7 WebKit Review Bot 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>
Comment 8 WebKit Review Bot 2012-01-12 21:13:54 PST
All reviewed patches have been landed.  Closing bug.