Diff
Modified: trunk/Source/WebKit/ChangeLog (239338 => 239339)
--- trunk/Source/WebKit/ChangeLog 2018-12-18 17:11:50 UTC (rev 239338)
+++ trunk/Source/WebKit/ChangeLog 2018-12-18 18:26:33 UTC (rev 239339)
@@ -1,3 +1,24 @@
+2018-12-18 Alex Christensen <[email protected]>
+
+ WKWebView has old URL while displaying SafeBrowsing interstitial, for link-click navigations
+ https://bugs.webkit.org/show_bug.cgi?id=192675
+
+ Reviewed by Geoffrey Garen.
+
+ When a safe browsing warning is being shown, WKWebView.URL should be the unsafe website, not the safe website before it.
+
+ * UIProcess/API/Cocoa/WKWebView.mm:
+ (-[WKWebView _showSafeBrowsingWarningWithTitle:warning:details:completionHandler:]):
+ (-[WKWebView _showSafeBrowsingWarningWithURL:title:warning:details:completionHandler:]):
+ * UIProcess/API/Cocoa/WKWebViewPrivate.h:
+ * UIProcess/Cocoa/SafeBrowsingWarningCocoa.mm:
+ (WebKit::SafeBrowsingWarning::SafeBrowsingWarning):
+ * UIProcess/SafeBrowsingWarning.h:
+ (WebKit::SafeBrowsingWarning::create):
+ (WebKit::SafeBrowsingWarning::url const):
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::decidePolicyForNavigationAction):
+
2018-12-18 Chris Dumez <[email protected]>
Regression(r239182) SuspendedPage's process reuse for link navigation optimization sometimes broken
Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm (239338 => 239339)
--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm 2018-12-18 17:11:50 UTC (rev 239338)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm 2018-12-18 18:26:33 UTC (rev 239339)
@@ -4800,7 +4800,13 @@
- (void)_showSafeBrowsingWarningWithTitle:(NSString *)title warning:(NSString *)warning details:(NSAttributedString *)details completionHandler:(void(^)(BOOL))completionHandler
{
- auto safeBrowsingWarning = WebKit::SafeBrowsingWarning::create(title, warning, details);
+ // FIXME: Adopt _showSafeBrowsingWarningWithURL and remove this function.
+ [self _showSafeBrowsingWarningWithURL:nil title:title warning:warning details:details completionHandler:completionHandler];
+}
+
+- (void)_showSafeBrowsingWarningWithURL:(NSURL *)url title:(NSString *)title warning:(NSString *)warning details:(NSAttributedString *)details completionHandler:(void(^)(BOOL))completionHandler
+{
+ auto safeBrowsingWarning = WebKit::SafeBrowsingWarning::create(url, title, warning, details);
auto wrapper = [completionHandler = makeBlockPtr(completionHandler)] (Variant<WebKit::ContinueUnsafeLoad, URL>&& variant) {
switchOn(variant, [&] (WebKit::ContinueUnsafeLoad continueUnsafeLoad) {
switch (continueUnsafeLoad) {
Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h (239338 => 239339)
--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h 2018-12-18 17:11:50 UTC (rev 239338)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h 2018-12-18 18:26:33 UTC (rev 239339)
@@ -194,6 +194,7 @@
+ (NSURL *)_confirmMalwareSentinel WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));
+ (NSURL *)_visitUnsafeWebsiteSentinel WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));
- (void)_showSafeBrowsingWarningWithTitle:(NSString *)title warning:(NSString *)warning details:(NSAttributedString *)details completionHandler:(void(^)(BOOL))completionHandler WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));
+- (void)_showSafeBrowsingWarningWithURL:(NSURL *)url title:(NSString *)title warning:(NSString *)warning details:(NSAttributedString *)details completionHandler:(void(^)(BOOL))completionHandler WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));
- (void)_isJITEnabled:(void(^)(BOOL))completionHandler WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));
- (void)_removeDataDetectedLinks:(dispatch_block_t)completion WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));
Modified: trunk/Source/WebKit/UIProcess/Cocoa/SafeBrowsingWarningCocoa.mm (239338 => 239339)
--- trunk/Source/WebKit/UIProcess/Cocoa/SafeBrowsingWarningCocoa.mm 2018-12-18 17:11:50 UTC (rev 239338)
+++ trunk/Source/WebKit/UIProcess/Cocoa/SafeBrowsingWarningCocoa.mm 2018-12-18 18:26:33 UTC (rev 239339)
@@ -150,7 +150,8 @@
}
SafeBrowsingWarning::SafeBrowsingWarning(const URL& url, SSBServiceLookupResult *result)
- : m_title(safeBrowsingTitleText(result))
+ : m_url(url)
+ , m_title(safeBrowsingTitleText(result))
, m_warning(safeBrowsingWarningText(result))
, m_details(safeBrowsingDetailsText(url, result))
{
@@ -157,8 +158,9 @@
}
#endif
-SafeBrowsingWarning::SafeBrowsingWarning(String&& title, String&& warning, RetainPtr<NSAttributedString>&& details)
- : m_title(WTFMove(title))
+SafeBrowsingWarning::SafeBrowsingWarning(URL&& url, String&& title, String&& warning, RetainPtr<NSAttributedString>&& details)
+ : m_url(WTFMove(url))
+ , m_title(WTFMove(title))
, m_warning(WTFMove(warning))
, m_details(WTFMove(details))
{
Modified: trunk/Source/WebKit/UIProcess/SafeBrowsingWarning.h (239338 => 239339)
--- trunk/Source/WebKit/UIProcess/SafeBrowsingWarning.h 2018-12-18 17:11:50 UTC (rev 239338)
+++ trunk/Source/WebKit/UIProcess/SafeBrowsingWarning.h 2018-12-18 18:26:33 UTC (rev 239339)
@@ -45,12 +45,13 @@
}
#endif
#if PLATFORM(COCOA)
- static Ref<SafeBrowsingWarning> create(String&& title, String&& warning, RetainPtr<NSAttributedString>&& details)
+ static Ref<SafeBrowsingWarning> create(URL&& url, String&& title, String&& warning, RetainPtr<NSAttributedString>&& details)
{
- return adoptRef(*new SafeBrowsingWarning(WTFMove(title), WTFMove(warning), WTFMove(details)));
+ return adoptRef(*new SafeBrowsingWarning(WTFMove(url), WTFMove(title), WTFMove(warning), WTFMove(details)));
}
#endif
+ const URL& url() const { return m_url; }
const String& title() const { return m_title; }
const String& warning() const { return m_warning; }
#if PLATFORM(COCOA)
@@ -65,9 +66,10 @@
SafeBrowsingWarning(const URL&, SSBServiceLookupResult *);
#endif
#if PLATFORM(COCOA)
- SafeBrowsingWarning(String&&, String&&, RetainPtr<NSAttributedString>&&);
+ SafeBrowsingWarning(URL&&, String&&, String&&, RetainPtr<NSAttributedString>&&);
#endif
+ URL m_url;
String m_title;
String m_warning;
#if PLATFORM(COCOA)
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (239338 => 239339)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2018-12-18 17:11:50 UTC (rev 239338)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2018-12-18 18:26:33 UTC (rev 239339)
@@ -4339,6 +4339,12 @@
m_pageClient->clearSafeBrowsingWarning();
if (safeBrowsingWarning) {
+ if (frame->isMainFrame() && safeBrowsingWarning->url().isValid()) {
+ auto transaction = m_pageLoadState.transaction();
+ m_pageLoadState.setPendingAPIRequestURL(transaction, safeBrowsingWarning->url());
+ m_pageLoadState.commitChanges();
+ }
+
m_pageClient->showSafeBrowsingWarning(*safeBrowsingWarning, [protectedThis = WTFMove(protectedThis), completionHandler = WTFMove(completionHandler), policyAction] (auto&& result) mutable {
switchOn(result, [&] (const URL& url) {
completionHandler(WebPolicyAction::Ignore);
Modified: trunk/Tools/ChangeLog (239338 => 239339)
--- trunk/Tools/ChangeLog 2018-12-18 17:11:50 UTC (rev 239338)
+++ trunk/Tools/ChangeLog 2018-12-18 18:26:33 UTC (rev 239339)
@@ -1,3 +1,17 @@
+2018-12-18 Alex Christensen <[email protected]>
+
+ WKWebView has old URL while displaying SafeBrowsing interstitial, for link-click navigations
+ https://bugs.webkit.org/show_bug.cgi?id=192675
+
+ Reviewed by Geoffrey Garen.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/SafeBrowsing.mm:
+ (goBack):
+ (TEST):
+ (visitUnsafeSite):
+ (-[SafeBrowsingHelper observeValueForKeyPath:ofObject:change:context:]):
+ (-[SafeBrowsingHelper webView:runJavaScriptAlertPanelWithMessage:initiatedByFrame:completionHandler:]):
+
2018-12-18 Chris Dumez <[email protected]>
Regression(r239182) SuspendedPage's process reuse for link navigation optimization sometimes broken
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/SafeBrowsing.mm (239338 => 239339)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/SafeBrowsing.mm 2018-12-18 17:11:50 UTC (rev 239338)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/SafeBrowsing.mm 2018-12-18 18:26:33 UTC (rev 239339)
@@ -35,6 +35,8 @@
#import <WebKit/WKUIDelegatePrivate.h>
#import <WebKit/WKWebViewPrivate.h>
#import <wtf/RetainPtr.h>
+#import <wtf/URL.h>
+#import <wtf/Vector.h>
static bool committedNavigation;
static bool warningShown;
@@ -208,15 +210,25 @@
}
#endif
-TEST(SafeBrowsing, GoBack)
+template<typename ViewType> void goBack(ViewType *view)
{
- auto webView = safeBrowsingView();
- auto warning = [webView _safeBrowsingWarning];
- auto box = warning.subviews.firstObject;
+ WKWebView *webView = (WKWebView *)view.superview;
+ auto box = view.subviews.firstObject;
checkTitleAndClick(box.subviews[3], "Go Back");
EXPECT_EQ([webView _safeBrowsingWarning], nil);
}
+TEST(SafeBrowsing, GoBack)
+{
+ auto webView = safeBrowsingView();
+ goBack([webView _safeBrowsingWarning]);
+}
+
+template<typename ViewType> void visitUnsafeSite(ViewType *view)
+{
+ [view performSelector:NSSelectorFromString(@"clickedOnLink:") withObject:[NSURL URLWithString:@"WKVisitUnsafeWebsiteSentinel"]];
+}
+
TEST(SafeBrowsing, VisitUnsafeWebsite)
{
auto webView = safeBrowsingView();
@@ -228,7 +240,7 @@
checkTitleAndClick(warning.subviews.firstObject.subviews[4], "Show Details");
EXPECT_EQ(warning.subviews.count, 2ull);
EXPECT_FALSE(committedNavigation);
- [warning performSelector:NSSelectorFromString(@"clickedOnLink:") withObject:[NSURL URLWithString:@"WKVisitUnsafeWebsiteSentinel"]];
+ visitUnsafeSite(warning);
TestWebKitAPI::Util::run(&committedNavigation);
}
@@ -248,7 +260,7 @@
auto webView = adoptNS([WKWebView new]);
auto showWarning = ^{
completionHandlerCalled = false;
- [webView _showSafeBrowsingWarningWithTitle:@"test title" warning:@"test warning" details:[[[NSAttributedString alloc] initWithString:@"test details"] autorelease] completionHandler:^(BOOL shouldContinue) {
+ [webView _showSafeBrowsingWarningWithURL:nil title:@"test title" warning:@"test warning" details:[[[NSAttributedString alloc] initWithString:@"test details"] autorelease] completionHandler:^(BOOL shouldContinue) {
shouldContinueValue = shouldContinue;
completionHandlerCalled = true;
}];
@@ -268,6 +280,83 @@
EXPECT_TRUE(shouldContinueValue);
}
+static Vector<URL> urls;
+static bool done;
+
+@interface SafeBrowsingHelper : NSObject<WKNavigationDelegate, WKUIDelegate>
+@end
+
+@implementation SafeBrowsingHelper
+
+- (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(NSDictionary<NSString *, id> *)change context:(void *)context
+{
+ urls.append((NSURL *)[change objectForKey:NSKeyValueChangeNewKey]);
+}
+
+- (void)webView:(WKWebView *)webView runJavaScriptAlertPanelWithMessage:(NSString *)message initiatedByFrame:(WKFrameInfo *)frame completionHandler:(void (^)(void))completionHandler
+{
+ done = true;
+ completionHandler();
+}
+
+@end
+
+TEST(SafeBrowsing, URLObservation)
+{
+ ClassMethodSwizzler swizzler(objc_getClass("SSBLookupContext"), @selector(sharedLookupContext), [TestLookupContext methodForSelector:@selector(sharedLookupContext)]);
+
+ RetainPtr<NSURL> simpleURL = [[NSBundle mainBundle] URLForResource:@"simple" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"];
+ RetainPtr<NSURL> simple2URL = [[NSBundle mainBundle] URLForResource:@"simple2" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"];
+ auto helper = adoptNS([SafeBrowsingHelper new]);
+
+ auto webViewWithWarning = [&] () -> RetainPtr<WKWebView> {
+ auto webView = adoptNS([WKWebView new]);
+ [webView setUIDelegate:helper.get()];
+ [webView setNavigationDelegate:helper.get()];
+ [webView addObserver:helper.get() forKeyPath:@"URL" options:NSKeyValueObservingOptionNew context:nil];
+
+ [webView loadHTMLString:@"<script>alert('loaded')</script>" baseURL:simpleURL.get()];
+ while (![webView _safeBrowsingWarning])
+ TestWebKitAPI::Util::spinRunLoop();
+ visitUnsafeSite([webView _safeBrowsingWarning]);
+ TestWebKitAPI::Util::run(&done);
+ EXPECT_FALSE(!![webView _safeBrowsingWarning]);
+
+ done = false;
+ [webView evaluateJavaScript:[NSString stringWithFormat:@"window.location='%@'", simple2URL.get()] completionHandler:nil];
+ while (![webView _safeBrowsingWarning])
+ TestWebKitAPI::Util::spinRunLoop();
+ return webView;
+ };
+
+ auto checkURLs = [&] (Vector<RetainPtr<NSURL>>&& expected) {
+ EXPECT_EQ(urls.size(), expected.size());
+ if (urls.size() != expected.size())
+ return;
+ for (size_t i = 0; i < expected.size(); ++i)
+ EXPECT_STREQ(urls[i].string().utf8().data(), [expected[i] absoluteString].UTF8String);
+ };
+
+ {
+ auto webView = webViewWithWarning();
+ checkURLs({ simpleURL, simple2URL });
+ goBack([webView _safeBrowsingWarning]);
+ checkURLs({ simpleURL, simple2URL, simpleURL });
+ [webView removeObserver:helper.get() forKeyPath:@"URL"];
+ }
+
+ urls.clear();
+
+ {
+ auto webView = webViewWithWarning();
+ checkURLs({ simpleURL, simple2URL });
+ visitUnsafeSite([webView _safeBrowsingWarning]);
+ TestWebKitAPI::Util::spinRunLoop(5);
+ checkURLs({ simpleURL, simple2URL });
+ [webView removeObserver:helper.get() forKeyPath:@"URL"];
+ }
+}
+
@interface NullLookupContext : NSObject
@end
@implementation NullLookupContext
