[webkit-reviews] review denied: [Bug 234439] Fix Safari-side SafeBrowsing telemetry : [Attachment 447631] Patch

[bugzilla-daemon at webkit.org]

Alex Christensen <achristensen at apple.com> has denied Eliot Hsu
<eliothsu at apple.com>'s request for review:
Bug 234439: Fix Safari-side SafeBrowsing telemetry
https://bugs.webkit.org/show_bug.cgi?id=234439

Attachment 447631: Patch

https://bugs.webkit.org/attachment.cgi?id=447631&action=review




--- Comment #3 from Alex Christensen <achristensen at apple.com> ---
Comment on attachment 447631
  --> https://bugs.webkit.org/attachment.cgi?id=447631
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=447631&action=review

> Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm:1801
>	   if (navigatesFrame && forMainFrameNavigation) {
>	       // The safe browsing warning will be hidden once the next page
is shown.
> +	       bool continuingUnsafeLoad = WTF::switchOn(result,
> +		   [] (ContinueUnsafeLoad continueUnsafeLoad) { return
continueUnsafeLoad == ContinueUnsafeLoad::Yes; },
> +		   [] (const URL&) { return false; }
> +	       );

If navigatesFrame is true, then we know that result is ContinueUnsafeLoad::Yes.
 This check seems unnecessary.

> Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm:1806
> +	       else
> +		   dictionary.set("action"_s, String("redirect to url"));

And this seems unreachable.