WKWebView.goBack should reload if there is a safe browsing warning
authorachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 25 Jan 2019 19:42:27 +0000 (19:42 +0000)
committerachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 25 Jan 2019 19:42:27 +0000 (19:42 +0000)
https://bugs.webkit.org/show_bug.cgi?id=193805
<rdar://problem/46908216>

Reviewed by Geoff Garen.

Source/WebKit:

If a WKWebView is showing a safe browsing warning and the user clicks a back button
in the app which calls WKWebView.goBack, the WKWebView is in a state where it has not navigated yet,
so actually going back will appear to the user to go back twice.  We can't just do nothing because the
app is in a state where it is expecting a navigation to happen.  Reloading achieves what the user expects
and makes the app work like the app expects.

* UIProcess/API/C/WKPage.cpp:
(WKPageGoBack):
* UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView goBack]):
* UIProcess/PageClient.h:
(WebKit::PageClient::hasSafeBrowsingWarning const):
* UIProcess/mac/PageClientImplMac.h:
* UIProcess/mac/PageClientImplMac.mm:
(WebKit::PageClientImpl::hasSafeBrowsingWarning const):

Tools:

* TestWebKitAPI/Tests/WebKitCocoa/SafeBrowsing.mm:
(+[Simple3LookupContext sharedLookupContext]):
(-[Simple3LookupContext lookUpURL:completionHandler:]):
(-[WKWebViewGoBackNavigationDelegate webView:didFinishNavigation:]):
(TEST):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@240490 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/API/C/WKPage.cpp
Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
Source/WebKit/UIProcess/PageClient.h
Source/WebKit/UIProcess/mac/PageClientImplMac.h
Source/WebKit/UIProcess/mac/PageClientImplMac.mm
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/SafeBrowsing.mm

index 4844081..bc5bb28 100644 (file)
@@ -1,3 +1,27 @@
+2019-01-25  Alex Christensen  <achristensen@webkit.org>
+
+        WKWebView.goBack should reload if there is a safe browsing warning
+        https://bugs.webkit.org/show_bug.cgi?id=193805
+        <rdar://problem/46908216>
+
+        Reviewed by Geoff Garen.
+
+        If a WKWebView is showing a safe browsing warning and the user clicks a back button
+        in the app which calls WKWebView.goBack, the WKWebView is in a state where it has not navigated yet,
+        so actually going back will appear to the user to go back twice.  We can't just do nothing because the
+        app is in a state where it is expecting a navigation to happen.  Reloading achieves what the user expects
+        and makes the app work like the app expects.
+
+        * UIProcess/API/C/WKPage.cpp:
+        (WKPageGoBack):
+        * UIProcess/API/Cocoa/WKWebView.mm:
+        (-[WKWebView goBack]):
+        * UIProcess/PageClient.h:
+        (WebKit::PageClient::hasSafeBrowsingWarning const):
+        * UIProcess/mac/PageClientImplMac.h:
+        * UIProcess/mac/PageClientImplMac.mm:
+        (WebKit::PageClientImpl::hasSafeBrowsingWarning const):
+
 2019-01-25  Chris Dumez  <cdumez@apple.com>
 
         Regression(PSON) cross-site provisional page is not canceled if a new same-site one is started
index b5fc2fa..35ff078 100644 (file)
@@ -57,6 +57,7 @@
 #include "NativeWebKeyboardEvent.h"
 #include "NativeWebWheelEvent.h"
 #include "NavigationActionData.h"
+#include "PageClient.h"
 #include "PluginInformation.h"
 #include "PrintInfo.h"
 #include "WKAPICast.h"
@@ -320,7 +321,12 @@ bool WKPageCanGoForward(WKPageRef pageRef)
 
 void WKPageGoBack(WKPageRef pageRef)
 {
-    toImpl(pageRef)->goBack();
+    auto& page = *toImpl(pageRef);
+    if (page.pageClient().hasSafeBrowsingWarning()) {
+        WKPageReload(pageRef);
+        return;
+    }
+    page.goBack();
 }
 
 bool WKPageCanGoBack(WKPageRef pageRef)
index 2a1bb53..0a30480 100644 (file)
@@ -1008,6 +1008,8 @@ static void validate(WKWebViewConfiguration *configuration)
 
 - (WKNavigation *)goBack
 {
+    if (self._safeBrowsingWarning)
+        return [self reload];
     return wrapper(_page->goBack());
 }
 
index 0795cd8..2b71abe 100644 (file)
@@ -424,6 +424,8 @@ public:
     virtual void pinnedStateWillChange() { }
     virtual void pinnedStateDidChange() { }
 
+    virtual bool hasSafeBrowsingWarning() const { return false; }
+    
 #if PLATFORM(MAC)
     virtual void didPerformImmediateActionHitTest(const WebHitTestResultData&, bool contentPreventsDefault, API::Object*) = 0;
     virtual NSObject *immediateActionAnimationControllerForHitTestResult(RefPtr<API::HitTestResult>, uint64_t, RefPtr<API::Object>) = 0;
index 08ffea4..4707a02 100644 (file)
@@ -107,6 +107,7 @@ private:
     void showSafeBrowsingWarning(const SafeBrowsingWarning&, CompletionHandler<void(Variant<WebKit::ContinueUnsafeLoad, URL>&&)>&&) override;
     void clearSafeBrowsingWarning() override;
     void clearSafeBrowsingWarningIfForMainFrameNavigation() override;
+    bool hasSafeBrowsingWarning() const override;
     
 #if WK_API_ENABLED
     bool showShareSheet(const WebCore::ShareDataWithParsedURL&, WTF::CompletionHandler<void(bool)>&&) override;
index b48ac3f..97b1435 100644 (file)
@@ -496,6 +496,13 @@ void PageClientImpl::showSafeBrowsingWarning(const SafeBrowsingWarning& warning,
     m_impl->showSafeBrowsingWarning(warning, WTFMove(completionHandler));
 }
 
+bool PageClientImpl::hasSafeBrowsingWarning() const
+{
+    if (!m_impl)
+        return false;
+    return !!m_impl->safeBrowsingWarning();
+}
+
 void PageClientImpl::clearSafeBrowsingWarning()
 {
     m_impl->clearSafeBrowsingWarning();
index a5446fe..f3ae32e 100644 (file)
@@ -1,3 +1,17 @@
+2019-01-25  Alex Christensen  <achristensen@webkit.org>
+
+        WKWebView.goBack should reload if there is a safe browsing warning
+        https://bugs.webkit.org/show_bug.cgi?id=193805
+        <rdar://problem/46908216>
+
+        Reviewed by Geoff Garen.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/SafeBrowsing.mm:
+        (+[Simple3LookupContext sharedLookupContext]):
+        (-[Simple3LookupContext lookUpURL:completionHandler:]):
+        (-[WKWebViewGoBackNavigationDelegate webView:didFinishNavigation:]):
+        (TEST):
+
 2019-01-25  Chris Dumez  <cdumez@apple.com>
 
         Regression(PSON) cross-site provisional page is not canceled if a new same-site one is started
index 9a46bc5..438757e 100644 (file)
@@ -355,6 +355,64 @@ TEST(SafeBrowsing, URLObservation)
     }
 }
 
+@interface Simple3LookupContext : NSObject
+@end
+
+@implementation Simple3LookupContext
+
++ (Simple3LookupContext *)sharedLookupContext
+{
+    static Simple3LookupContext *context = [[Simple3LookupContext alloc] init];
+    return context;
+}
+
+- (void)lookUpURL:(NSURL *)URL completionHandler:(void (^)(TestLookupResult *, NSError *))completionHandler
+{
+    BOOL phishing = NO;
+    if ([URL isEqual:resourceURL(@"simple3")])
+        phishing = YES;
+    completionHandler([TestLookupResult resultWithResults:@[[TestServiceLookupResult resultWithProvider:@"TestProvider" phishing:phishing malware:NO unwantedSoftware:NO]]], nil);
+}
+
+@end
+
+static bool navigationFinished;
+
+@interface WKWebViewGoBackNavigationDelegate : NSObject <WKNavigationDelegate>
+@end
+
+@implementation WKWebViewGoBackNavigationDelegate
+
+- (void)webView:(WKWebView *)webView didFinishNavigation:(null_unspecified WKNavigation *)navigation
+{
+    navigationFinished = true;
+}
+
+@end
+
+TEST(SafeBrowsing, WKWebViewGoBack)
+{
+    ClassMethodSwizzler swizzler(objc_getClass("SSBLookupContext"), @selector(sharedLookupContext), [Simple3LookupContext methodForSelector:@selector(sharedLookupContext)]);
+    
+    auto delegate = adoptNS([WKWebViewGoBackNavigationDelegate new]);
+    auto webView = adoptNS([WKWebView new]);
+    [webView setNavigationDelegate:delegate.get()];
+    [webView loadRequest:[NSURLRequest requestWithURL:resourceURL(@"simple")]];
+    TestWebKitAPI::Util::run(&navigationFinished);
+
+    navigationFinished = false;
+    [webView loadRequest:[NSURLRequest requestWithURL:resourceURL(@"simple2")]];
+    TestWebKitAPI::Util::run(&navigationFinished);
+
+    navigationFinished = false;
+    [webView loadRequest:[NSURLRequest requestWithURL:resourceURL(@"simple3")]];
+    while (![webView _safeBrowsingWarning])
+        TestWebKitAPI::Util::spinRunLoop();
+    [webView goBack];
+    TestWebKitAPI::Util::run(&navigationFinished);
+    EXPECT_TRUE([[webView URL] isEqual:resourceURL(@"simple2")]);
+}
+
 @interface NullLookupContext : NSObject
 @end
 @implementation NullLookupContext