| Summary: | JSString should not have JS_EXPORTCLASS annotation | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Hajime Morrita <morrita> | ||||||
| Component: | JavaScriptCore | Assignee: | Hajime Morrita <morrita> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | ap, aroben, dbates, ggaren, kevino, webkit.review.bot | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Hajime Morrita
2011-12-28 00:54:08 PST
Created attachment 120640 [details]
Patch
JSString is used in WebCore (see e.g. DOMWrapperWorld.h). Why does it matter that it's not part of a public API? (In reply to comment #2) > JSString is used in WebCore (see e.g. DOMWrapperWorld.h). Why does it matter that it's not part of a public API? It is WebKit policy that APIs that are not public should export only the methods needed by WebCore, etc., to discourage third party developers from coding against those APIs. Hajime's patch is not removing the JSString exports entirely, just decreasing what is exported to only what is needed. Comment on attachment 120640 [details]
Patch
It looks like Windows needs the class declaration exported in order to provide access to JSString's vtable. I'd have to double-check, but I think the reason this patch works for other platforms is that gcc lets you add the class's vtable to the symbol exports file, but Windows does not. This is probably why we have JS_EXPORTCLASS and JS_EXPORTDATA macros even though we're using symbol export files.
(In reply to comment #4) > (From update of attachment 120640 [details]) > It looks like Windows needs the class declaration exported in order to provide access to JSString's vtable. I'd have to double-check, but I think the reason this patch works for other platforms is that gcc lets you add the class's vtable to the symbol exports file, but Windows does not. This is probably why we have JS_EXPORTCLASS and JS_EXPORTDATA macros even though we're using symbol export files. Sorry, by "Windows" I meant the Apple Windows port's compiler, that is, Visual C++. Oliver, thanks for your explanation and investigation.
> It looks like Windows needs the class declaration exported in order to provide access to JSString's vtable. I'd have to double-check, but I think the reason this patch works for other platforms is that gcc lets you add the class's vtable to the symbol exports file, but Windows does not. This is probably why we have JS_EXPORTCLASS and JS_EXPORTDATA macros even though we're using symbol export files
Interesting.
And I didn't know that Apple Windows port already relies on the macro.
I'll try another patch to examine my understanding. If if doesn't work, I'll close this bug as invalid.
Created attachment 121408 [details]
An attempt to fix windows build break
Comment on attachment 121408 [details] An attempt to fix windows build break Clearing flags on attachment: 121408 Committed r104359: <http://trac.webkit.org/changeset/104359> All reviewed patches have been landed. Closing bug. FYI, this caused bug 76101 (In reply to comment #10) > FYI, this caused bug 76101 Oops. Thanks for fixing that. |