> On Jun 16, 2017, at 11:11 AM, Maciej Stachowiak <[email protected]> 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
_______________________________________________ webkit-dev mailing list [email protected] https://lists.webkit.org/mailman/listinfo/webkit-dev
