Symbols not always properly hidden when using WebKitAdditions to introduce new API.
<rdar://87999257>
Created attachment 450070 [details] Patch
Comment on attachment 450070 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=450070&action=review > Source/WebKit/mac/replace-webkit-additions-includes.py:93 > + additions_import_pattern = re.compile(r"\#if ENABLE\(API_WEBKIT_ADDITIONS\)\n#import <WebKitAdditions/(.*)>\n#endif") (Not necessarily in this patch, but) I wonder if it's even worth throwing a #if ENABLE(API_WEBKIT_ADDITIONS) #error "…" #endif …somewhere. Not exactly sure where, though.
Committed r288658 (246466@main): <https://commits.webkit.org/246466@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 450070 [details].
Comment on attachment 450070 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=450070&action=review > Source/WebKit/UIProcess/API/Cocoa/WKWebpagePreferences.h:74 > +#if ENABLE(API_WEBKIT_ADDITIONS) Given that this foes not behave like an ENABLE macro, why make it look like one? Something like this seems like it would convey the purpose better: #if 0 // API_WEBKIT_ADDITIONS_REPLACEMENT
(In reply to Alexey Proskuryakov from comment #5) > Comment on attachment 450070 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=450070&action=review > > > Source/WebKit/UIProcess/API/Cocoa/WKWebpagePreferences.h:74 > > +#if ENABLE(API_WEBKIT_ADDITIONS) > > Given that this foes not behave like an ENABLE macro, why make it look like > one? Something like this seems like it would convey the purpose better: > > #if 0 // API_WEBKIT_ADDITIONS_REPLACEMENT Sure, this seems reasonable. I'll discuss this with Wenson & Tim and follow-up.
Reopening to attach new patch.
Created attachment 450244 [details] Follow-up to address ap's comments.
(In reply to Chris Dumez from comment #6) > (In reply to Alexey Proskuryakov from comment #5) > > Comment on attachment 450070 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=450070&action=review > > > > > Source/WebKit/UIProcess/API/Cocoa/WKWebpagePreferences.h:74 > > > +#if ENABLE(API_WEBKIT_ADDITIONS) > > > > Given that this foes not behave like an ENABLE macro, why make it look like > > one? Something like this seems like it would convey the purpose better: > > > > #if 0 // API_WEBKIT_ADDITIONS_REPLACEMENT > > Sure, this seems reasonable. I'll discuss this with Wenson & Tim and > follow-up. Thanks for the improvement suggestion :)
Committed r288747 (246540@main): <https://commits.webkit.org/246540@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 450244 [details].