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

Eliot Hsu
Reported 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.
Attachments
Patch (5.76 KB, patch)
2021-10-15 17:07 PDT, Eliot Hsu 		no flags
DetailsFormatted DiffDiff
Patch (5.65 KB, patch)
2022-01-10 12:09 PST, Eliot Hsu 		no flags
DetailsFormatted DiffDiff
Patch (5.99 KB, patch)
2022-01-10 12:18 PST, Eliot Hsu 		no flags
DetailsFormatted DiffDiff
Patch (5.55 KB, patch)
2022-01-10 13:33 PST, Alex Christensen 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Eliot Hsu
Comment 1 2021-10-15 17:07:27 PDT
Created attachment 441457 [details] Patch
Eliot Hsu
Comment 2 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.
Anders Carlsson
Comment 3 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.
Sam Weinig
Comment 4 2021-10-16 09:39:54 PDT
Comment on attachment 441457 [details] Patch r- since this leaks.
Alexey Proskuryakov
Comment 5 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.
Radar WebKit Bug Importer
Comment 6 2021-10-20 11:56:31 PDT
<rdar://problem/84472985>
Eliot Hsu
Comment 7 2022-01-10 12:09:02 PST
Created attachment 448783 [details] Patch
Eliot Hsu
Comment 8 2022-01-10 12:18:03 PST
Created attachment 448784 [details] Patch
Alex Christensen
Comment 9 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.
Alex Christensen
Comment 10 2022-01-10 13:33:35 PST
Created attachment 448797 [details] Patch
Alex Christensen
Comment 11 2022-01-10 13:37:23 PST
This probably needs to wait for rdar://problem/87348334 to be integrated before landing.
EWS
Comment 12 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].
Alex Christensen
Comment 13 2022-02-10 14:07:06 PST
Reverted in http://trac.webkit.org/r289573
Alex Christensen
Comment 14 2022-02-16 10:23:33 PST
r289910
Note You need to log in before you can comment on or make changes to this bug.