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+

Description mitz 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.
Comment 1 mitz 2014-09-26 21:18:51 PDT
Can those declarations be in the prefix header instead?
Comment 2 Daniel Bates 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>
Comment 3 Daniel Bates 2014-09-30 10:22:29 PDT
Created attachment 238933 [details]
Patch
Comment 4 Daniel Bates 2014-09-30 10:33:15 PDT
Created attachment 238935 [details]
Patch
Comment 5 Daniel Bates 2014-09-30 10:49:21 PDT
Created attachment 238936 [details]
Patch

Rebased patch following <http://trac.webkit.org/changeset/174108> (bug #137254)
Comment 6 Daniel Bates 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.
Comment 7 Geoffrey Garen 2014-09-30 11:04:17 PDT
Comment on attachment 238938 [details]
Patch

r=me
Comment 8 Daniel Bates 2014-09-30 11:40:44 PDT
Committed r174110: <http://trac.webkit.org/changeset/174110>