Bug 198726

Summary:

Roll out PAC cage

Product:

WebKit

Reporter:

Saam Barati <saam>

Component:

JavaScriptCore

Assignee:

Saam Barati <saam>

Status:

RESOLVED WONTFIX

Severity:

Normal

CC:

benjamin, commit-queue, ews-watchlist, fpizlo, ggaren, gskachkov, guijemont, keith_miller, mark.lam, msaboff, rmorisset, ticaiolima, tzagallo, webkit-bug-importer, ysuzuki

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

198796

Bug Blocks:

Attachments:

Description	Flags
WIP	none
patch	keith_miller: review+
patch for landing	none

Saam Barati

Reported 2019-06-10 12:42:20 PDT

...

Attachments
WIP (110.47 KB, patch) 2019-06-10 14:40 PDT, Saam Barati	no flags	Details Formatted Diff Diff
patch (121.38 KB, patch) 2019-06-10 16:20 PDT, Saam Barati	keith_miller: review+	Details Formatted Diff Diff
patch for landing (121.37 KB, patch) 2019-06-11 08:53 PDT, Saam Barati	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Saam Barati

Comment 1 2019-06-10 12:43:16 PDT

This will leave us with how things were, with Gigacage enabled, but no PAC cage.

Saam Barati

Comment 2 2019-06-10 14:40:11 PDT

Created attachment 371779 [details] WIP testing

EWS Watchlist

Comment 3 2019-06-10 15:01:44 PDT

Attachment 371779 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/GenericTypedArrayViewInlines.h:36: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/runtime/GenericTypedArrayViewInlines.h:36: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/JavaScriptCore/runtime/GenericTypedArrayViewInlines.h:37: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBufferView.h:179: Please replace ASSERT_WITH_SECURITY_IMPLICATION() with RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(). [security/assertion] [5] ERROR: Source/JavaScriptCore/runtime/ArrayBufferView.h:195: Please replace ASSERT_WITH_SECURITY_IMPLICATION() with RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(). [security/assertion] [5] ERROR: Source/JavaScriptCore/runtime/JSArrayBufferView.cpp:53: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/WTF/wtf/CagedPtr.h:39: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/CagedPtr.h:101: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/CagedPtr.h:102: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/CagedUniquePtr.h:84: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/CagedUniquePtr.h:135: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/runtime/Options.h:496: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/runtime/Options.h:495: One space before end of line comments [whitespace/comments] [5] ERROR: Source/JavaScriptCore/runtime/Options.h:495: Should have a space between // and comment [whitespace/comments] [4] Total errors found: 14 in 44 files If any of these errors are false positives, please file a bug against check-webkit-style.

Saam Barati

Comment 4 2019-06-10 16:20:29 PDT

Created attachment 371790 [details] patch

EWS Watchlist

Comment 5 2019-06-10 16:24:06 PDT

Attachment 371790 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/ChangeLog:15: Line contains tab character. [whitespace/tab] [5] ERROR: Source/JavaScriptCore/runtime/GenericTypedArrayViewInlines.h:36: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/runtime/GenericTypedArrayViewInlines.h:36: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/JavaScriptCore/runtime/GenericTypedArrayViewInlines.h:37: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBufferView.h:179: Please replace ASSERT_WITH_SECURITY_IMPLICATION() with RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(). [security/assertion] [5] ERROR: Source/JavaScriptCore/runtime/ArrayBufferView.h:195: Please replace ASSERT_WITH_SECURITY_IMPLICATION() with RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(). [security/assertion] [5] ERROR: Source/JavaScriptCore/runtime/JSArrayBufferView.cpp:53: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/WTF/wtf/CagedPtr.h:39: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/CagedPtr.h:101: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/CagedPtr.h:102: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/CagedUniquePtr.h:84: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/CagedUniquePtr.h:135: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/runtime/Options.h:496: Code inside a namespace should not be indented. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/runtime/Options.h:495: One space before end of line comments [whitespace/comments] [5] ERROR: Source/JavaScriptCore/runtime/Options.h:495: Should have a space between // and comment [whitespace/comments] [4] Total errors found: 15 in 47 files If any of these errors are false positives, please file a bug against check-webkit-style.

Keith Miller

Comment 6 2019-06-11 01:29:15 PDT

Is this just a revert?

Saam Barati

Comment 7 2019-06-11 08:11:18 PDT

(In reply to Keith Miller from comment #6) > Is this just a revert? Yes. See the changelog

Keith Miller

Comment 8 2019-06-11 08:44:15 PDT

Comment on attachment 371790 [details] patch r=me

Saam Barati

Comment 9 2019-06-11 08:46:25 PDT

(In reply to Saam Barati from comment #7) > (In reply to Keith Miller from comment #6) > > Is this just a revert? > > Yes. See the changelog Specifically, this rolls out: r245064, r245145, r245168, r245313, r245432, r245622

Saam Barati

Comment 10 2019-06-11 08:53:21 PDT

Created attachment 371848 [details] patch for landing Thanks for the review.

WebKit Commit Bot

Comment 11 2019-06-11 10:18:54 PDT

Comment on attachment 371848 [details] patch for landing Clearing flags on attachment: 371848 Committed r246322: <https://trac.webkit.org/changeset/246322>

WebKit Commit Bot

Comment 12 2019-06-11 10:18:56 PDT

All reviewed patches have been landed. Closing bug.

Radar WebKit Bug Importer

Comment 13 2019-06-11 10:19:16 PDT

<rdar://problem/51628704>

WebKit Commit Bot

Comment 14 2019-06-12 11:37:17 PDT

Re-opened since this is blocked by bug 198796

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