184721 – The InternalFunction hierarchy should be in IsoSubspaces

RESOLVED FIXED 184721

The InternalFunction hierarchy should be in IsoSubspaces

https://bugs.webkit.org/show_bug.cgi?id=184721

Summary The InternalFunction hierarchy should be in IsoSubspaces

Filip Pizlo

Reported 2018-04-17 17:31:06 PDT

With the caveat that subclasses of InternalFunction that don't add new fields may as well share the same IsoSubspace as InternalFunction, since I can't see a security downside of doing so. They will dynamically figure out what to do based on state in JSCell, JSObject, JSDestructibleObject, and InternalFunction.

Attachments
possibly the patch (45.21 KB, patch) 2018-04-17 17:34 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
maybe the patch (47.05 KB, patch) 2018-04-17 17:42 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
maybe the patch (47.05 KB, patch) 2018-04-17 18:46 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
maybe the patch (47.06 KB, patch) 2018-04-17 19:02 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (47.42 KB, patch) 2018-04-17 19:22 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (47.42 KB, patch) 2018-04-17 19:24 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (47.42 KB, patch) 2018-04-17 19:52 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (47.52 KB, patch) 2018-04-17 20:01 PDT, Filip Pizlo	saam: review+	Details Formatted Diff Diff
patch for landing (47.40 KB, patch) 2018-04-19 11:17 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
Show Obsolete (8) View All Add attachment proposed patch, testcase, etc.

Filip Pizlo

Comment 1 2018-04-17 17:34:02 PDT

Created attachment 338169 [details] possibly the patch

EWS Watchlist

Comment 2 2018-04-17 17:37:12 PDT

Attachment 338169 [details] did not pass style-queue: ERROR: Source/WebKit/WebProcess/Plugins/Netscape/JSNPMethod.cpp:66: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/bindings/js/WebCoreJSClientData.cpp:38: Bad include order. Mixing system and custom headers. [build/include_order] [4] ERROR: Source/WebKit/WebProcess/Plugins/Netscape/JSNPObject.cpp:534: More than one command on the same line [whitespace/newline] [4] Total errors found: 3 in 31 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 3 2018-04-17 17:42:52 PDT

Created attachment 338171 [details] maybe the patch

EWS Watchlist

Comment 4 2018-04-17 17:44:20 PDT

Attachment 338171 [details] did not pass style-queue: ERROR: Source/WebKit/WebProcess/Plugins/Netscape/JSNPMethod.cpp:66: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/bindings/js/WebCoreJSClientData.cpp:38: Bad include order. Mixing system and custom headers. [build/include_order] [4] ERROR: Source/WebKit/WebProcess/Plugins/Netscape/JSNPObject.cpp:534: More than one command on the same line [whitespace/newline] [4] WARNING: File exempt from style guide. Skipping: "Source/JavaScriptCore/API/glib/JSCCallbackFunction.h" ERROR: Source/JavaScriptCore/API/glib/JSCCallbackFunction.cpp:212: More than one command on the same line [whitespace/newline] [4] Total errors found: 4 in 33 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 5 2018-04-17 18:46:15 PDT

Created attachment 338180 [details] maybe the patch

EWS Watchlist

Comment 6 2018-04-17 18:48:07 PDT

Attachment 338180 [details] did not pass style-queue: ERROR: Source/WebKit/WebProcess/Plugins/Netscape/JSNPMethod.cpp:66: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/bindings/js/WebCoreJSClientData.cpp:38: Bad include order. Mixing system and custom headers. [build/include_order] [4] ERROR: Source/WebKit/WebProcess/Plugins/Netscape/JSNPObject.cpp:534: More than one command on the same line [whitespace/newline] [4] WARNING: File exempt from style guide. Skipping: "Source/JavaScriptCore/API/glib/JSCCallbackFunction.h" ERROR: Source/JavaScriptCore/API/glib/JSCCallbackFunction.cpp:212: More than one command on the same line [whitespace/newline] [4] Total errors found: 4 in 33 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 7 2018-04-17 19:02:34 PDT

Created attachment 338181 [details] maybe the patch

EWS Watchlist

Comment 8 2018-04-17 19:05:39 PDT

Attachment 338181 [details] did not pass style-queue: ERROR: Source/WebKit/WebProcess/Plugins/Netscape/JSNPMethod.cpp:66: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/bindings/js/WebCoreJSClientData.cpp:38: Bad include order. Mixing system and custom headers. [build/include_order] [4] ERROR: Source/WebKit/WebProcess/Plugins/Netscape/JSNPObject.cpp:534: More than one command on the same line [whitespace/newline] [4] WARNING: File exempt from style guide. Skipping: "Source/JavaScriptCore/API/glib/JSCCallbackFunction.h" ERROR: Source/JavaScriptCore/API/glib/JSCCallbackFunction.cpp:212: More than one command on the same line [whitespace/newline] [4] Total errors found: 4 in 33 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 9 2018-04-17 19:22:03 PDT

Created attachment 338183 [details] the patch

Filip Pizlo

Comment 10 2018-04-17 19:24:47 PDT

Created attachment 338184 [details] the patch

EWS Watchlist

Comment 11 2018-04-17 19:26:10 PDT

Attachment 338184 [details] did not pass style-queue: ERROR: Source/WebKit/WebProcess/Plugins/Netscape/JSNPMethod.cpp:66: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/bindings/js/WebCoreJSClientData.cpp:38: Bad include order. Mixing system and custom headers. [build/include_order] [4] ERROR: Source/WebKit/WebProcess/Plugins/Netscape/JSNPObject.cpp:534: More than one command on the same line [whitespace/newline] [4] WARNING: File exempt from style guide. Skipping: "Source/JavaScriptCore/API/glib/JSCCallbackFunction.h" ERROR: Source/JavaScriptCore/API/glib/JSCCallbackFunction.cpp:212: More than one command on the same line [whitespace/newline] [4] Total errors found: 4 in 33 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 12 2018-04-17 19:52:02 PDT

Created attachment 338188 [details] the patch

EWS Watchlist

Comment 13 2018-04-17 19:54:05 PDT

Attachment 338188 [details] did not pass style-queue: ERROR: Source/WebKit/WebProcess/Plugins/Netscape/JSNPMethod.cpp:66: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/bindings/js/WebCoreJSClientData.cpp:38: Bad include order. Mixing system and custom headers. [build/include_order] [4] ERROR: Source/WebKit/WebProcess/Plugins/Netscape/JSNPObject.cpp:534: More than one command on the same line [whitespace/newline] [4] WARNING: File exempt from style guide. Skipping: "Source/JavaScriptCore/API/glib/JSCCallbackFunction.h" ERROR: Source/JavaScriptCore/API/glib/JSCCallbackFunction.cpp:212: More than one command on the same line [whitespace/newline] [4] Total errors found: 4 in 33 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 14 2018-04-17 20:01:02 PDT

Created attachment 338189 [details] the patch

EWS Watchlist

Comment 15 2018-04-17 20:04:50 PDT

Attachment 338189 [details] did not pass style-queue: ERROR: Source/WebKit/WebProcess/Plugins/Netscape/JSNPMethod.cpp:66: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/bindings/js/WebCoreJSClientData.cpp:38: Bad include order. Mixing system and custom headers. [build/include_order] [4] ERROR: Source/WebKit/WebProcess/Plugins/Netscape/JSNPObject.cpp:534: More than one command on the same line [whitespace/newline] [4] WARNING: File exempt from style guide. Skipping: "Source/JavaScriptCore/API/glib/JSCCallbackFunction.h" ERROR: Source/JavaScriptCore/API/glib/JSCCallbackFunction.cpp:35: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/API/glib/JSCCallbackFunction.cpp:213: More than one command on the same line [whitespace/newline] [4] Total errors found: 5 in 33 files If any of these errors are false positives, please file a bug against check-webkit-style.

Saam Barati

Comment 16 2018-04-18 15:10:17 PDT

Comment on attachment 338189 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=338189&action=review > Source/JavaScriptCore/heap/IsoSubspacePerVM.cpp:65 > + if (result) { style nit: I feel result.isNewEntry is easier to read than this (I just had to look up that AddResult has operator bool that returns newEntry).

Filip Pizlo

Comment 17 2018-04-19 11:15:30 PDT

(In reply to Saam Barati from comment #16) > Comment on attachment 338189 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=338189&action=review > > > Source/JavaScriptCore/heap/IsoSubspacePerVM.cpp:65 > > + if (result) { > > style nit: I feel result.isNewEntry is easier to read than this (I just had > to look up that AddResult has operator bool that returns newEntry). Makes sense, will change.

Filip Pizlo

Comment 18 2018-04-19 11:17:39 PDT

Created attachment 338345 [details] patch for landing

EWS Watchlist

Comment 19 2018-04-19 11:20:02 PDT

Attachment 338345 [details] did not pass style-queue: ERROR: Source/WebKit/WebProcess/Plugins/Netscape/JSNPMethod.cpp:66: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/bindings/js/WebCoreJSClientData.cpp:38: Bad include order. Mixing system and custom headers. [build/include_order] [4] ERROR: Source/WebKit/WebProcess/Plugins/Netscape/JSNPObject.cpp:534: More than one command on the same line [whitespace/newline] [4] WARNING: File exempt from style guide. Skipping: "Source/JavaScriptCore/API/glib/JSCCallbackFunction.h" ERROR: Source/JavaScriptCore/API/glib/JSCCallbackFunction.cpp:35: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/API/glib/JSCCallbackFunction.cpp:213: More than one command on the same line [whitespace/newline] [4] Total errors found: 5 in 33 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 20 2018-04-19 12:33:21 PDT

Landed in https://trac.webkit.org/changeset/230813/webkit

Radar WebKit Bug Importer

Comment 21 2018-04-19 12:34:27 PDT

<rdar://problem/39572914>

Note You need to log in before you can comment on or make changes to this bug.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version WebKit Nightly Build

Hardware All

OS All

Product WebKit

Component JavaScriptCore

Assignee

Filip Pizlo

Reported

2018-04-17 17:31 PDT

Modified

2018-04-19 12:34 PDT History

CC List

3 users Show

URL

Keywords InRadar

Depends on

Blocks