Bug 214371 - Remove a few more uses of the terms black/white list
Summary: Remove a few more uses of the terms black/white list
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Darin Adler
URL:
Keywords: InRadar
Depends on:
Blocks: 213092
  Show dependency treegraph
 
Reported: 2020-07-15 12:34 PDT by Darin Adler
Modified: 2020-07-15 15:33 PDT (History)
7 users (show)

See Also:


Attachments
Patch (12.92 KB, patch)
2020-07-15 12:37 PDT, Darin Adler
no flags Details | Formatted Diff | Diff
Patch (12.50 KB, patch)
2020-07-15 13:02 PDT, Darin Adler
no flags Details | Formatted Diff | Diff
Patch (12.47 KB, patch)
2020-07-15 13:59 PDT, Darin Adler
achristensen: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Adler 2020-07-15 12:34:14 PDT
Remove a few more uses of the terms black/white list
Comment 1 Darin Adler 2020-07-15 12:37:14 PDT
Created attachment 404375 [details]
Patch
Comment 2 Alex Christensen 2020-07-15 12:55:19 PDT
Comment on attachment 404375 [details]
Patch

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

Looks good except the part that adds another file read.

> Source/WTF/wtf/URLHelpers.cpp:61
> +// Cocoa has an implementation that read this from a file in /Library or ~/Library.

reads

> Source/WTF/wtf/cocoa/NSURLExtras.mm:85
> +            // FIXME: Remove this at some point after checking that no one is depending on it.

I've tried to remove this in the past and was informed that it exists only for third parties and we have no way of checking if they still use it.  There's also an optimization for it in Foundation, which we should rename at the same time.  I think we should do that instead of checking both filenames, which will significantly adversely affect startup time of many things.
Comment 3 Darin Adler 2020-07-15 13:00:36 PDT
Comment on attachment 404375 [details]
Patch

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

>> Source/WTF/wtf/cocoa/NSURLExtras.mm:85
>> +            // FIXME: Remove this at some point after checking that no one is depending on it.
> 
> I've tried to remove this in the past and was informed that it exists only for third parties and we have no way of checking if they still use it.  There's also an optimization for it in Foundation, which we should rename at the same time.  I think we should do that instead of checking both filenames, which will significantly adversely affect startup time of many things.

Yes, I am familiar with how this was originally used. I’m the person who added this in the first place.

But I don’t know anything about this optimization in Foundation. I’ll leave this part out of the patch based on the FUD.
Comment 4 Darin Adler 2020-07-15 13:02:42 PDT
Created attachment 404378 [details]
Patch
Comment 5 Alex Christensen 2020-07-15 13:08:45 PDT
Let's just do it now.  I filed rdar://problem/65622670
Comment 6 Alex Christensen 2020-07-15 13:23:38 PDT
And rdar://problem/65623485
Comment 7 Darin Adler 2020-07-15 13:55:25 PDT
Comment on attachment 404378 [details]
Patch

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

> Source/WTF/wtf/URLHelpers.cpp:61
> +// Cocoa has an implementation that read this from a file in /Library or ~/Library.

Oops, forgot to fix "read this" to "reads this".
Comment 8 Darin Adler 2020-07-15 13:59:58 PDT
Created attachment 404391 [details]
Patch
Comment 9 Darin Adler 2020-07-15 14:33:36 PDT
Committed r264423: <https://trac.webkit.org/changeset/264423>
Comment 10 Radar WebKit Bug Importer 2020-07-15 14:34:12 PDT
<rdar://problem/65627367>
Comment 11 Alex Christensen 2020-07-15 14:45:18 PDT
Comment on attachment 404375 [details]
Patch

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

>>> Source/WTF/wtf/cocoa/NSURLExtras.mm:85
>>> +            // FIXME: Remove this at some point after checking that no one is depending on it.
>> 
>> I've tried to remove this in the past and was informed that it exists only for third parties and we have no way of checking if they still use it.  There's also an optimization for it in Foundation, which we should rename at the same time.  I think we should do that instead of checking both filenames, which will significantly adversely affect startup time of many things.
> 
> Yes, I am familiar with how this was originally used. I’m the person who added this in the first place.
> 
> But I don’t know anything about this optimization in Foundation. I’ll leave this part out of the patch based on the FUD.

I was wrong about the optimization in Foundation.  That is just an optimization for the instantiation of an NSString, not in reading the file.  That's not important to change at the same time as this.
Comment 12 Darin Adler 2020-07-15 14:49:13 PDT
I think it’s fine to either remove the feature entirely, or change the name now, or add support for both names now to lay the ground work for removing the old name later.

But I suggest we *not* use this bug to track any of those changes. This one should be left as is.
Comment 13 Alex Christensen 2020-07-15 15:33:20 PDT
I think we should remove it in a separate patch, probably after the next branch.