Bug 175907 - Stop using PolicyCallback for new window policies
Summary: Stop using PolicyCallback for new window policies
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-08-23 14:28 PDT by Alex Christensen
Modified: 2017-08-24 14:45 PDT (History)
8 users (show)

See Also:


Attachments
Patch (10.78 KB, patch)
2017-08-23 14:38 PDT, Alex Christensen
aestes: review+
Details | Formatted Diff | Diff
patch (10.78 KB, patch)
2017-08-24 11:10 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2017-08-23 14:28:46 PDT
Stop using PolicyCallback for new window policies
Comment 1 Alex Christensen 2017-08-23 14:38:17 PDT
Created attachment 318922 [details]
Patch
Comment 2 Alex Christensen 2017-08-23 15:11:36 PDT
I made the NavigationAction copy more explicit.
http://trac.webkit.org/r221109
Comment 3 Ryan Haddad 2017-08-23 17:15:54 PDT
(In reply to Alex Christensen from comment #2)
> I made the NavigationAction copy more explicit.
> http://trac.webkit.org/r221109
This change introduced assertion failures on iOS and macOS:
https://build.webkit.org/results/Apple%20Sierra%20Debug%20WK2%20(Tests)/r221109%20(2679)/results.html
https://build.webkit.org/results/Apple%20iOS%2010%20Simulator%20Debug%20WK2%20(Tests)/r221109%20(3496)/results.html
Comment 4 Radar WebKit Bug Importer 2017-08-23 17:16:13 PDT
<rdar://problem/34048194>
Comment 5 Ryan Haddad 2017-08-23 17:26:39 PDT
(In reply to Ryan Haddad from comment #3)
> (In reply to Alex Christensen from comment #2)
> > I made the NavigationAction copy more explicit.
> > http://trac.webkit.org/r221109
> This change introduced assertion failures on iOS and macOS:
> https://build.webkit.org/results/Apple%20Sierra%20Debug%20WK2%20(Tests)/
> r221109%20(2679)/results.html
> https://build.webkit.org/results/
> Apple%20iOS%2010%20Simulator%20Debug%20WK2%20(Tests)/r221109%20(3496)/
> results.html

SHOULD NEVER BE REACHED
/Volumes/Data/slave/sierra-debug/build/Source/WebCore/loader/PolicyChecker.cpp(177) : auto WebCore::PolicyChecker::checkNewWindowPolicy(WebCore::NavigationAction &&, const WebCore::ResourceRequest &, WebCore::FormState *, const WTF::String &, NewWindowPolicyDecisionFunction)::(anonymous class)::operator()(WebCore::PolicyAction) const
Comment 6 Ryan Haddad 2017-08-23 17:31:27 PDT
Reverted r221109 for reason:

This change caused assertion failures on iOS and macOS debug bots.

Committed r221122: <http://trac.webkit.org/changeset/221122>
Comment 7 Alex Christensen 2017-08-24 11:09:21 PDT
Comment on attachment 318922 [details]
Patch

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

> Source/WebCore/loader/PolicyChecker.cpp:171
> +            break;

Oops, these need to be returns.
Comment 8 Alex Christensen 2017-08-24 11:10:35 PDT
Created attachment 318998 [details]
patch
Comment 9 WebKit Commit Bot 2017-08-24 14:45:42 PDT
Comment on attachment 318998 [details]
patch

Clearing flags on attachment: 318998

Committed r221162: <http://trac.webkit.org/changeset/221162>
Comment 10 WebKit Commit Bot 2017-08-24 14:45:43 PDT
All reviewed patches have been landed.  Closing bug.