Changeset 244089 in webkit

Timestamp:

Apr 9, 2019, 12:07:55 PM (7 years ago)

Author:

commit-queue@webkit.org

Message:

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>

Patch by Alex Christensen <​achristensen@webkit.org> on 2019-04-09
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.

Location:

trunk

Files:

• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm (modified) (1 diff)
• Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKitCocoa/SafeBrowsing.mm (modified) (5 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2019-04-09  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-09  John Wilander  <wilander@apple.com>
 

trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm

@@ -1356 +1356 @@
         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->_page->goBack();
+            return;
+        }
         [std::exchange(strongSelf->_safeBrowsingWarning, nullptr) removeFromSuperview];
     }]);

trunk/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm

@@ -1588 +1588 @@
         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];
     }]);

trunk/Tools/ChangeLog

@@ +1 @@
+2019-04-09  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-09  John Wilander  <wilander@apple.com>
 

trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/SafeBrowsing.mm

@@ -221 +221 @@
 #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);
 }
 
@@ -369 +372 @@
 }
 
-@interface Simple3LookupContext : NSObject
-@end
-
-@implementation Simple3LookupContext
-
-+ (Simple3LookupContext *)sharedLookupContext
-{
-    static Simple3LookupContext *context = [[Simple3LookupContext alloc] init];
+static RetainPtr<NSString> phishingResourceName;
+
+@interface SimpleLookupContext : NSObject
+@end
+
+@implementation SimpleLookupContext
+
++ (SimpleLookupContext *)sharedLookupContext
+{
+    static SimpleLookupContext *context = [[SimpleLookupContext alloc] init];
     return context;
 }
@@ -383 +388 @@
 {
     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);
@@ -406 +411 @@
 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]);
@@ -428 +434 @@
 }
 
+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();
+#if !PLATFORM(MAC)
+    [[webView _safeBrowsingWarning] didMoveToWindow];
+#endif
+    navigationFinished = false;
+    goBack([webView _safeBrowsingWarning], false);
+    TestWebKitAPI::Util::run(&navigationFinished);
+    EXPECT_TRUE([[webView URL] isEqual:resourceURL(@"simple2")]);
+}
+
 @interface NullLookupContext : NSObject
 @end