Bug 198726

Summary: Roll out PAC cage
Product: WebKit Reporter: Saam Barati <saam>
Component: JavaScriptCoreAssignee: 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: P2 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

Description Saam Barati 2019-06-10 12:42:20 PDT
...
Comment 1 Saam Barati 2019-06-10 12:43:16 PDT
This will leave us with how things were, with Gigacage enabled, but no PAC cage.
Comment 2 Saam Barati 2019-06-10 14:40:11 PDT
Created attachment 371779 [details]
WIP

testing
Comment 3 EWS Watchlist 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.
Comment 4 Saam Barati 2019-06-10 16:20:29 PDT
Created attachment 371790 [details]
patch
Comment 5 EWS Watchlist 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.
Comment 6 Keith Miller 2019-06-11 01:29:15 PDT
Is this just a revert?
Comment 7 Saam Barati 2019-06-11 08:11:18 PDT
(In reply to Keith Miller from comment #6)
> Is this just a revert?

Yes. See the changelog
Comment 8 Keith Miller 2019-06-11 08:44:15 PDT
Comment on attachment 371790 [details]
patch

r=me
Comment 9 Saam Barati 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
Comment 10 Saam Barati 2019-06-11 08:53:21 PDT
Created attachment 371848 [details]
patch for landing

Thanks for the review.
Comment 11 WebKit Commit Bot 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>
Comment 12 WebKit Commit Bot 2019-06-11 10:18:56 PDT
All reviewed patches have been landed.  Closing bug.
Comment 13 Radar WebKit Bug Importer 2019-06-11 10:19:16 PDT
<rdar://problem/51628704>
Comment 14 WebKit Commit Bot 2019-06-12 11:37:17 PDT
Re-opened since this is blocked by bug 198796