Bug 137170

Summary: REGRESSION (r172532): JSBase.h declares NSMapTable functions that are SPI
Product: WebKit Reporter: mitz
Component: JavaScriptCoreAssignee: Daniel Bates <dbates>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, benjamin, cmarcelo, commit-queue, dbates, ggaren, mjs, msaboff, oliver
Priority: P1 Keywords: InRadar, Regression
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch ggaren: review+

mitz
Reported 2014-09-26 21:14:17 PDT
<rdar://problem/18477384> As of <http://trac.webkit.org/r172532>, JSBase.h includes declarations of NSMapTable functions that are SPI. That seems inappropriate for a JavaScriptCore API header to do.
Attachments
Patch (11.39 KB, patch)
2014-09-30 10:22 PDT, Daniel Bates 		no flags
DetailsFormatted DiffDiff
Patch (11.64 KB, patch)
2014-09-30 10:33 PDT, Daniel Bates 		no flags
DetailsFormatted DiffDiff
Patch (11.63 KB, patch)
2014-09-30 10:49 PDT, Daniel Bates 		no flags
DetailsFormatted DiffDiff
Patch (11.70 KB, patch)
2014-09-30 10:57 PDT, Daniel Bates 		ggaren: review+
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
mitz
Comment 1 2014-09-26 21:18:51 PDT
Can those declarations be in the prefix header instead?
Daniel Bates
Comment 2 2014-09-30 10:14:37 PDT
(In reply to comment #1) > Can those declarations be in the prefix header instead? I could move those declarations to the prefix header. I plan to move the forward declarations for XPC SPI that we use in JavaScriptCore (*) to a file in WTF/wtf/spi (so that they can be shared with projects JavaScriptCore, WebCore, and WebKit). As we discussed in-person today (09/30), I propose moving these declarations to WTF/wtf/spi/NSMapTableSPI.h for consistency. This would also make it straight forward to use NSMapTable SPI in other projects in the future (by including WTF/wtf/spi/NSMapTableSPI.h). (*) For instance, <http://trac.webkit.org/browser/trunk/Source/JavaScriptCore/inspector/remote/RemoteInspector.mm?rev=174028#L41>
Daniel Bates
Comment 3 2014-09-30 10:22:29 PDT
Created attachment 238933 [details] Patch
Daniel Bates
Comment 4 2014-09-30 10:33:15 PDT
Created attachment 238935 [details] Patch
Daniel Bates
Comment 5 2014-09-30 10:49:21 PDT
Created attachment 238936 [details] Patch Rebased patch following <http://trac.webkit.org/changeset/174108> (bug #137254)
Daniel Bates
Comment 6 2014-09-30 10:57:00 PDT
Created attachment 238938 [details] Patch Add radar bug URL to change log entry. Also remove inline comment "__OBJC__" from file Source/JavaScriptCore/API/JSBase.h as it is obvious that the macro #endif directive corresponds to the #ifdef __OBJC__ directive.
Geoffrey Garen
Comment 7 2014-09-30 11:04:17 PDT
Comment on attachment 238938 [details] Patch r=me
Daniel Bates
Comment 8 2014-09-30 11:40:44 PDT
Committed r174110: <http://trac.webkit.org/changeset/174110>
Note You need to log in before you can comment on or make changes to this bug.