Created attachment 347880 [details] Patch

Created attachment 347888 [details] Patch

Created attachment 347904 [details] Patch

Comment on attachment 347904 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=347904&action=review > Source/WebKit/UIProcess/WebPageProxy.cpp:4045 > + loadAlternateHTML({ reinterpret_cast<const uint8_t*>(buffer->data()), buffer->size() }, "UTF-8"_s, navigation->currentRequest().url(), navigation->currentRequest().url(), nullptr, true); It seems wrong that WebKit would show an error page here rather than leaving that up to the client to do (like we do with all other page load errors). I would expect a safe browsing failure to just be another type of error returned to the client.

Comment on attachment 347904 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=347904&action=review >> Source/WebKit/UIProcess/WebPageProxy.cpp:4045 >> + loadAlternateHTML({ reinterpret_cast<const uint8_t*>(buffer->data()), buffer->size() }, "UTF-8"_s, navigation->currentRequest().url(), navigation->currentRequest().url(), nullptr, true); > > It seems wrong that WebKit would show an error page here rather than leaving that up to the client to do (like we do with all other page load errors). I would expect a safe browsing failure to just be another type of error returned to the client. Does this introduce a situation in which, unbeknownst to the client, the WKWebView’s URL property reports a certain URL, but the view is displaying content that wasn’t loaded from that URL?

Comment on attachment 347904 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=347904&action=review >>> Source/WebKit/UIProcess/WebPageProxy.cpp:4045 >>> + loadAlternateHTML({ reinterpret_cast<const uint8_t*>(buffer->data()), buffer->size() }, "UTF-8"_s, navigation->currentRequest().url(), navigation->currentRequest().url(), nullptr, true); >> >> It seems wrong that WebKit would show an error page here rather than leaving that up to the client to do (like we do with all other page load errors). I would expect a safe browsing failure to just be another type of error returned to the client. > > Does this introduce a situation in which, unbeknownst to the client, the WKWebView’s URL property reports a certain URL, but the view is displaying content that wasn’t loaded from that URL? It does. For the record, I have also argued that this ought to be a new error type, so I'm not the person to argue against your views which I see as correct.

Comment on attachment 347904 [details] Patch The WebKit project works best when we only make changes that at least one author and one reviewer deem to be correct.

Comment on attachment 347904 [details] Patch I'm going to take a different approach.

Created attachment 349327 [details] Patch

Created attachment 349334 [details] Patch

Created attachment 349462 [details] Patch

Comment on attachment 349462 [details] Patch This new patch doesn't seem to address the concerns raised earlier in this bug.

Comment on attachment 349462 [details] Patch This patch does address the concerns from earlier in this bug. With the new delegate callback in WKUIDelegate, an app will have the ability to see and control whether a safe browsing warning is being shown. Because loading an unsafe URL in an iframe is determined to indicate that the whole page should not be shown, we cannot use WKNavigationDelegate's existing error callbacks because they are not called for iframe navigation. Because we want unmodified clients to get this safety coverage but still have the ability to not be broken, this is the design. Re-requesting review.

As noted before, the patch introduces new meaning to WKWebView’s URL property for clients that were built with an SDK in which it had a different meaning.

(In reply to Alex Christensen from comment #13) > Comment on attachment 349462 [details] > Patch > > This patch does address the concerns from earlier in this bug. > With the new delegate callback in WKUIDelegate, an app will have the ability > to see and control whether a safe browsing warning is being shown. > Because loading an unsafe URL in an iframe is determined to indicate that > the whole page should not be shown, we cannot use WKNavigationDelegate's > existing error callbacks because they are not called for iframe navigation. > Because we want unmodified clients to get this safety coverage but still > have the ability to not be broken, this is the design. Re-requesting review. I think you may have misunderstood my concern. My specific stated concern was that the design up until this point has been to require clients to implement their own error pages, the idea being that only the client could design something that matches the UI of their application. This was a deliberate choice when designing the API. As such, it seems like a better approach would be to return an error to the client and allow them to construct an error page.

> As such, it seems like a better > approach would be to return an error to the client and allow them to > construct an error page. What default behavior are you proposing for clients that don't design and implement their own error pages?

(In reply to Geoffrey Garen from comment #16) > What default behavior are you proposing for clients that don't design and > implement their own error pages? I’m no Sam Weinig, but I would propose the same behavior that clients that don’t design and implement their own “untrustworthy TLS certificate” error pages have: the navigation fails (if it’s a main-frame navigation, the navigation delegate receives an error).

I'm all for that, but I ran up against a problem: if it's an iframe navigating to an unsafe URL, there's no API on WKWebView to tell the application about the error. didFailProvisionalNavigation and didFailNavigation are only called for top level navigations. Do you think it would be better to just unexpectedly call didStartProvisionalNavigation and didFailProvisionalNavigation even when the WKFrameInfos of the decidePolicyForNavigationAction call said it was not for a main frame navigation? I could, but that would also be changing the meaning of the API.

(In reply to Alex Christensen from comment #18) > I'm all for that, but I ran up against a problem: if it's an iframe > navigating to an unsafe URL, there's no API on WKWebView to tell the > application about the error. Right, this mirrors what happens when an iframe navigates to a URL with an untrustworthy certificate. > Do you think > it would be better to just unexpectedly call didStartProvisionalNavigation > and didFailProvisionalNavigation even when the WKFrameInfos of the > decidePolicyForNavigationAction call said it was not for a main frame > navigation? This doesn’t strike me as a good design, and like you say, it’s not a binary-compatible change. There is private API for notifying the delegate of errors in subframes, which is used by Safari in iOS for the “untrustworthy certificate in subframe navigation” case. Perhaps this private API is a good starting point for public API for communicating subframe navigation errors.

(In reply to mitz from comment #17) > (In reply to Geoffrey Garen from comment #16) > > What default behavior are you proposing for clients that don't design and > > implement their own error pages? > > I’m no Sam Weinig, but I would propose the same behavior that clients that > don’t design and implement their own “untrustworthy TLS certificate” error > pages have: the navigation fails (if it’s a main-frame navigation, the > navigation delegate receives an error). OK, now that I understand this proposal, I see that it may be aesthetically pleasing and similar to existing behaviors -- maybe that's why Sam called it "better"? -- but it's also a security regression compared to the behavior in this patch (and the behavior Safari currently implements). The Safe Browsing service can detect that a website is a phishing attempt at any time -- before, during, or after the load. When the Safe Browsing service flags a subframe or subresource, that doesn't mean "this resource is phishing, but everything else is OK". Instead it means "because of this resource, we believe the whole webpage is a phishing attempt". So, responding to a flagged subframe or subresource by just neglecting to load it would leave the user exposed to the phishing attack in the main frame. Can you modify your proposal in some way to resolve this problem?

(In reply to Geoffrey Garen from comment #20) > (In reply to mitz from comment #17) > > (In reply to Geoffrey Garen from comment #16) > > > What default behavior are you proposing for clients that don't design and > > > implement their own error pages? > > > > I’m no Sam Weinig, but I would propose the same behavior that clients that > > don’t design and implement their own “untrustworthy TLS certificate” error > > pages have: the navigation fails (if it’s a main-frame navigation, the > > navigation delegate receives an error). > > OK, now that I understand this proposal, I see that it may be aesthetically > pleasing and similar to existing behaviors -- maybe that's why Sam called it > "better"? -- but it's also a security regression compared to the behavior in > this patch (and the behavior Safari currently implements). > > The Safe Browsing service can detect that a website is a phishing attempt at > any time -- before, during, or after the load. When the Safe Browsing > service flags a subframe or subresource, that doesn't mean "this resource is > phishing, but everything else is OK". Instead it means "because of this > resource, we believe the whole webpage is a phishing attempt". > > So, responding to a flagged subframe or subresource by just neglecting to > load it would leave the user exposed to the phishing attack in the main > frame. > > Can you modify your proposal in some way to resolve this problem? Since, as you explained, being unsafe-to-browse is not a characteristic of a navigation but rather a state that the web view enters, a good binary-compatible way to present this state to clients would be to mimic one of these existing error states: Web Content process hang and Web Content process termination.

> Since, as you explained, being unsafe-to-browse is not a characteristic of a > navigation but rather a state that the web view enters, a good > binary-compatible way to present this state to clients would be to mimic one > of these existing error states: Web Content process hang and Web Content > process termination. I think something like the process termination API could fit here. In that case, what user-facing behavior would you propose?

Created attachment 349793 [details] Patch

Created attachment 350060 [details] Patch

Created attachment 350071 [details] Patch

(In reply to Geoffrey Garen from comment #22) > > Since, as you explained, being unsafe-to-browse is not a characteristic of a > > navigation but rather a state that the web view enters, a good > > binary-compatible way to present this state to clients would be to mimic one > > of these existing error states: Web Content process hang and Web Content > > process termination. > > I think something like the process termination API could fit here. In that > case, what user-facing behavior would you propose? The difference between initiating a main-frame navigation to an unsafe URL and learning that the current page has become unsafe is significant enough, that they should be presented differently to existing clients. I would propose that a main-frame navigation to an unsafe URL will appear to have failed prior to being committed. The user-facing behavior depends on how the client handles navigation errors (or not) so it can range from nothing happening through the app presenting a modal alert to the app loading an HTML error page. The current page becoming unsafe, while currently can only be triggered by a main frame navigation, is not really a navigational concept. It’s conceivable that in the future it might be triggered by any sub-resource load or even by analysis of something the page is doing with the DOM or with style. Once this condition is triggered, it’s unsafe for the user to interact with the webpage. The user-facing behavior, again, will depend on whether and how the client handles -webViewWebContentProcessDidTerminate: so it can range from the view just going blank through the app presenting a modal alert, or reloading the view with or without showing banner, or reloading the view repeatedly, or navigating to an HTML error page. Whatever it is, it’s something the app and its users are should be familiar with from experiencing Web Content process crashes. I would propose that those behaviors for existing clients also apply to clients building against the future SDK, as long as they don’t bother to adopt any new WebKit API related to safe browsing. The above behaviors also inform what such new API might look like, because it will remain important for clients to distinguish between “a bad navigation could happen, or we could stop it” and “the currently committed webpage is now known to be unsafe”. The former is best presented to clients as a new error from the existing navigation delegate method. The latter is better presented via a new API similar to -webViewWebContentProcessDidTerminate:, which requires them to take specific action prior to returning if they wish to avoid the default behavior of blanking-the-web-view-as-if-the-process-has-crashed.

Created attachment 350738 [details] Patch

Comment on attachment 350738 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=350738&action=review > Source/WebKit/UIProcess/WebPageProxy.cpp:4077 > + case UnsafeContentDecision::YepItsUnsafe: YepItsUnsafe? really? > Source/WebKit/UIProcess/WebPageProxy.cpp:4085 > + case UnsafeContentDecision::IThinkItsActuallySafe: IThinkItsActuallySafe ? > Source/WebKit/UIProcess/API/APINavigationClient.h:61 > +enum class UnsafeContentDecision : uint8_t { YepItsUnsafe, IThinkItsActuallySafe }; I do not think this naming style is good or common in the project. > Source/WebKit/UIProcess/API/Cocoa/WKNavigationDelegatePrivate.h:65 > + _WKUnsafeContentDecisionYepItsUnsafe, Ditto. > Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:477 > + provisionalLoader = static_cast<WebDocumentLoader*>(m_frame->coreFrame()->loader().policyDocumentLoader()); We would not need this workaround if you moved the DocumentLoader from policy to provisional before calling didStartProvisionalLoad(), like we usually do, e.g.: setProvisionalDocumentLoader(m_policyDocumentLoader.get()); setState(FrameStateProvisional); setPolicyDocumentLoader(nullptr); m_client.dispatchDidStartProvisionalLoad(); > Source/WebKit/WebProcess/WebPage/WebPage.cpp:4142 > + documentLoader = frame->coreFrame()->loader().policyDocumentLoader(); Ditto

> YepItsUnsafe? really? No, not really. This is still in the design exploration phase, but I think we're closing in on what we want, and I'd be up for some name suggestions.

Created attachment 350808 [details] Patch

Created attachment 350813 [details] Patch

Comment on attachment 350813 [details] Patch Attachment 350813 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/9350684 New failing tests: imported/w3c/web-platform-tests/dom/nodes/Document-characterSet-normalization.html editing/selection/iframe.html

Created attachment 350826 [details] Archive of layout-test-results from ews106 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6

Created attachment 350872 [details] Patch

Here's my understanding of where we stand on this feature, and why. # Consensus items 1. WebKit should do safe browsing checks by default. There are lots of WebKit clients and it's not practical to expect each one to adopt a safe browsing API separately. 2. Clients should be able to disable WebKit's checks, or override their outcomes. Some clients may consider it more private not to perform checks. Others might want to consult the user on the outcome. 3. loadAlternateHTML is not the best way for WebKit to present the user with an interstitial choice about safe browsing. It doesn't work for POST and other one-time loads because it cancels the load, making it impossible to bypass the warning and continue. It also invites apps that modify the DOM to accidentally modify the presentation of the interstitial choice. 4. Safe browsing is a continuous evaluation. You can decide that a webpage is unsafe at any time -- even after a page finishes loading. A simple page load error would be an incomplete implementation (though it sure would be simpler!). # Open Questions 1. Do we need to offer a default UI to bypass a safe browsing warning? Unlike a TLS error or a network disconnection, a safe browsing warning is a trusted guesstimate and not a fact. We might want or need to maintain some humility about WebKit's guesstimates. # Geoff says 1. It's not so great to kill the web process. Might a client display something like a crash banner if we did? It would be surprising and confusing if a user complained about a crash, and the complaint turned out to be a successful but poorly communicated identification of a flagged webpage -- if and only if the webpage were flagged because of a subframe. One alternative is to "undo" the navigation -- by going back or forward or to about:blank, depending on whether the navigation had been a forward or a back or a new WebView. Another alternative is to fail the load in v1, and retain the option to add a default interstitial UI prior to the load failure in v2 if it proves necessary. Another option is to add a default interstitial UI in v1, using a mechanism other than loadAlternateHTML.

(In reply to Geoffrey Garen from comment #35) > 3. loadAlternateHTML is not the best way for WebKit to present the user with > an interstitial choice about safe browsing. It doesn't work for POST and > other one-time loads because it cancels the load, making it impossible to > bypass the warning and continue. It also invites apps that modify the DOM to > accidentally modify the presentation of the interstitial choice. It's not about losing the POST data, but more about losing the state of the current page. If we decide to go back to safety when doing a main frame navigation, we want to not have destroyed the state of the previous page. Even more important, if the user or client sees that a subresource load is marked as unsafe but decides to load it anyway, we want to be able to load that subresource into a page that has not lost state or been reloaded.

Created attachment 352850 [details] Patch

Created attachment 353655 [details] Patch

Known issues with this latest patch: 1. no tests yet :( 2. WKSafeBrowsingClickableTextView.intrinsicContentSize isn't quite right, but it's good enough for all sizes 3. Not implemented on iOS yet even though it builds 4. Navigating the WKWebView should clear the interstitial. 5. Minor UI tweaks are probably needed

Comment on attachment 353655 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=353655&action=review > Source/WebKit/UIProcess/PageClient.h:120 > +enum class HeedSafeBrowsingWarning : bool { No, Yes }; I don't love "Heed" > Source/WebKit/UIProcess/WebPageProxy.cpp:4139 > + if (result->needsInterstitialWarning()) { Reduce indentation a little with a `if (!..) continue`? > Source/WebKit/UIProcess/API/C/mac/WKContextPrivateMac.mm:176 > +#if PLATFORM(MAC) What was this even here for before! Does anyone use it? > Source/WebKit/UIProcess/Cocoa/WKSafeBrowsingWarning.h:61 > +- (void)frameChangedTo:(CGRect)frame; Why? Why not just set the frame in the place where this is called? > Source/WebKit/UIProcess/Cocoa/WKSafeBrowsingWarning.mm:60 > +static ColorType *redColor() You should stick all of the colors in an asset catalog with specializations for dark mode and use them by name. We have one on iOS, but you should add one that we use on both platforms. > Source/WebKit/UIProcess/Cocoa/WKSafeBrowsingWarning.mm:96 > + return WebCore::URL({ }, "http://webkit.org/"_s); What‽ Why webkit.org > Source/WebKit/UIProcess/Cocoa/WKSafeBrowsingWarning.mm:146 > + _dark = dark; Why all this passing dark state around? Just use NSAppearance like it was made for. > Source/WebKit/UIProcess/Cocoa/WKSafeBrowsingWarning.mm:178 > + [[NSColor windowBackgroundColor] set]; Ditto the question from below; this could just be a layer with background color and corner radius. Also does it want continuous curvature corners? Probably. > Source/WebKit/UIProcess/Cocoa/WKSafeBrowsingWarning.mm:304 > +} > +- (void)dealloc Lots of missing newlines > Source/WebKit/UIProcess/Cocoa/WKSafeBrowsingWarning.mm:325 > + NSAlert *alert = [[NSAlert alloc] init]; Leak? > Source/WebKit/UIProcess/Cocoa/WKSafeBrowsingWarning.mm:361 > +- (void)drawRect:(CGRect)rect > +{ > + [redColor() set]; > + [[BezierPathType bezierPathWithRect:rect] fill]; > +} Why make backing store when it could just be a layer with background color! > Source/WebKit/UIProcess/Cocoa/WKSafeBrowsingWarning.mm:365 > +- (void)frameChangedTo:(CGRect)frame > +{ > + [self setFrame:frame]; > +} This is wrong. You're setting the frame of this view to the frame of the WKWebView. It's OK in Safari and MiniBrowser because Safari/MiniBrowser put the WKWebView at the origin, but in general you want to make this view (which is installed as a child of the WKWebView, right?) cover the *bounds* of the WKWebView, not its frame. But I also think as stated above that you should just set the frame of this view in the places where currently call this (frameChangedTo:) and get rid of this.

Comment on attachment 353655 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=353655&action=review Thanks for the great review, Tim! >> Source/WebKit/UIProcess/API/C/mac/WKContextPrivateMac.mm:176 >> +#if PLATFORM(MAC) > > What was this even here for before! Does anyone use it? This was added and adopted in Safari so that one change in WebKit can enable safe browsing in WebKit and disable safe browsing in Safari at the same time so there aren't broken builds between landing two patches in two repositories. It will be removed after this project is over.

Smart!

Comment on attachment 353655 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=353655&action=review >> Source/WebKit/UIProcess/Cocoa/WKSafeBrowsingWarning.mm:178 >> + [[NSColor windowBackgroundColor] set]; > > Ditto the question from below; this could just be a layer with background color and corner radius. Also does it want continuous curvature corners? Probably. NSView doesn't have backgroundColor, just UIView.

Comment on attachment 353655 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=353655&action=review >>> Source/WebKit/UIProcess/Cocoa/WKSafeBrowsingWarning.mm:178 >>> + [[NSColor windowBackgroundColor] set]; >> >> Ditto the question from below; this could just be a layer with background color and corner radius. Also does it want continuous curvature corners? Probably. > > NSView doesn't have backgroundColor, just UIView. I said layer! Which you can make a view have by setting wantsLayer, and ten grab it via ‘layer’ and set a background color

Created attachment 353711 [details] Patch

Comment on attachment 353711 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=353711&action=review > Source/WebKit/UIProcess/SafeBrowsingResult.h:56 > + bool needsSafeBrowsingWarning() { return m_isPhishing || m_isMalware || m_isUnwantedSoftware; } Nit - needsSafeBrowsingWarning() const? > Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:3282 > + _impl->frameOrBoundsChanged(); Can we make this work using existing machinery (e.g. WebViewImpl::setFrameSize) instead? > Source/WebKit/UIProcess/Cocoa/SafeBrowsingResultCocoa.mm:36 > +// FIXME: These ought to be API calls to the SafariSafeBrowsing framework when such SPI is available. If there's a radar tracking this, I think it would be nice to reference it here. > Source/WebKit/UIProcess/Cocoa/WKSafeBrowsingWarning.mm:340 > + NSFontAttributeName:[FontType systemFontOfSize:13], > + NSForegroundColorAttributeName : foregroundColor Nit - spaces around the : seem inconsistent here. > Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm:1625 > + [m_safeBrowsingWarning setFrame:[m_view frame]]; > + [m_safeBrowsingWarning setBounds:[m_view bounds]]; 🤔

Created attachment 353715 [details] Patch

Created attachment 353740 [details] Patch

Created attachment 353776 [details] Patch

Created attachment 353780 [details] Patch

Comment on attachment 353780 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=353780&action=review > Source/WebKit/UIProcess/API/C/mac/WKContextPrivateMac.mm:177 > + return true; Should this follow the preference? > Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:3282 > + _impl->frameOrBoundsChanged(); Prettttty likely you can get rid of this and just use WebViewImpl::setFrameSize. But this is also fine I guess? Though the name is a bit of a lie. > Source/WebKit/UIProcess/Cocoa/WKSafeBrowsingWarning.mm:89 > + NSUnderlineStyleAttributeName: [NSNumber numberWithInteger:1] @(1) > Source/WebKit/UIProcess/Cocoa/WKSafeBrowsingWarning.mm:90 > + } range:NSMakeRange(0, [replaceWith length])]; More dots! replaceWith.length > Source/WebKit/UIProcess/Cocoa/WKSafeBrowsingWarning.mm:102 > + [colorNamed(@"WKSafeBrowsingWarningTitle") set]; Pretty good chance this function would look way less crazy if you just used CG, but w/e. > Source/WebKit/UIProcess/Cocoa/WKSafeBrowsingWarning.mm:257 > + if (_completionHandler) > + _completionHandler(WebKit::ContinueUnsafeLoad::Yes); Really, continue if you're being deallocated?

Created attachment 353792 [details] Patch

Created attachment 353870 [details] Patch

Comment on attachment 353870 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=353870&action=review > Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:6657 > +- (NSView *)_safeBrowsingWarningForTesting This obviously isn't going to fly on iOS > Source/WebKit/UIProcess/Cocoa/WKSafeBrowsingWarning.mm:200 > ++ (instancetype)viewWithAttributedString:(NSMutableAttributedString *)attributedString linkTarget:(WKSafeBrowsingWarning *)target; Why mutable! (Ideally there would be a mutableCopy hiding inside here, and the caller's string wouldn't get mutated) > Source/WebKit/UIProcess/Cocoa/WKSafeBrowsingWarning.mm:295 > + _completionHandler(makeUnexpected(link)); Alex says "I could use a variant instead of Expected" > Source/WebKit/UIProcess/Cocoa/WKSafeBrowsingWarning.mm:302 > + NSStackView *bottom =box.views.lastObject; Missing a space after the = > Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm:1609 > + weakThis->m_ignoresNonWheelEvents = oldIgnoresNonWheelEvents; ?? do you want to allow scrolling (but not other events) ??

Comment on attachment 353870 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=353870&action=review > Source/WebKit/UIProcess/Cocoa/WKSafeBrowsingWarning.mm:89 > + NSUnderlineStyleAttributeName: @(1) Or just @1

Created attachment 353915 [details] Patch

Created attachment 353918 [details] Patch

Created attachment 353928 [details] Patch

<rdar://problem/45842451>

This change introduced an API test failure on High Sierra and Mojave: TestWebKitAPI.ProcessSwap.LoadAfterPolicyDecision Received data during response processing, queuing it. Received data during response processing, queuing it. /Volumes/Data/slave/highsierra-release/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:485 Expected equality of these values: @"pson://www.webkit.org/main2.html" Which is: "pson://www.webkit.org/main2.html" [[webView URL] absoluteString] Which is: "pson://www.apple.com/main.html" https://build.webkit.org/builders/Apple%20High%20Sierra%20Release%20WK2%20(Tests)/builds/7662

Tests fixed in http://trac.webkit.org/r237876 Also, this is part of <rdar://problem/26892893>