Bug 231692 - Move Safe Browsing knowledge into SafariSafeBrowsing framework
Summary: Move Safe Browsing knowledge into SafariSafeBrowsing framework
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Minor
Assignee: Alex Christensen
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-10-13 11:55 PDT by Eliot Hsu
Modified: 2022-02-16 10:23 PST (History)
8 users (show)

See Also:


Attachments
Patch (5.76 KB, patch)
2021-10-15 17:07 PDT, Eliot Hsu
no flags Details | Formatted Diff | Diff
Patch (5.65 KB, patch)
2022-01-10 12:09 PST, Eliot Hsu
no flags Details | Formatted Diff | Diff
Patch (5.99 KB, patch)
2022-01-10 12:18 PST, Eliot Hsu
no flags Details | Formatted Diff | Diff
Patch (5.55 KB, patch)
2022-01-10 13:33 PST, Alex Christensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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