Slight reordering. > On Jun 15, 2017, at 5:27 PM, 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. > 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. Also... > 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. > 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. Thanks, Brady > > > Regards, > Maciej > > >> On Jun 15, 2017, at 5:11 PM, Brady Eidson <[email protected]> wrote: >> >> Hi all, >> >> The IconDatabase in WebCore is the source of crashes, spins, and complexity. >> Additionally it’s not flexible enough to acknowledge that there’s multiple >> types of site icons in use on the modern web, nor to adapt to the embedding >> client’s need for customization. >> >> I recently introduced the “_WKIconLoadingDelegate” model to WebKit2Cocoa. >> >> WebCore gathers all of the candidate icon URLs and asks the embedding app >> for each one whether or not it wants to load them. >> If the app says yes, the icon will be loaded as a subresource in the current >> document and the data will be handed off to the client. >> >> From Apple’s perspective: >> >> The new model is powerful and flexible enough that Safari has adopted it. >> In WebKit1, the “WebIconDatabase” class was never API and is currently >> unused. >> In WebKit2, the C-API support was never API and is currently unused. >> >> Therefore we intend to remove the current WebCore IconDatabase from the >> project soon. >> >> Starting in on this task, I of course noticed GTK’s API has exposed >> “WebKitFaviconDatabase” >> >> Is that something that’s published API and that is used? >> If not, I can get rid of it right now >> >> If so, then I need a GTK maintainer to come up with a plan soon. >> >> The icon load delegate mechanism is powerful enough to rebuild an >> IconDatabase on top of, so if GTK needs to keep this API functional they can >> do so - maintaining the actual IconDatabase code themselves up in their API >> layer. >> >> Thanks, >> ~Brady >> _______________________________________________ >> webkit-dev mailing list >> [email protected] >> https://lists.webkit.org/mailman/listinfo/webkit-dev > _______________________________________________ webkit-dev mailing list [email protected] https://lists.webkit.org/mailman/listinfo/webkit-dev
