Bug 192193 – Add initial support for 'Cross-Origin-Opener-Policy' HTTP header

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Chris Dumez]

Add initial support for 'Cross-Origin-Opener-Policy' HTTP header:
- https://github.com/whatwg/html/issues/3740

[Chris Dumez]

<rdar://problem/42013525>

[Chris Dumez]

Created attachment 356095 [details]
Patch

[EWS Watchlist]

Attachment 356095 [details] did not pass style-queue:


ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:348:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:350:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:354:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:362:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:364:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:366:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:3123:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:3125:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:3129:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
Total errors found: 9 in 36 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 356095 [details]
Patch

Attachment 356095 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/10206760

New failing tests:
http/tests/security/xss-DENIED-script-inject-into-inactive-window3.html
http/tests/security/xss-DENIED-script-inject-into-inactive-window2.html
http/tests/security/xss-DENIED-script-inject-into-inactive-window2-pson.html
http/tests/navigation/window-open-cross-origin-then-navigated-back-same-origin.html

[EWS Watchlist]

Created attachment 356103 [details]
Archive of layout-test-results from ews107 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 356095 [details]
Patch

Attachment 356095 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/10207300

New failing tests:
http/tests/security/xss-DENIED-script-inject-into-inactive-window3.html
http/tests/navigation/window-open-cross-origin-then-navigated-back-same-origin.html

[EWS Watchlist]

Created attachment 356115 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[Ryosuke Niwa]

Comment on attachment 356095 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=356095&action=review

> Source/WebCore/loader/FrameLoader.cpp:3521
> +void FrameLoader::isolateIfNecessaryDueCrossOriginOpenerPolicy(const String& content, const URL& destinationURL)

DueToCrossOriginOpenerPolicy??

> Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:3224
> +}

Can we also add a test to make sure targeted navigation wouldn't use the same namespace once we created a new browsing context group?

[Chris Dumez]

(In reply to Ryosuke Niwa from comment #8)
> Comment on attachment 356095 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=356095&action=review
> 
> > Source/WebCore/loader/FrameLoader.cpp:3521
> > +void FrameLoader::isolateIfNecessaryDueCrossOriginOpenerPolicy(const String& content, const URL& destinationURL)
> 
> DueToCrossOriginOpenerPolicy??
> 
> > Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:3224
> > +}
> 
> Can we also add a test to make sure targeted navigation wouldn't use the
> same namespace once we created a new browsing context group?

If I understand your request correctly, this is already covered by my layout test.

[Chris Dumez]

Created attachment 356182 [details]
Patch

[EWS Watchlist]

Attachment 356182 [details] did not pass style-queue:


ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:348:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:350:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:354:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:362:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:364:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:366:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:3123:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:3125:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:3129:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
Total errors found: 9 in 37 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Created attachment 356185 [details]
Patch

[Chris Dumez]

(In reply to Chris Dumez from comment #9)
> (In reply to Ryosuke Niwa from comment #8)
> > Comment on attachment 356095 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=356095&action=review
> > 
> > > Source/WebCore/loader/FrameLoader.cpp:3521
> > > +void FrameLoader::isolateIfNecessaryDueCrossOriginOpenerPolicy(const String& content, const URL& destinationURL)
> > 
> > DueToCrossOriginOpenerPolicy??
> > 
> > > Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:3224
> > > +}
> > 
> > Can we also add a test to make sure targeted navigation wouldn't use the
> > same namespace once we created a new browsing context group?
> 
> If I understand your request correctly, this is already covered by my layout
> test.

The existing layout test is doing name targeting via window.open(). My latest iteration also adds an API test that covers name targeting via <a target=x>.

[EWS Watchlist]

Attachment 356185 [details] did not pass style-queue:


ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:348:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:350:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:354:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:362:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:364:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:367:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:371:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:373:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:375:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:3132:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:3134:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:3138:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
Total errors found: 12 in 37 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Created attachment 356196 [details]
Patch

[EWS Watchlist]

Attachment 356196 [details] did not pass style-queue:


ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:348:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:350:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:354:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:362:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:364:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:367:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:371:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:373:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:375:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:3133:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:3135:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:3139:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
Total errors found: 12 in 39 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Created attachment 356213 [details]
Patch

[EWS Watchlist]

Attachment 356213 [details] did not pass style-queue:


ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:348:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:350:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:354:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:362:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:364:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:367:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:371:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:373:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:375:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:3138:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:3140:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:3144:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
Total errors found: 12 in 39 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Too complex.

[Chris Dumez]

Reopening as I am going to take another shot at this.

[Chris Dumez]

Created attachment 433910 [details]
WIP Patch

[Chris Dumez]

Created attachment 433976 [details]
WIP Patch

[Chris Dumez]

Created attachment 434029 [details]
WIP Patch

[Chris Dumez]

Created attachment 434047 [details]
WIP Patch

[Chris Dumez]

Created attachment 434127 [details]
WIP Patch

[EWS Watchlist]

This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess

[Chris Dumez]

Created attachment 434134 [details]
WIP Patch

[Chris Dumez]

Created attachment 434136 [details]
WIP Patch

[Chris Dumez]

Created attachment 434288 [details]
WIP Patch

[Chris Dumez]

Created attachment 434317 [details]
WIP Patch

[Chris Dumez]

I filed the following issue against the spec:
- https://github.com/whatwg/html/issues/6903

I had to make a change to checkIfCOOPValuesRequireBrowsingContextGroupSwitch() that is not part of the spec in order to make this test pass.

Either the spec is incomplete or I have an implementation bug elsewhere.

[Chris Dumez]

Created attachment 434433 [details]
WIP Patch

There are still some failures in imported/w3c/web-platform-tests/html/cross-origin-opener-policy/iframe-popup-same-origin-allow-popups-to-same-origin-allow-popups.https.html that I need to figure out.

There are also some blob-related failures but I may address those separately as the fix doesn't seem trivial.

[Chris Dumez]

Created attachment 434436 [details]
WIP Patch

[Chris Dumez]

Created attachment 434468 [details]
Patch

[Geoffrey Garen]

Comment on attachment 434468 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=434468&action=review

r=me

> Source/WebCore/ChangeLog:25
> +        merely so isolation of the new browsing context at JS level for now. At some point,

so -> do

[Chris Dumez]

Created attachment 434660 [details]
Patch

[EWS]

Committed r280504 (240136@main): <https://commits.webkit.org/240136@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 434660 [details].