Bug 195392 - Exception is a JSCell, not a JSObject.
Summary: Exception is a JSCell, not a JSObject.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-03-06 17:59 PST by Mark Lam
Modified: 2019-03-07 02:18 PST (History)
9 users (show)

See Also:


Attachments
work in progress for EWS testing only. (51.16 KB, patch)
2019-03-06 18:04 PST, Mark Lam
no flags Details | Formatted Diff | Diff
proposed patch. (56.92 KB, patch)
2019-03-06 22:52 PST, Mark Lam
no flags Details | Formatted Diff | Diff
proposed patch. (56.88 KB, patch)
2019-03-06 22:56 PST, Mark Lam
sbarati: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>