Clicking "Go Back" from a safe browsing warning from an iframe should navigate the...
authorachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 6 Apr 2019 00:10:15 +0000 (00:10 +0000)
committerachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 6 Apr 2019 00:10:15 +0000 (00:10 +0000)
https://bugs.webkit.org/show_bug.cgi?id=196665
<rdar://45115669>

Reviewed by Geoff Garen.

Source/WebKit:

It is insufficient to just not navigate the subframe.  We must leave the page that contained it.

* UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView _showSafeBrowsingWarning:completionHandler:]):
* UIProcess/Cocoa/WebViewImpl.mm:
(WebKit::WebViewImpl::showSafeBrowsingWarning):

Tools:

* TestWebKitAPI/Tests/WebKitCocoa/SafeBrowsing.mm:
(goBack):
(+[SimpleLookupContext sharedLookupContext]):
(-[SimpleLookupContext lookUpURL:completionHandler:]):
(TEST):
(+[Simple3LookupContext sharedLookupContext]): Deleted.
(-[Simple3LookupContext lookUpURL:completionHandler:]): Deleted.

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

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/SafeBrowsing.mm

index f858810..756328e 100644 (file)
@@ -1,5 +1,20 @@
 2019-04-05  Alex Christensen  <achristensen@webkit.org>
 
+        Clicking "Go Back" from a safe browsing warning from an iframe should navigate the WKWebView back to the previous page
+        https://bugs.webkit.org/show_bug.cgi?id=196665
+        <rdar://45115669>
+
+        Reviewed by Geoff Garen.
+
+        It is insufficient to just not navigate the subframe.  We must leave the page that contained it.
+
+        * UIProcess/API/Cocoa/WKWebView.mm:
+        (-[WKWebView _showSafeBrowsingWarning:completionHandler:]):
+        * UIProcess/Cocoa/WebViewImpl.mm:
+        (WebKit::WebViewImpl::showSafeBrowsingWarning):
+
+2019-04-05  Alex Christensen  <achristensen@webkit.org>
+
         Undeprecate WKNavigationData
         https://bugs.webkit.org/show_bug.cgi?id=196559
         <rdar://44927425>
index afb85cd..85c3273 100644 (file)
@@ -1356,12 +1356,19 @@ static _WKSelectionAttributes selectionAttributes(const WebKit::EditorState& edi
         auto strongSelf = weakSelf.get();
         if (!strongSelf)
             return;
-        bool navigatesMainFrame = WTF::switchOn(result,
+        bool navigatesFrame = WTF::switchOn(result,
             [] (WebKit::ContinueUnsafeLoad continueUnsafeLoad) { return continueUnsafeLoad == WebKit::ContinueUnsafeLoad::Yes; },
             [] (const URL&) { return true; }
         );
-        if (navigatesMainFrame && [strongSelf->_safeBrowsingWarning forMainFrameNavigation])
+        bool forMainFrameNavigation = [strongSelf->_safeBrowsingWarning forMainFrameNavigation];
+        if (navigatesFrame && forMainFrameNavigation) {
+            // The safe browsing warning will be hidden once the next page is shown.
             return;
+        }
+        if (!navigatesFrame && strongSelf->_safeBrowsingWarning && !forMainFrameNavigation) {
+            [strongSelf goBack];
+            return;
+        }
         [std::exchange(strongSelf->_safeBrowsingWarning, nullptr) removeFromSuperview];
     }]);
     [self addSubview:_safeBrowsingWarning.get()];
index 1bd83b7..4c9014a 100644 (file)
@@ -1587,12 +1587,19 @@ void WebViewImpl::showSafeBrowsingWarning(const SafeBrowsingWarning& warning, Co
         completionHandler(WTFMove(result));
         if (!weakThis)
             return;
-        bool navigatesMainFrame = WTF::switchOn(result,
+        bool navigatesFrame = WTF::switchOn(result,
             [] (ContinueUnsafeLoad continueUnsafeLoad) { return continueUnsafeLoad == ContinueUnsafeLoad::Yes; },
             [] (const URL&) { return true; }
         );
-        if (navigatesMainFrame && [weakThis->m_safeBrowsingWarning forMainFrameNavigation])
+        bool forMainFrameNavigation = [weakThis->m_safeBrowsingWarning forMainFrameNavigation];
+        if (navigatesFrame && forMainFrameNavigation) {
+            // The safe browsing warning will be hidden once the next page is shown.
             return;
+        }
+        if (!navigatesFrame && weakThis->m_safeBrowsingWarning && !forMainFrameNavigation) {
+            weakThis->m_page->goBack();
+            return;
+        }
         [std::exchange(weakThis->m_safeBrowsingWarning, nullptr) removeFromSuperview];
     }]);
     [m_view addSubview:m_safeBrowsingWarning.get()];
index 085da65..d9fd453 100644 (file)
@@ -1,3 +1,19 @@
+2019-04-05  Alex Christensen  <achristensen@webkit.org>
+
+        Clicking "Go Back" from a safe browsing warning from an iframe should navigate the WKWebView back to the previous page
+        https://bugs.webkit.org/show_bug.cgi?id=196665
+        <rdar://45115669>
+
+        Reviewed by Geoff Garen.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/SafeBrowsing.mm:
+        (goBack):
+        (+[SimpleLookupContext sharedLookupContext]):
+        (-[SimpleLookupContext lookUpURL:completionHandler:]):
+        (TEST):
+        (+[Simple3LookupContext sharedLookupContext]): Deleted.
+        (-[Simple3LookupContext lookUpURL:completionHandler:]): Deleted.
+
 2019-04-05  Aakash Jain  <aakash_jain@apple.com>
 
         [ews-build] Add configuration and architecture for windows builders
index a730d0d..ad9222e 100644 (file)
@@ -220,12 +220,15 @@ static void checkTitleAndClick(UIButton *button, const char* expectedTitle)
 }
 #endif
 
-template<typename ViewType> void goBack(ViewType *view)
+template<typename ViewType> void goBack(ViewType *view, bool mainFrame = true)
 {
     WKWebView *webView = (WKWebView *)view.superview;
     auto box = view.subviews.firstObject;
     checkTitleAndClick(box.subviews[3], "Go Back");
-    EXPECT_EQ([webView _safeBrowsingWarning], nil);
+    if (mainFrame)
+        EXPECT_EQ([webView _safeBrowsingWarning], nil);
+    else
+        EXPECT_NE([webView _safeBrowsingWarning], nil);
 }
 
 TEST(SafeBrowsing, GoBack)
@@ -368,21 +371,23 @@ TEST(SafeBrowsing, URLObservation)
     }
 }
 
-@interface Simple3LookupContext : NSObject
+static RetainPtr<NSString> phishingResourceName;
+
+@interface SimpleLookupContext : NSObject
 @end
 
-@implementation Simple3LookupContext
+@implementation SimpleLookupContext
 
-+ (Simple3LookupContext *)sharedLookupContext
++ (SimpleLookupContext *)sharedLookupContext
 {
-    static Simple3LookupContext *context = [[Simple3LookupContext alloc] init];
+    static SimpleLookupContext *context = [[SimpleLookupContext alloc] init];
     return context;
 }
 
 - (void)lookUpURL:(NSURL *)URL completionHandler:(void (^)(TestLookupResult *, NSError *))completionHandler
 {
     BOOL phishing = NO;
-    if ([URL isEqual:resourceURL(@"simple3")])
+    if ([URL isEqual:resourceURL(phishingResourceName.get())])
         phishing = YES;
     completionHandler([TestLookupResult resultWithResults:@[[TestServiceLookupResult resultWithProvider:@"TestProvider" phishing:phishing malware:NO unwantedSoftware:NO]]], nil);
 }
@@ -405,7 +410,8 @@ static bool navigationFinished;
 
 TEST(SafeBrowsing, WKWebViewGoBack)
 {
-    ClassMethodSwizzler swizzler(objc_getClass("SSBLookupContext"), @selector(sharedLookupContext), [Simple3LookupContext methodForSelector:@selector(sharedLookupContext)]);
+    phishingResourceName = @"simple3";
+    ClassMethodSwizzler swizzler(objc_getClass("SSBLookupContext"), @selector(sharedLookupContext), [SimpleLookupContext methodForSelector:@selector(sharedLookupContext)]);
     
     auto delegate = adoptNS([WKWebViewGoBackNavigationDelegate new]);
     auto webView = adoptNS([WKWebView new]);
@@ -427,6 +433,27 @@ TEST(SafeBrowsing, WKWebViewGoBack)
     EXPECT_TRUE([[webView URL] isEqual:resourceURL(@"simple2")]);
 }
 
+TEST(SafeBrowsing, WKWebViewGoBackIFrame)
+{
+    phishingResourceName = @"simple";
+    ClassMethodSwizzler swizzler(objc_getClass("SSBLookupContext"), @selector(sharedLookupContext), [SimpleLookupContext methodForSelector:@selector(sharedLookupContext)]);
+    
+    auto delegate = adoptNS([WKWebViewGoBackNavigationDelegate new]);
+    auto webView = adoptNS([WKWebView new]);
+    [webView configuration].preferences._safeBrowsingEnabled = YES;
+    [webView setNavigationDelegate:delegate.get()];
+    [webView loadRequest:[NSURLRequest requestWithURL:resourceURL(@"simple2")]];
+    TestWebKitAPI::Util::run(&navigationFinished);
+    
+    [webView loadRequest:[NSURLRequest requestWithURL:resourceURL(@"simple-iframe")]];
+    while (![webView _safeBrowsingWarning])
+        TestWebKitAPI::Util::spinRunLoop();
+    navigationFinished = false;
+    goBack([webView _safeBrowsingWarning], false);
+    TestWebKitAPI::Util::run(&navigationFinished);
+    EXPECT_TRUE([[webView URL] isEqual:resourceURL(@"simple2")]);
+}
+
 @interface NullLookupContext : NSObject
 @end
 @implementation NullLookupContext