WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED CONFIGURATION CHANGED
185273
Use SafeBrowsing in WKWebView
https://bugs.webkit.org/show_bug.cgi?id=185273
Summary
Use SafeBrowsing in WKWebView
Ali Juma
Reported
2018-05-03 16:06:30 PDT
Use the SafeBrowsing SPI added in
bug 181804
to check URLs before loading them.
Attachments
WIP
(42.68 KB, patch)
2018-05-04 10:09 PDT
,
Ali Juma
no flags
Details
Formatted Diff
Diff
WIP
(41.62 KB, patch)
2018-05-04 12:08 PDT
,
Ali Juma
no flags
Details
Formatted Diff
Diff
WIP
(41.82 KB, patch)
2018-05-07 10:44 PDT
,
Ali Juma
no flags
Details
Formatted Diff
Diff
WIP
(64.07 KB, patch)
2018-05-16 11:08 PDT
,
Ali Juma
no flags
Details
Formatted Diff
Diff
WIP
(63.88 KB, patch)
2018-05-16 12:21 PDT
,
Ali Juma
no flags
Details
Formatted Diff
Diff
WIP
(63.93 KB, patch)
2018-05-16 13:16 PDT
,
Ali Juma
no flags
Details
Formatted Diff
Diff
WIP
(83.33 KB, patch)
2018-05-31 11:08 PDT
,
Ali Juma
no flags
Details
Formatted Diff
Diff
WIP
(84.75 KB, patch)
2018-05-31 11:15 PDT
,
Ali Juma
no flags
Details
Formatted Diff
Diff
WIP
(131.12 KB, patch)
2018-06-11 10:25 PDT
,
Ali Juma
no flags
Details
Formatted Diff
Diff
WIP
(133.26 KB, patch)
2018-06-11 10:52 PDT
,
Ali Juma
no flags
Details
Formatted Diff
Diff
WIP
(130.79 KB, patch)
2018-06-11 10:57 PDT
,
Ali Juma
no flags
Details
Formatted Diff
Diff
WIP
(131.00 KB, patch)
2018-06-11 13:41 PDT
,
Ali Juma
no flags
Details
Formatted Diff
Diff
WIP
(131.02 KB, patch)
2018-06-12 10:09 PDT
,
Ali Juma
no flags
Details
Formatted Diff
Diff
WIP
(131.02 KB, patch)
2018-06-12 13:49 PDT
,
Ali Juma
no flags
Details
Formatted Diff
Diff
WIP
(131.15 KB, patch)
2018-06-13 10:48 PDT
,
Ali Juma
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews112 for mac-sierra
(3.02 MB, application/zip)
2018-06-13 12:44 PDT
,
EWS Watchlist
no flags
Details
Patch
(75.65 KB, patch)
2018-06-18 15:19 PDT
,
Ali Juma
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews106 for mac-sierra-wk2
(2.71 MB, application/zip)
2018-06-19 04:02 PDT
,
EWS Watchlist
no flags
Details
Patch
(77.49 KB, patch)
2018-06-19 13:28 PDT
,
Ali Juma
no flags
Details
Formatted Diff
Diff
patch
(77.16 KB, patch)
2018-07-04 11:42 PDT
,
Ali Juma
no flags
Details
Formatted Diff
Diff
Patch
(76.33 KB, patch)
2018-07-04 12:04 PDT
,
Ali Juma
achristensen
: review-
Details
Formatted Diff
Diff
Show Obsolete
(18)
View All
Add attachment
proposed patch, testcase, etc.
Ali Juma
Comment 1
2018-05-04 10:09:46 PDT
Created
attachment 339558
[details]
WIP
Ali Juma
Comment 2
2018-05-04 10:14:59 PDT
(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.
Ali Juma
Comment 3
2018-05-04 12:08:23 PDT
Created
attachment 339580
[details]
WIP Rebased
Ali Juma
Comment 4
2018-05-07 10:44:14 PDT
Created
attachment 339730
[details]
WIP Try to fix Sierra build
Ali Juma
Comment 5
2018-05-16 11:08:32 PDT
Created
attachment 340500
[details]
WIP Fixed back/forward navigation to/from a warning page
Ali Juma
Comment 6
2018-05-16 11:16:11 PDT
(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.
Ali Juma
Comment 7
2018-05-16 12:21:04 PDT
Created
attachment 340514
[details]
WIP Rebased
Ali Juma
Comment 8
2018-05-16 13:16:11 PDT
Created
attachment 340519
[details]
WIP Fix Release builds
Ali Juma
Comment 9
2018-05-31 11:08:10 PDT
Created
attachment 341674
[details]
WIP More back/forward list fixes
EWS Watchlist
Comment 10
2018-05-31 11:10:55 PDT
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.
Ali Juma
Comment 11
2018-05-31 11:15:15 PDT
Created
attachment 341677
[details]
WIP Fix style
Ali Juma
Comment 12
2018-06-11 10:25:58 PDT
Created
attachment 342442
[details]
WIP Add tests
Ali Juma
Comment 13
2018-06-11 10:52:55 PDT
Created
attachment 342444
[details]
WIP Fix build
Ali Juma
Comment 14
2018-06-11 10:57:50 PDT
Created
attachment 342446
[details]
WIP Fix build
Ali Juma
Comment 15
2018-06-11 13:41:13 PDT
Created
attachment 342459
[details]
WIP More build fixes
Ali Juma
Comment 16
2018-06-12 10:09:56 PDT
Created
attachment 342555
[details]
WIP
Ali Juma
Comment 17
2018-06-12 13:49:14 PDT
Created
attachment 342591
[details]
WIP
Ali Juma
Comment 18
2018-06-13 10:48:24 PDT
Created
attachment 342674
[details]
WIP
EWS Watchlist
Comment 19
2018-06-13 12:44:51 PDT
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
EWS Watchlist
Comment 20
2018-06-13 12:44:53 PDT
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
Chris Dumez
Comment 21
2018-06-14 11:52:42 PDT
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.
Ali Juma
Comment 22
2018-06-18 15:19:24 PDT
Created
attachment 342977
[details]
Patch
Ali Juma
Comment 23
2018-06-18 15:20:28 PDT
(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.
Alex Christensen
Comment 24
2018-06-18 15:40:57 PDT
(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.
EWS Watchlist
Comment 25
2018-06-19 04:02:20 PDT
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
EWS Watchlist
Comment 26
2018-06-19 04:02:22 PDT
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
Ali Juma
Comment 27
2018-06-19 13:28:32 PDT
Created
attachment 343090
[details]
Patch
Ali Juma
Comment 28
2018-06-19 13:29:51 PDT
(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.
Ali Juma
Comment 29
2018-07-04 11:42:19 PDT
Created
attachment 344296
[details]
patch Rebased
Ali Juma
Comment 30
2018-07-04 12:04:09 PDT
Created
attachment 344297
[details]
Patch Rebased
Radar WebKit Bug Importer
Comment 31
2020-01-06 21:25:03 PST
<
rdar://problem/58363818
>
Brent Fulgham
Comment 32
2020-01-14 12:31:12 PST
This was shipped in WebKit -- I'm not sure why this bug wasn't updated to reflect that.
Alex Christensen
Comment 33
2021-11-01 12:13:45 PDT
Comment on
attachment 344297
[details]
Patch This was done elsewhere.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug