> On Jun 15, 2017, at 8:01 PM, Brady Eidson <beid...@apple.com> wrote: > > Slight reordering. > >> On Jun 15, 2017, at 5:27 PM, Maciej Stachowiak <m...@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. > >> 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. > > 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. 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. Regards, Maciej _______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
