Bug 184721

Summary:

The InternalFunction hierarchy should be in IsoSubspaces

Product:

WebKit

Reporter:

Filip Pizlo <fpizlo>

Component:

JavaScriptCore

Assignee:

Filip Pizlo <fpizlo>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ews-watchlist, saam, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

All

OS:

All

Attachments:

Description	Flags
possibly the patch	none
maybe the patch	none
maybe the patch	none
maybe the patch	none
the patch	none
the patch	none
the patch	none
the patch	saam: review+
patch for landing	none

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.