It would be easier to recommend advice if I could see / review the code. Can you provide a link to the in-progress CL? -Darin
On Thu, Aug 20, 2009 at 9:46 PM, Drew Wilson <[email protected]> wrote: > > > On Thu, Aug 20, 2009 at 8:39 PM, Darin Fisher <[email protected]> wrote: > >> On Thu, Aug 20, 2009 at 8:37 PM, Darin Fisher <[email protected]> wrote: >> >>> On Thu, Aug 20, 2009 at 6:17 PM, Drew Wilson <[email protected]>wrote: >>> >>>> I have to admit I'm somewhat fuzzy on the motivation behind our webkit >>>> API, although I gather the plan is to eventually upstream it to WebKit, and >>>> use it as our abstraction layer instead of using the (more mutable) WebCore >>>> APIs? Or is there another motivation? >>>> I'm just curious because it seems like every non-backwards-compatible >>>> change I have to make to WebCore seems to translate to a similar change to >>>> the WebKit API (case in point, I'm currently changing parameters to >>>> MessagePort.postMessage() to take multiple ports instead of a single port >>>> and this requires changes to things like WebKit::WebChannel), so >>>> upstreaming >>>> the WebKit API wouldn't really shield us from breakage in those cases. >>>> >>>> Anyhow, I'm trying to understand the philosophy around when to use >>>> classes like WebVector (our WebKit API version of Vector). >>>> >>> >>> I try to avoid WebVector since it necessitates a copy. I'm not sure that >>> I really want to keep it in the API long term. It is a crutch to help us >>> out. On the Chromium side, use std::vector. On the WebKit side, use >>> WTF::Vector. WebVector should only be used for data exchange, and should >>> just be a temporary. >>> >> > Here's the crux of the issue. > > WebMessagePortChannel.h is defined in src/webkit/api/public. I'm assuming > we can't use std::vector here since we ultimately want to upstream this. It > seems like our only choices here are to use WTF::Vector or WebVector. > > The implementation of WebMessagePortChannel is in > src/chrome/common/webmessageportchannel_impl.cc. We can't use WTF::Vector > here (I'm assuming) since that belies the whole point of the webkit API. > > So it seems like I do need to use WebVector here. Luckily, I don't then > need to pass this data around anywhere else (it's converted to a vector of > ints and passed through IPC) so I can avoid doing any copies. > > >> >>> In some cases, visitor or iterator patterns can be better than a >>> WebVector. See WebHTTPHeaderVisitor and WebPluginListBuilder for examples. >>> >> > I really need to pass ownership of an array of data around, so I don't > think those patterns will work here. > > Speaking of which, how do we capture the idea of passing ownership of a > pointer? If this were in WebCore, I'd use WTF::OwnPtr/PassOwnPtr to signify > that I was passing off ownership of a pointer. Is there an analogous idiom > in the Chrome codebase and/or the Chrome WebKit API? > > >> >>> -Darin >>> >>> >> doh, one more thing... i'm toying with the idea of just making WebVector >> be implemented as a std::vector in our configuration, allowing still for >> other configurations where it might be implemented using a different native >> type. if i did that, then i'd be happier with WebVector because at least it >> would only require one copy... between std::vector and WTF::Vector. >> >> -darin >> >> >> >>> >>> >>>> I'm updating some of the WebKit API classes to accept a WebVector as a >>>> parameter as part of the change described above. Down in the calling code, >>>> should I use STL classes like std::vector, and then convert to WebVector >>>> only when actually calling into the WebKit API? Or should I use WebVector >>>> elsewhere in the code (like down in the glue code)? It's certainly more >>>> efficient *not* to have to convert between std::vector and WebVector if I >>>> don't have to, but that seems like a slippery slope as WebKit API classes >>>> would start spreading through the rest of the codebase. >>>> >>>> Any guidance for me? >>>> >>>> -atw >>>> >>>> >>>> >>>> >>> >> > --~--~---------~--~----~------------~-------~--~----~ Chromium Developers mailing list: [email protected] View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~----------~----~----~----~------~----~------~--~---
