Bug 195392

Summary: Exception is a JSCell, not a JSObject.
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, fpizlo, keith_miller, msaboff, rmorisset, saam, tzagallo, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
work in progress for EWS testing only.
none
proposed patch.
none
proposed patch. saam: review+

Description Mark Lam 2019-03-06 17:59:08 PST
Hence, it should not insert from JSDestructibleObject.
Comment 1 Mark Lam 2019-03-06 18:04:25 PST
Created attachment 363828 [details]
work in progress for EWS testing only.
Comment 2 Mark Lam 2019-03-06 18:05:23 PST
(In reply to Mark Lam from comment #0)
> Hence, it should not insert from JSDestructibleObject.

That should have said:
Hence, it should not inherit from JSDestructibleObject.

Thank you auto-correct.
Comment 3 Mark Lam 2019-03-06 22:52:11 PST
Created attachment 363847 [details]
proposed patch.
Comment 4 EWS Watchlist 2019-03-06 22:54:55 PST
Attachment 363847 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 36 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Mark Lam 2019-03-06 22:56:54 PST
Created attachment 363848 [details]
proposed patch.
Comment 6 Saam Barati 2019-03-06 23:32:19 PST
Comment on attachment 363848 [details]
proposed patch.

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

> Source/JavaScriptCore/ChangeLog:31
> +           We'll leave these as is for now.  To facilitate the conversion from Exception*

I don’t understand why you need anything special here. We already define JSValue(JSCell*)

> Source/JavaScriptCore/runtime/JSCJSValueInlines.h:189
> +inline JSValue::JSValue(Exception* exception)

This isn’t needed

> Source/JavaScriptCore/runtime/JSCast.h:36
> +    if (!(!from || from->JSCell::inherits(*from->JSCell::vm(), std::remove_pointer<To>::type::info()))) {

OOPS
Comment 7 Mark Lam 2019-03-07 02:11:36 PST
Comment on attachment 363848 [details]
proposed patch.

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

Thanks for the review.

>> Source/JavaScriptCore/ChangeLog:31
>> +           We'll leave these as is for now.  To facilitate the conversion from Exception*
> 
> I don’t understand why you need anything special here. We already define JSValue(JSCell*)

You are right.  We don't need this.  All we need is to #include "Exception.h" in "Error.h" and "ExceptionHelper.h".  Fixed.

>> Source/JavaScriptCore/runtime/JSCJSValueInlines.h:189
>> +inline JSValue::JSValue(Exception* exception)
> 
> This isn’t needed

Removed.

>> Source/JavaScriptCore/runtime/JSCast.h:36
>> +    if (!(!from || from->JSCell::inherits(*from->JSCell::vm(), std::remove_pointer<To>::type::info()))) {
> 
> OOPS

Removed.
Comment 8 Mark Lam 2019-03-07 02:17:53 PST
Landed in r242596: <http://trac.webkit.org/r242596>.
Comment 9 Radar WebKit Bug Importer 2019-03-07 02:18:17 PST
<rdar://problem/48669863>