[webkit-dev] Intent to remove the WebCore::IconDatabase (GTK needs to make a decision)

Fri Jun 16 11:25:20 PDT 2017

> On Jun 16, 2017, at 11:11 AM, Maciej Stachowiak <mjs at apple.com> wrote:
>>> This is slightly tangential, but a comment on the model: it doesn't seem like there's a way for clients to check what range of icons are available and only then choose which to load. Even though Safari may not have needed this to move over, if you wanted to do something rigorous like load the closest available size to what you need or else a scalable format, there's no way to do that without potentially loading icons you don't need. While Safari hasn't done this, it might be useful if Safari's various places that display icons are ever made more consistent and coherent.
>> 
>> This was discussed when implementing the model for Safari, which was admittedly done quickly.
>> 
>> While Safari didn't need what you suggest right now, they agreed they might need it in the future. Since the load decision request has a completion callback, and since it's a known implementation detail that they will all come in quick succession, it was accepted that the current model could work for "choosing the right icon of all choices" in a pinch.
> 
> How are you supposed to do this? Save all the completion handlers indefinitely? I can sort of see how that could work, but if the completion handler is intended to be used at a possibly later time than the shouldLoadIconWithParameters: method, it's kind of weird that it's a callback at all.

It definitely needs to be asynchronous because loading decisions can (often? always?) depend on i/o to a database.

Scenario:

1 - WebKit says “There’s a favicon of size 64x64 from URL “http://foo.com/favicon.ico <http://foo.com/favicon.ico>” - Do you want me to load it?
2 - Client saves off that decision handler and queries its database to see if it wants that icon. (Do I have this one already? Do I have it but in a smaller size? Is my icon weeks old and therefore I want a refreshed one?)
3 - After the i/o for the database operation, the client decides it does (or does not) want this icon, and calls the completion handler.

>>> I can see that there needs to be some per-icon notification, since <link> elements can be added after the fact, and also incremental parsing is a thing. So one notification that tells you all icons wouldn't cut it.
>> 
>> Right - The "just one icon" notification was the bare minimum because of this need.
>> We can definitely add a greater API surface to deliver a bulk "here's all the icons in the <head>" in addition to the individual icon notification.
>> 
>>> Another minor comment: it seems like this new API returns raw data. It seems like the native way to use this would result in running untrusted data from the network through image decoders outside the Web Process sandbox. Do we have a way to avoid that?
>> 
>> This came up while implementing it for Safari, too. In practice we didn't decode icons out-of-process before so this model was not a regression. I see value in offering this, but it's also something conscientious clients can do on their own with the raw data.
> 
> It's not that easy for an arbitrary macOS app to make their own tightly sandboxed service for this, and probably impossible on iOS. So if we made this interface into public API, there wouldn't be a way for most clients to do the right thing.

There’s a *lot* of questions to answer before we even consider making this API.

Considering nothing related to the icon database has ever been API, there wasn’t a compelling reason to answer those questions this time around.

>>> But separating the loading mechanism from the notification, plus adding a notification that the <head> section is done parsing, could allow avoidance of unnecessary loads. In addition, there could be other useful future uses of a mechanism to properly load a resource as if it was being loaded by the page. So it would be nice to decouple this from the notification of discovering an icon link.
>> 
>> That's the full-on tangent!
>> 
>> As you know, "load an arbitrary subresource in the context of the page" has come up plenty of times over the history of the WK API, so it does seem like there's value here. 
>> Offering that, however, does seem to make the "decode the icon image out of process" less of a fit with a general purpose API.
> 
> Yeah, we'd either have to provide a generic image decoding service or make a generic loading facility that's capable of vending decoded image data instead of raw data, which both seem a bit inelegant.
> 
>>> Asking these questions because if this is to be the One True Model of icon loading going forward, we should try to make sure the design is future-proof.
>> 
>> Yes, these are all great questions to ask. 
>> 
>> Everything that's been brought up so far only really touches the API surface (Do we batch together icon load decision requests? Do we give an out-of-process decoded image instead of raw data? Do we offer an API for loading arbitrary subresource?)
>> 
>> Speaking with understanding of what's implemented today and what would be necessary to make any or all of these changes, I know that the current mechanism is a solid base on which these ideas can easily be built.
> 
> OK, as long as we still have room to address these issues before it becomes locked in as public API, then I am fine with not addressing these issues for now.

👍👍

Thanks,
~Brady

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20170616/932c9435/attachment.html>