Attachment 357544 Details for Bug 192675 – Patch

[patch] Patch

bug-192675-20181218000250.patch (text/plain), 13.95 KB, created by Alex Christensen on 2018-12-18 00:02:51 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Alex Christensen

Created: 2018-12-18 00:02:51 PST

Size: 13.95 KB

patch

obsolete

>Index: Source/WebKit/ChangeLog
>===================================================================
>--- Source/WebKit/ChangeLog	(revision 239303)
>+++ Source/WebKit/ChangeLog	(working copy)
>@@ -1,3 +1,24 @@
>+2018-12-17  Alex Christensen  <achristensen@webkit.org>
>+
>+        WKWebView has old URL while displaying SafeBrowsing interstitial, for link-click navigations
>+        https://bugs.webkit.org/show_bug.cgi?id=192675
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        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-17  Chris Dumez  <cdumez@apple.com>
> 
>         Unreviewed, revert recent CrashReporterClient build fixes as they are no longer needed.
>Index: Source/WebKit/UIProcess/SafeBrowsingWarning.h
>===================================================================
>--- Source/WebKit/UIProcess/SafeBrowsingWarning.h	(revision 239222)
>+++ Source/WebKit/UIProcess/SafeBrowsingWarning.h	(working copy)
>@@ -45,12 +45,13 @@ public:
>     }
> #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 @@ private:
>     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)
>Index: Source/WebKit/UIProcess/WebPageProxy.cpp
>===================================================================
>--- Source/WebKit/UIProcess/WebPageProxy.cpp	(revision 239222)
>+++ Source/WebKit/UIProcess/WebPageProxy.cpp	(working copy)
>@@ -4335,6 +4335,12 @@ void WebPageProxy::decidePolicyForNaviga
>         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);
>Index: Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>===================================================================
>--- Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm	(revision 239222)
>+++ Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm	(working copy)
>@@ -4768,7 +4768,13 @@ + (BOOL)_handlesSafeBrowsing
> 
> - (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) {
>Index: Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h
>===================================================================
>--- Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h	(revision 239222)
>+++ Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h	(working copy)
>@@ -194,6 +194,7 @@ typedef NS_OPTIONS(NSUInteger, _WKRectEd
> + (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));
>Index: Source/WebKit/UIProcess/Cocoa/SafeBrowsingWarningCocoa.mm
>===================================================================
>--- Source/WebKit/UIProcess/Cocoa/SafeBrowsingWarningCocoa.mm	(revision 239222)
>+++ Source/WebKit/UIProcess/Cocoa/SafeBrowsingWarningCocoa.mm	(working copy)
>@@ -150,15 +150,17 @@ static NSMutableAttributedString *safeBr
> }
> 
> 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))
> {
> }
> #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))
> {
>Index: Tools/ChangeLog
>===================================================================
>--- Tools/ChangeLog	(revision 239325)
>+++ Tools/ChangeLog	(working copy)
>@@ -1,3 +1,17 @@
>+2018-12-17  Alex Christensen  <achristensen@webkit.org>
>+
>+        WKWebView has old URL while displaying SafeBrowsing interstitial, for link-click navigations
>+        https://bugs.webkit.org/show_bug.cgi?id=192675
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * TestWebKitAPI/Tests/WebKitCocoa/SafeBrowsing.mm:
>+        (goBack):
>+        (TEST):
>+        (visitUnsafeSite):
>+        (-[SafeBrowsingHelper observeValueForKeyPath:ofObject:change:context:]):
>+        (-[SafeBrowsingHelper webView:runJavaScriptAlertPanelWithMessage:initiatedByFrame:completionHandler:]):
>+
> 2018-12-17  Chris Dumez  <cdumez@apple.com>
> 
>         Allow passing nil as session state to [WKWebView _restoreSessionState:]
>Index: Tools/TestWebKitAPI/Tests/WebKitCocoa/SafeBrowsing.mm
>===================================================================
>--- Tools/TestWebKitAPI/Tests/WebKitCocoa/SafeBrowsing.mm	(revision 239222)
>+++ Tools/TestWebKitAPI/Tests/WebKitCocoa/SafeBrowsing.mm	(working copy)
>@@ -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 @@ static void checkTitleAndClick(UIButton 
> }
> #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 @@ TEST(SafeBrowsing, VisitUnsafeWebsite)
>     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 @@ TEST(SafeBrowsing, ShowWarningSPI)
>     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 @@ TEST(SafeBrowsing, ShowWarningSPI)
>     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

Flags:

ggaren: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 192675: 357544