Navigations away from the SafeBrowsing interstitial show a flash of old content
authorachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 20 Dec 2018 00:55:10 +0000 (00:55 +0000)
committerachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 20 Dec 2018 00:55:10 +0000 (00:55 +0000)
https://bugs.webkit.org/show_bug.cgi?id=192676

Reviewed by Chris Dumez.

Source/WebKit:

When a user clicks through a safe browsing warning, do not remove the warning until content is drawn for the destination.
Otherwise, the user will confusingly see the page before the warning while the navigation happens.
We can only do this for warnings caused by main frame navigations, though.  Other warnings (such as those caused by iframes)
need to be cleared immediately, and we still need to clear the warning immediately if the user has said to go back.

This change is reflected in an updated API test.

* UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView _showSafeBrowsingWarning:completionHandler:]):
* UIProcess/Cocoa/SafeBrowsingWarningCocoa.mm:
(WebKit::SafeBrowsingWarning::SafeBrowsingWarning):
* UIProcess/Cocoa/WKSafeBrowsingWarning.h:
* UIProcess/Cocoa/WKSafeBrowsingWarning.mm:
(-[WKSafeBrowsingWarning forMainFrameNavigation]):
* UIProcess/Cocoa/WebPageProxyCocoa.mm:
(WebKit::WebPageProxy::beginSafeBrowsingCheck):
* UIProcess/Cocoa/WebViewImpl.h:
* UIProcess/Cocoa/WebViewImpl.mm:
(WebKit::WebViewImpl::showSafeBrowsingWarning):
(WebKit::WebViewImpl::clearSafeBrowsingWarningIfForMainFrameNavigation):
* UIProcess/PageClient.h:
(WebKit::PageClient::clearSafeBrowsingWarningIfForMainFrameNavigation):
* UIProcess/SafeBrowsingWarning.h:
(WebKit::SafeBrowsingWarning::create):
(WebKit::SafeBrowsingWarning::forMainFrameNavigation const):
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::didReachLayoutMilestone):
(WebKit::WebPageProxy::beginSafeBrowsingCheck):
(WebKit::WebPageProxy::decidePolicyForNavigationAction):
* UIProcess/WebPageProxy.h:
* UIProcess/ios/PageClientImplIOS.h:
* UIProcess/mac/PageClientImplMac.h:
* UIProcess/mac/PageClientImplMac.mm:
(WebKit::PageClientImpl::clearSafeBrowsingWarningIfForMainFrameNavigation):

Tools:

* TestWebKitAPI/Tests/WebKitCocoa/SafeBrowsing.mm:
(safeBrowsingView):
(TEST):
(-[SafeBrowsingHelper webView:runJavaScriptAlertPanelWithMessage:initiatedByFrame:completionHandler:]): Deleted.

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

19 files changed:
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
Source/WebKit/UIProcess/API/Cocoa/WKWebViewInternal.h
Source/WebKit/UIProcess/Cocoa/SafeBrowsingWarningCocoa.mm
Source/WebKit/UIProcess/Cocoa/WKSafeBrowsingWarning.h
Source/WebKit/UIProcess/Cocoa/WKSafeBrowsingWarning.mm
Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm
Source/WebKit/UIProcess/Cocoa/WebViewImpl.h
Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm
Source/WebKit/UIProcess/PageClient.h
Source/WebKit/UIProcess/SafeBrowsingWarning.h
Source/WebKit/UIProcess/WebPageProxy.cpp
Source/WebKit/UIProcess/WebPageProxy.h
Source/WebKit/UIProcess/ios/PageClientImplIOS.h
Source/WebKit/UIProcess/ios/PageClientImplIOS.mm
Source/WebKit/UIProcess/mac/PageClientImplMac.h
Source/WebKit/UIProcess/mac/PageClientImplMac.mm
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/SafeBrowsing.mm

index d7e45ef..9c59b27 100644 (file)
@@ -1,3 +1,45 @@
+2018-12-19  Alex Christensen  <achristensen@webkit.org>
+
+        Navigations away from the SafeBrowsing interstitial show a flash of old content
+        https://bugs.webkit.org/show_bug.cgi?id=192676
+
+        Reviewed by Chris Dumez.
+
+        When a user clicks through a safe browsing warning, do not remove the warning until content is drawn for the destination.
+        Otherwise, the user will confusingly see the page before the warning while the navigation happens.
+        We can only do this for warnings caused by main frame navigations, though.  Other warnings (such as those caused by iframes)
+        need to be cleared immediately, and we still need to clear the warning immediately if the user has said to go back.
+
+        This change is reflected in an updated API test.
+
+        * UIProcess/API/Cocoa/WKWebView.mm:
+        (-[WKWebView _showSafeBrowsingWarning:completionHandler:]):
+        * UIProcess/Cocoa/SafeBrowsingWarningCocoa.mm:
+        (WebKit::SafeBrowsingWarning::SafeBrowsingWarning):
+        * UIProcess/Cocoa/WKSafeBrowsingWarning.h:
+        * UIProcess/Cocoa/WKSafeBrowsingWarning.mm:
+        (-[WKSafeBrowsingWarning forMainFrameNavigation]):
+        * UIProcess/Cocoa/WebPageProxyCocoa.mm:
+        (WebKit::WebPageProxy::beginSafeBrowsingCheck):
+        * UIProcess/Cocoa/WebViewImpl.h:
+        * UIProcess/Cocoa/WebViewImpl.mm:
+        (WebKit::WebViewImpl::showSafeBrowsingWarning):
+        (WebKit::WebViewImpl::clearSafeBrowsingWarningIfForMainFrameNavigation):
+        * UIProcess/PageClient.h:
+        (WebKit::PageClient::clearSafeBrowsingWarningIfForMainFrameNavigation):
+        * UIProcess/SafeBrowsingWarning.h:
+        (WebKit::SafeBrowsingWarning::create):
+        (WebKit::SafeBrowsingWarning::forMainFrameNavigation const):
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::didReachLayoutMilestone):
+        (WebKit::WebPageProxy::beginSafeBrowsingCheck):
+        (WebKit::WebPageProxy::decidePolicyForNavigationAction):
+        * UIProcess/WebPageProxy.h:
+        * UIProcess/ios/PageClientImplIOS.h:
+        * UIProcess/mac/PageClientImplMac.h:
+        * UIProcess/mac/PageClientImplMac.mm:
+        (WebKit::PageClientImpl::clearSafeBrowsingWarningIfForMainFrameNavigation):
+
 2018-12-19  Tim Horton  <timothy_horton@apple.com>
 
         UI process crash when focusing an editable image
index 9f65f2e..30b1ca9 100644 (file)
@@ -1279,9 +1279,17 @@ static NSDictionary *dictionaryRepresentationForEditorState(const WebKit::Editor
 - (void)_showSafeBrowsingWarning:(const WebKit::SafeBrowsingWarning&)warning completionHandler:(CompletionHandler<void(Variant<WebKit::ContinueUnsafeLoad, URL>&&)>&&)completionHandler
 {
     _safeBrowsingWarning = adoptNS([[WKSafeBrowsingWarning alloc] initWithFrame:self.bounds safeBrowsingWarning:warning completionHandler:[weakSelf = WeakObjCPtr<WKWebView>(self), completionHandler = WTFMove(completionHandler)] (auto&& result) mutable {
-        if (auto strongSelf = weakSelf.get())
-            [std::exchange(strongSelf->_safeBrowsingWarning, nullptr) removeFromSuperview];
         completionHandler(WTFMove(result));
+        auto strongSelf = weakSelf.get();
+        if (!strongSelf)
+            return;
+        bool navigatesMainFrame = WTF::switchOn(result,
+            [] (WebKit::ContinueUnsafeLoad continueUnsafeLoad) { return continueUnsafeLoad == WebKit::ContinueUnsafeLoad::Yes; },
+            [] (const URL&) { return true; }
+        );
+        if (navigatesMainFrame && [strongSelf->_safeBrowsingWarning forMainFrameNavigation])
+            return;
+        [std::exchange(strongSelf->_safeBrowsingWarning, nullptr) removeFromSuperview];
     }]);
     [self addSubview:_safeBrowsingWarning.get()];
 }
@@ -1291,6 +1299,12 @@ static NSDictionary *dictionaryRepresentationForEditorState(const WebKit::Editor
     [std::exchange(_safeBrowsingWarning, nullptr) removeFromSuperview];
 }
 
+- (void)_clearSafeBrowsingWarningIfForMainFrameNavigation
+{
+    if ([_safeBrowsingWarning forMainFrameNavigation])
+        [self _clearSafeBrowsingWarning];
+}
+
 #if ENABLE(ATTACHMENT_ELEMENT)
 
 - (void)_didInsertAttachment:(API::Attachment&)attachment withSource:(NSString *)source
index 86e71fb..fd06faf 100644 (file)
@@ -182,6 +182,7 @@ struct PrintInfo;
 
 - (void)_showSafeBrowsingWarning:(const WebKit::SafeBrowsingWarning&)warning completionHandler:(CompletionHandler<void(Variant<WebKit::ContinueUnsafeLoad, URL>&&)>&&)completionHandler;
 - (void)_clearSafeBrowsingWarning;
+- (void)_clearSafeBrowsingWarningIfForMainFrameNavigation;
 
 - (std::optional<BOOL>)_resolutionForShareSheetImmediateCompletionForTesting;
 
index 0984b5b..d36c86b 100644 (file)
@@ -149,10 +149,11 @@ static NSMutableAttributedString *safeBrowsingDetailsText(const URL& url, SSBSer
     return malwareOrUnwantedSoftwareDetails(WEB_UI_NSSTRING(@"Warnings are shown for websites where harmful software has been detected. You can check %the-status-of-site% on the %safeBrowsingProvider% diagnostic page.", "Unwanted software warning description"), @"%the-status-of-site%", false);
 }
 
-SafeBrowsingWarning::SafeBrowsingWarning(const URL& url, SSBServiceLookupResult *result)
+SafeBrowsingWarning::SafeBrowsingWarning(const URL& url, bool forMainFrameNavigation, SSBServiceLookupResult *result)
     : m_url(url)
     , m_title(safeBrowsingTitleText(result))
     , m_warning(safeBrowsingWarningText(result))
+    , m_forMainFrameNavigation(forMainFrameNavigation)
     , m_details(safeBrowsingDetailsText(url, result))
 {
 }
index 2c76225..88a2be0 100644 (file)
@@ -61,4 +61,6 @@ using RectType = CGRect;
 
 - (instancetype)initWithFrame:(RectType)frame safeBrowsingWarning:(const WebKit::SafeBrowsingWarning&)warning completionHandler:(CompletionHandler<void(Variant<WebKit::ContinueUnsafeLoad, URL>&&)>&&)completionHandler;
 
+- (BOOL)forMainFrameNavigation;
+
 @end
index 68d2810..0741d51 100644 (file)
@@ -510,6 +510,11 @@ static void setBackground(ViewType *view, ColorType *color)
     _completionHandler((NSURL *)link);
 }
 
+- (BOOL)forMainFrameNavigation
+{
+    return _warning->forMainFrameNavigation();
+}
+
 @end
 
 @implementation WKSafeBrowsingTextView
index a50b5ef..83f5e8f 100644 (file)
@@ -72,14 +72,14 @@ void WebPageProxy::loadRecentSearches(const String& name, Vector<WebCore::Recent
     searchItems = WebCore::loadRecentSearches(name);
 }
 
-void WebPageProxy::beginSafeBrowsingCheck(const URL& url, WebFramePolicyListenerProxy& listener)
+void WebPageProxy::beginSafeBrowsingCheck(const URL& url, bool forMainFrameNavigation, WebFramePolicyListenerProxy& listener)
 {
 #if HAVE(SAFE_BROWSING)
     SSBLookupContext *context = [SSBLookupContext sharedLookupContext];
     if (!context)
         return listener.didReceiveSafeBrowsingResults({ });
-    [context lookUpURL:url completionHandler:makeBlockPtr([listener = makeRef(listener), url = url] (SSBLookupResult *result, NSError *error) mutable {
-        RunLoop::main().dispatch([listener = WTFMove(listener), result = retainPtr(result), error = retainPtr(error), url = WTFMove(url)] {
+    [context lookUpURL:url completionHandler:makeBlockPtr([listener = makeRef(listener), forMainFrameNavigation, url = url] (SSBLookupResult *result, NSError *error) mutable {
+        RunLoop::main().dispatch([listener = WTFMove(listener), result = retainPtr(result), error = retainPtr(error), forMainFrameNavigation, url = WTFMove(url)] {
             if (error) {
                 listener->didReceiveSafeBrowsingResults({ });
                 return;
@@ -87,7 +87,7 @@ void WebPageProxy::beginSafeBrowsingCheck(const URL& url, WebFramePolicyListener
 
             for (SSBServiceLookupResult *lookupResult in [result serviceLookupResults]) {
                 if (lookupResult.isPhishing || lookupResult.isMalware || lookupResult.isUnwantedSoftware) {
-                    listener->didReceiveSafeBrowsingResults(SafeBrowsingWarning::create(url, lookupResult));
+                    listener->didReceiveSafeBrowsingResults(SafeBrowsingWarning::create(url, forMainFrameNavigation, lookupResult));
                     return;
                 }
             }
index 01a19fd..d53ab8b 100644 (file)
@@ -229,6 +229,7 @@ public:
 
     void showSafeBrowsingWarning(const SafeBrowsingWarning&, CompletionHandler<void(Variant<ContinueUnsafeLoad, URL>&&)>&&);
     void clearSafeBrowsingWarning();
+    void clearSafeBrowsingWarningIfForMainFrameNavigation();
 
     WKLayoutMode layoutMode() const;
     void setLayoutMode(WKLayoutMode);
index 1536508..2800fff 100644 (file)
@@ -1622,9 +1622,16 @@ void WebViewImpl::showSafeBrowsingWarning(const SafeBrowsingWarning& warning, Co
         return completionHandler(ContinueUnsafeLoad::Yes);
 
     m_safeBrowsingWarning = adoptNS([[WKSafeBrowsingWarning alloc] initWithFrame:[m_view bounds] safeBrowsingWarning:warning completionHandler:[weakThis = makeWeakPtr(*this), completionHandler = WTFMove(completionHandler)] (auto&& result) mutable {
-        if (weakThis)
-            [std::exchange(weakThis->m_safeBrowsingWarning, nullptr) removeFromSuperview];
         completionHandler(WTFMove(result));
+        if (!weakThis)
+            return;
+        bool navigatesMainFrame = WTF::switchOn(result,
+            [] (ContinueUnsafeLoad continueUnsafeLoad) { return continueUnsafeLoad == ContinueUnsafeLoad::Yes; },
+            [] (const URL&) { return true; }
+        );
+        if (navigatesMainFrame && [weakThis->m_safeBrowsingWarning forMainFrameNavigation])
+            return;
+        [std::exchange(weakThis->m_safeBrowsingWarning, nullptr) removeFromSuperview];
     }]);
     [m_view addSubview:m_safeBrowsingWarning.get()];
 }
@@ -1634,6 +1641,12 @@ void WebViewImpl::clearSafeBrowsingWarning()
     [std::exchange(m_safeBrowsingWarning, nullptr) removeFromSuperview];
 }
 
+void WebViewImpl::clearSafeBrowsingWarningIfForMainFrameNavigation()
+{
+    if ([m_safeBrowsingWarning forMainFrameNavigation])
+        clearSafeBrowsingWarning();
+}
+
 bool WebViewImpl::isFocused() const
 {
     if (m_inBecomeFirstResponder)
index c5179af..26971d9 100644 (file)
@@ -218,6 +218,7 @@ public:
 
     virtual void showSafeBrowsingWarning(const SafeBrowsingWarning&, CompletionHandler<void(Variant<ContinueUnsafeLoad, URL>&&)>&& completionHandler) { completionHandler(ContinueUnsafeLoad::Yes); }
     virtual void clearSafeBrowsingWarning() { }
+    virtual void clearSafeBrowsingWarningIfForMainFrameNavigation() { }
     
 #if ENABLE(DRAG_SUPPORT)
 #if PLATFORM(GTK)
index f1908c6..d4c0f0e 100644 (file)
@@ -39,9 +39,9 @@ namespace WebKit {
 class SafeBrowsingWarning : public RefCounted<SafeBrowsingWarning> {
 public:
 #if HAVE(SAFE_BROWSING)
-    static Ref<SafeBrowsingWarning> create(const URL& url, SSBServiceLookupResult *result)
+    static Ref<SafeBrowsingWarning> create(const URL& url, bool forMainFrameNavigation, SSBServiceLookupResult *result)
     {
-        return adoptRef(*new SafeBrowsingWarning(url, result));
+        return adoptRef(*new SafeBrowsingWarning(url, forMainFrameNavigation, result));
     }
 #endif
 #if PLATFORM(COCOA)
@@ -54,6 +54,7 @@ public:
     const URL& url() const { return m_url; }
     const String& title() const { return m_title; }
     const String& warning() const { return m_warning; }
+    bool forMainFrameNavigation() const { return m_forMainFrameNavigation; }
 #if PLATFORM(COCOA)
     RetainPtr<NSAttributedString> details() const { return m_details; }
 #endif
@@ -63,7 +64,7 @@ public:
     
 private:
 #if HAVE(SAFE_BROWSING)
-    SafeBrowsingWarning(const URL&, SSBServiceLookupResult *);
+    SafeBrowsingWarning(const URL&, bool, SSBServiceLookupResult *);
 #endif
 #if PLATFORM(COCOA)
     SafeBrowsingWarning(URL&&, String&&, String&&, RetainPtr<NSAttributedString>&&);
@@ -72,6 +73,7 @@ private:
     URL m_url;
     String m_title;
     String m_warning;
+    bool m_forMainFrameNavigation { false };
 #if PLATFORM(COCOA)
     RetainPtr<NSAttributedString> m_details;
 #endif
index 23b4514..183d54f 100644 (file)
@@ -4195,6 +4195,9 @@ void WebPageProxy::didReachLayoutMilestone(OptionSet<WebCore::LayoutMilestone> l
 {
     PageClientProtector protector(pageClient());
 
+    if (layoutMilestones.contains(DidFirstVisuallyNonEmptyLayout))
+        pageClient().clearSafeBrowsingWarningIfForMainFrameNavigation();
+    
     if (m_loaderClient)
         m_loaderClient->didReachLayoutMilestone(*this, layoutMilestones);
     m_navigationClient->renderingProgressDidChange(*this, layoutMilestones);
@@ -4250,7 +4253,7 @@ void WebPageProxy::frameDidBecomeFrameSet(uint64_t frameID, bool value)
 }
 
 #if !PLATFORM(COCOA)
-void WebPageProxy::beginSafeBrowsingCheck(const URL&, WebFramePolicyListenerProxy& listener)
+void WebPageProxy::beginSafeBrowsingCheck(const URL&, bool, WebFramePolicyListenerProxy& listener)
 {
     listener.didReceiveSafeBrowsingResults({ });
 }
@@ -4367,7 +4370,7 @@ void WebPageProxy::decidePolicyForNavigationAction(WebFrameProxy& frame, WebCore
 
     }, shouldExpectSafeBrowsingResult));
     if (shouldExpectSafeBrowsingResult == ShouldExpectSafeBrowsingResult::Yes)
-        beginSafeBrowsingCheck(request.url(), listener);
+        beginSafeBrowsingCheck(request.url(), frame.isMainFrame(), listener);
 
     API::Navigation* mainFrameNavigation = frame.isMainFrame() ? navigation.get() : nullptr;
     WebFrameProxy* originatingFrame = m_process->webFrame(originatingFrameInfoData.frameID);
index 9df6390..648c6ab 100644 (file)
@@ -1494,7 +1494,7 @@ private:
     void decidePolicyForNewWindowAction(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, NavigationActionData&&, WebCore::ResourceRequest&&, const String& frameName, uint64_t listenerID, const UserData&);
     void decidePolicyForResponse(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, const WebCore::ResourceResponse&, const WebCore::ResourceRequest&, bool canShowMIMEType, uint64_t listenerID, const UserData&);
     void unableToImplementPolicy(uint64_t frameID, const WebCore::ResourceError&, const UserData&);
-    void beginSafeBrowsingCheck(const URL&, WebFramePolicyListenerProxy&);
+    void beginSafeBrowsingCheck(const URL&, bool, WebFramePolicyListenerProxy&);
 
     void willSubmitForm(uint64_t frameID, uint64_t sourceFrameID, const Vector<std::pair<String, String>>& textFieldValues, uint64_t listenerID, const UserData&);
         
index 6f14b40..ea9fc84 100644 (file)
@@ -117,6 +117,7 @@ private:
 
     void showSafeBrowsingWarning(const SafeBrowsingWarning&, CompletionHandler<void(Variant<WebKit::ContinueUnsafeLoad, URL>&&)>&&) override;
     void clearSafeBrowsingWarning() override;
+    void clearSafeBrowsingWarningIfForMainFrameNavigation() override;
 
     void enterAcceleratedCompositingMode(const LayerTreeContext&) override;
     void exitAcceleratedCompositingMode() override;
index dc0cd36..904e53e 100644 (file)
@@ -485,6 +485,11 @@ void PageClientImpl::clearSafeBrowsingWarning()
     [m_webView _clearSafeBrowsingWarning];
 }
 
+void PageClientImpl::clearSafeBrowsingWarningIfForMainFrameNavigation()
+{
+    [m_webView _clearSafeBrowsingWarningIfForMainFrameNavigation];
+}
+
 void PageClientImpl::exitAcceleratedCompositingMode()
 {
     notImplemented();
index 87b305e..491ea48 100644 (file)
@@ -106,6 +106,7 @@ private:
     void selectionDidChange() override;
     void showSafeBrowsingWarning(const SafeBrowsingWarning&, CompletionHandler<void(Variant<WebKit::ContinueUnsafeLoad, URL>&&)>&&) override;
     void clearSafeBrowsingWarning() override;
+    void clearSafeBrowsingWarningIfForMainFrameNavigation() override;
     
 #if WK_API_ENABLED
     bool showShareSheet(const WebCore::ShareDataWithParsedURL&, WTF::CompletionHandler<void(bool)>&&) override;
index 11235a1..529f8e6 100644 (file)
@@ -501,6 +501,11 @@ void PageClientImpl::clearSafeBrowsingWarning()
     m_impl->clearSafeBrowsingWarning();
 }
 
+void PageClientImpl::clearSafeBrowsingWarningIfForMainFrameNavigation()
+{
+    m_impl->clearSafeBrowsingWarningIfForMainFrameNavigation();
+}
+
 void PageClientImpl::setTextIndicator(Ref<TextIndicator> textIndicator, WebCore::TextIndicatorWindowLifetime lifetime)
 {
     m_impl->setTextIndicator(textIndicator.get(), lifetime);
index 33d46aa..59d76be 100644 (file)
@@ -1,3 +1,15 @@
+2018-12-19  Alex Christensen  <achristensen@webkit.org>
+
+        Navigations away from the SafeBrowsing interstitial show a flash of old content
+        https://bugs.webkit.org/show_bug.cgi?id=192676
+
+        Reviewed by Chris Dumez.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/SafeBrowsing.mm:
+        (safeBrowsingView):
+        (TEST):
+        (-[SafeBrowsingHelper webView:runJavaScriptAlertPanelWithMessage:initiatedByFrame:completionHandler:]): Deleted.
+
 2018-12-19  Adrian Perez de Castro  <aperez@igalia.com>
 
         [GTK][WPE] Unify TestController::platformRunUntil() and honor condition flag
index 7b4892a..9a46bc5 100644 (file)
@@ -281,24 +281,17 @@ TEST(SafeBrowsing, ShowWarningSPI)
 }
 
 static Vector<URL> urls;
-static bool done;
 
-@interface SafeBrowsingHelper : NSObject<WKNavigationDelegate, WKUIDelegate>
+@interface SafeBrowsingObserver : NSObject
 @end
 
-@implementation SafeBrowsingHelper
+@implementation SafeBrowsingObserver
 
 - (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)
@@ -307,25 +300,24 @@ TEST(SafeBrowsing, URLObservation)
 
     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 observer = adoptNS([SafeBrowsingObserver 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 addObserver:observer.get() forKeyPath:@"URL" options:NSKeyValueObservingOptionNew context:nil];
 
-        [webView loadHTMLString:@"<script>alert('loaded')</script>" baseURL:simpleURL.get()];
+        [webView loadHTMLString:@"meaningful content to be drawn" baseURL:simpleURL.get()];
         while (![webView _safeBrowsingWarning])
             TestWebKitAPI::Util::spinRunLoop();
 #if !PLATFORM(MAC)
         [[webView _safeBrowsingWarning] didMoveToWindow];
 #endif
         visitUnsafeSite([webView _safeBrowsingWarning]);
-        TestWebKitAPI::Util::run(&done);
+        EXPECT_TRUE(!![webView _safeBrowsingWarning]);
+        while ([webView _safeBrowsingWarning])
+            TestWebKitAPI::Util::spinRunLoop();
         EXPECT_FALSE(!![webView _safeBrowsingWarning]);
 
-        done = false;
         [webView evaluateJavaScript:[NSString stringWithFormat:@"window.location='%@'", simple2URL.get()] completionHandler:nil];
         while (![webView _safeBrowsingWarning])
             TestWebKitAPI::Util::spinRunLoop();
@@ -348,7 +340,7 @@ TEST(SafeBrowsing, URLObservation)
         checkURLs({ simpleURL, simple2URL });
         goBack([webView _safeBrowsingWarning]);
         checkURLs({ simpleURL, simple2URL, simpleURL });
-        [webView removeObserver:helper.get() forKeyPath:@"URL"];
+        [webView removeObserver:observer.get() forKeyPath:@"URL"];
     }
     
     urls.clear();
@@ -359,7 +351,7 @@ TEST(SafeBrowsing, URLObservation)
         visitUnsafeSite([webView _safeBrowsingWarning]);
         TestWebKitAPI::Util::spinRunLoop(5);
         checkURLs({ simpleURL, simple2URL });
-        [webView removeObserver:helper.get() forKeyPath:@"URL"];
+        [webView removeObserver:observer.get() forKeyPath:@"URL"];
     }
 }