Bug 231692

Summary: Move Safe Browsing knowledge into SafariSafeBrowsing framework
Product: WebKit Reporter: Eliot Hsu <eliothsu>
Component: New BugsAssignee: Alex Christensen <achristensen>
Status: RESOLVED FIXED    
Severity: Minor CC: achristensen, andersca, benjamin, cdumez, cmarcelo, ews-watchlist, sam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Description Eliot Hsu 2021-10-13 11:55:36 PDT 
Following up on a FIXME in SafeBrowsingWarningCocoa, move knowledge of Safe Browsing's Report an Error URLs, Learn More URLs, etc. into the SafariSafeBrowsing framework.
Comment 1 Eliot Hsu 2021-10-15 17:07:27 PDT 
Created attachment 441457 [details]
Patch
Comment 2 Eliot Hsu 2021-10-15 17:11:12 PDT 
As per previous conversation, holding this patch at least until the relevant internal changes are in a build.
Comment 3 Anders Carlsson 2021-10-15 18:50:24 PDT 
Comment on attachment 441457 [details]
Patch

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

> Source/WebKit/UIProcess/Cocoa/SafeBrowsingWarningCocoa.mm:40
>  static const char* malwareDetailsBase(SSBServiceLookupResult *result)

You probably want to have this return String otherwise the memory returned by strdup will be leaked.

> Source/WebKit/UIProcess/Cocoa/SafeBrowsingWarningCocoa.mm:62
>  static const char* reportAnErrorBase(SSBServiceLookupResult *result)

You probably want to have this return String otherwise the memory returned by strdup will be leaked.
Comment 4 Sam Weinig 2021-10-16 09:39:54 PDT 
Comment on attachment 441457 [details]
Patch

r- since this leaks.
Comment 5 Alexey Proskuryakov 2021-10-16 10:37:34 PDT 
Comment on attachment 441457 [details]
Patch

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

> Source/WTF/wtf/PlatformHave.h:1061
> +#if (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 130000) || (PLATFORM(IOS_FAMILY) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 160000) || (PLATFORM(WATCHOS) && __WATCH_OS_VERSION_MIN_REQUIRED >= 90000)

I think that you want PLATFORM(IOS) here. IOS_FAMILY includes watchOS and tvOS, but the version check excludes them.
Comment 6 Radar WebKit Bug Importer 2021-10-20 11:56:31 PDT 
<rdar://problem/84472985>
Comment 7 Eliot Hsu 2022-01-10 12:09:02 PST 
Created attachment 448783 [details]
Patch
Comment 8 Eliot Hsu 2022-01-10 12:18:03 PST 
Created attachment 448784 [details]
Patch
Comment 9 Alex Christensen 2022-01-10 13:23:25 PST 
Comment on attachment 448784 [details]
Patch

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

> Source/WebKit/Platform/spi/Cocoa/SafeBrowsingSPI.h:54
> +@property (nonatomic, readonly) NSString *malwareDetailsBaseURLString NS_AVAILABLE(13_0, 16_0);

For some reason we usually omit NS_AVAILABLE in our SPI headers.
Comment 10 Alex Christensen 2022-01-10 13:33:35 PST 
Created attachment 448797 [details]
Patch
Comment 11 Alex Christensen 2022-01-10 13:37:23 PST 
This probably needs to wait for rdar://problem/87348334 to be integrated before landing.
Comment 12 EWS 2022-02-09 10:46:33 PST 
Committed r289485 (247026@main): <https://commits.webkit.org/247026@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 448797 [details].
Comment 13 Alex Christensen 2022-02-10 14:07:06 PST 
Reverted in http://trac.webkit.org/r289573
Comment 14 Alex Christensen 2022-02-16 10:23:33 PST 
r289910