[webkit-reviews] review canceled: [Bug 173793] Move JSONValues to WTF and convert uses of InspectorValues.h to JSONValues.h : [Attachment 327352] Proposed Fix v2

[bugzilla-daemon at webkit.org]

Brian Burg <bburg at apple.com> has canceled Brian Burg <bburg at apple.com>'s
request for review:
Bug 173793: Move JSONValues to WTF and convert uses of InspectorValues.h to
JSONValues.h
https://bugs.webkit.org/show_bug.cgi?id=173793

Attachment 327352: Proposed Fix v2

https://bugs.webkit.org/attachment.cgi?id=327352&action=review




--- Comment #40 from Brian Burg <bburg at apple.com> ---
Comment on attachment 327352
  --> https://bugs.webkit.org/attachment.cgi?id=327352
Proposed Fix v2

View in context: https://bugs.webkit.org/attachment.cgi?id=327352&action=review

>> Source/WTF/wtf/JSONValues.h:39
>> +namespace JSON {
> 
> Are we starting to put things in WTF that aren't in namespace WTF?  Could
this be "namespace WTF { namespace JSON {" ?

Hi Alex. I don't want to have to use 'using WTF;' or 'WTF::JSON::Value'
throughout the entire codebase. If we can export the necessary symbols in the
header, as we do in WTFString.h, then I am fine with the suggestion.

>> Source/WTF/wtf/JSONValues.h:46
>> +class WTF_EXPORT_PRIVATE Value : public RefCounted<Value> {
> 
> This is all duplicated logic.  Please add // FIXME: Merge this with JSONParse

Yes. I'm not sure if a bug already existed for that. It would need to be
event-based, but JSONParse currently assumes a JSC object model. JSON::Value
objects are not associated with a particular JSContext and are used in places
such as WebDriver server where JS does not execute.