Use the SafeBrowsing SPI added in bug 181804 to check URLs before loading them.
Created attachment 339558 [details] WIP
(In reply to Ali Juma from comment #1) > Created attachment 339558 [details] > WIP The main things that still need to be done here: 1) Add tests. 2) Make the warning page more detailed (i.e., match Safari's warning page), with different content for different warning types. 3) Figure out what needs to be done to make text on the warning page localizable (that is, where to put the strings -- maybe in LocalizedStringsCocoa.mm?). 4) Decide on a persistence model for user decisions to bypass warning -- should they persist for that URL for the lifetime of the WKWebView, or only for back/forward navigation to the same item, or not persist at all.
Created attachment 339580 [details] WIP Rebased
Created attachment 339730 [details] WIP Try to fix Sierra build
Created attachment 340500 [details] WIP Fixed back/forward navigation to/from a warning page
(In reply to Ali Juma from comment #2) > (In reply to Ali Juma from comment #1) > > Created attachment 339558 [details] > > WIP > > The main things that still need to be done here: > > 1) Add tests. > > 2) Make the warning page more detailed (i.e., match Safari's warning page), > with different content for different warning types. > > 3) Figure out what needs to be done to make text on the warning page > localizable (that is, where to put the strings -- maybe in > LocalizedStringsCocoa.mm?). > > 4) Decide on a persistence model for user decisions to bypass warning -- > should they persist for that URL for the lifetime of the WKWebView, or only > for back/forward navigation to the same item, or not persist at all. Another thing that still needs to be done: 5) Figure out how this feature interacts with process-swap-on-navigation. In particular, make sure swapping works as expected when navigating to/from a SafeBrowsing warning, and when bypassing a warning.
Created attachment 340514 [details] WIP Rebased
Created attachment 340519 [details] WIP Fix Release builds
Created attachment 341674 [details] WIP More back/forward list fixes
Attachment 341674 [details] did not pass style-queue: ERROR: Source/WebCore/loader/FrameLoader.cpp:1456: One line control clauses should not use braces. [whitespace/braces] [4] Total errors found: 1 in 40 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 341677 [details] WIP Fix style
Created attachment 342442 [details] WIP Add tests
Created attachment 342444 [details] WIP Fix build
Created attachment 342446 [details] WIP Fix build
Created attachment 342459 [details] WIP More build fixes
Created attachment 342555 [details] WIP
Created attachment 342591 [details] WIP
Created attachment 342674 [details] WIP
Comment on attachment 342674 [details] WIP Attachment 342674 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/8165750 New failing tests: js/dom/JSON-stringify.html
Created attachment 342682 [details] Archive of layout-test-results from ews112 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews112 Port: mac-sierra Platform: Mac OS X 10.12.6
As explained offline, to facilitate landing the change, we have the following suggestions: 1. Do not include any new API in the first patch (i.e. no client delegate) 2. Do not try and fix history just yet 3. I believe Alex might have some concerns about the global SafeBrowsingContextProvider too? Alex, could you please clarify? This should keep the patch a lot simpler and facilitate its review / prompt landing. We can build on it in follow-up patches.
Created attachment 342977 [details] Patch
(In reply to Chris Dumez from comment #21) > As explained offline, to facilitate landing the change, we have the > following suggestions: > 1. Do not include any new API in the first patch (i.e. no client delegate) Done, removed the client delegate method. > 2. Do not try and fix history just yet Removed too.
(In reply to Chris Dumez from comment #21) > 3. I believe Alex might have some concerns about the global > SafeBrowsingContextProvider too? Alex, could you please clarify? Yes, right now it's being used to hold a global static boolean that is set via an instance method with no corresponding accessor. We should really avoid global things in the WKWebView API design because they prevent us from having multiple WKWebViews with different settings.
Comment on attachment 342977 [details] Patch Attachment 342977 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/8245511 New failing tests: accessibility/mac/selection-notification-focus-change.html
Created attachment 343043 [details] Archive of layout-test-results from ews106 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6
Created attachment 343090 [details] Patch
(In reply to Alex Christensen from comment #24) > (In reply to Chris Dumez from comment #21) > > 3. I believe Alex might have some concerns about the global > > SafeBrowsingContextProvider too? Alex, could you please clarify? > Yes, right now it's being used to hold a global static boolean that is set > via an instance method with no corresponding accessor. We should really > avoid global things in the WKWebView API design because they prevent us from > having multiple WKWebViews with different settings. Good point, moved the boolean to WKWebViewConfiguration.
Created attachment 344296 [details] patch Rebased
Created attachment 344297 [details] Patch Rebased
<rdar://problem/58363818>
This was shipped in WebKit -- I'm not sure why this bug wasn't updated to reflect that.
Comment on attachment 344297 [details] Patch This was done elsewhere.