Title: [244089] trunk
Revision
244089
Author
commit-qu...@webkit.org
Date
2019-04-09 12:07:55 -0700 (Tue, 09 Apr 2019)

Log 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 <achristen...@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.

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (244088 => 244089)


--- trunk/Source/WebKit/ChangeLog	2019-04-09 18:50:00 UTC (rev 244088)
+++ trunk/Source/WebKit/ChangeLog	2019-04-09 19:07:55 UTC (rev 244089)
@@ -1,3 +1,18 @@
+2019-04-09  Alex Christensen  <achristen...@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  <wilan...@apple.com>
 
         Pick up Ad Click Attribution conversions in NetworkResourceLoader::willSendRedirectedRequest()

Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm (244088 => 244089)


--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm	2019-04-09 18:50:00 UTC (rev 244088)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm	2019-04-09 19:07:55 UTC (rev 244089)
@@ -1355,12 +1355,19 @@
         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->_page->goBack();
+            return;
+        }
         [std::exchange(strongSelf->_safeBrowsingWarning, nullptr) removeFromSuperview];
     }]);
     [self addSubview:_safeBrowsingWarning.get()];

Modified: trunk/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm (244088 => 244089)


--- trunk/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm	2019-04-09 18:50:00 UTC (rev 244088)
+++ trunk/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm	2019-04-09 19:07:55 UTC (rev 244089)
@@ -1587,12 +1587,19 @@
         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()];

Modified: trunk/Tools/ChangeLog (244088 => 244089)


--- trunk/Tools/ChangeLog	2019-04-09 18:50:00 UTC (rev 244088)
+++ trunk/Tools/ChangeLog	2019-04-09 19:07:55 UTC (rev 244089)
@@ -1,3 +1,19 @@
+2019-04-09  Alex Christensen  <achristen...@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  <wilan...@apple.com>
 
         Pick up Ad Click Attribution conversions in NetworkResourceLoader::willSendRedirectedRequest()

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/SafeBrowsing.mm (244088 => 244089)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/SafeBrowsing.mm	2019-04-09 18:50:00 UTC (rev 244088)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/SafeBrowsing.mm	2019-04-09 19:07:55 UTC (rev 244089)
@@ -220,12 +220,15 @@
 }
 #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,14 +371,16 @@
     }
 }
 
-@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;
 }
 
@@ -382,7 +387,7 @@
 - (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 @@
 
 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,30 @@
     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();
+#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
 @implementation NullLookupContext
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to