On Wed, Apr 22, 2009 at 3:35 PM, Aaron Boodman <[email protected]> wrote:
> > A few of us extension guys were talking offline Monday about how now > that we've all had a chance to implement some APIs, we are probably > better positioned to come to consensus on the remaining style points. > > I went through the existing three browser APIs (windows, tabs, and > bookmarks -- > http://dev.chromium.org/developers/design-documents/extensions) > and tried to identify all the places where they differ. I've also put > my vote for whether and how they should be aligned. > > Let me know what you think... (Sorry for the slow response) Naming: > - We have chromium.windows.getWindows(), > chromium.windows.createWindow(), etc and chromium.bookmarks.get(), > chromium.bookmarks.create(). > - My vote: I like chromium.bookmarks.create(). The downside to this is > that we can never put more than one primary object in a namespace, but > I'm OK with that. We should also change the event names to be like > onMove instead of onTabMoved. love it Querying: > - We have getWindows(WindowQuery), getTabs(TabQuery), and > bookmarks.get(ids)+bookmarks.search(text). > - My vote: It doesn't look to me like there is a general solution > here. Searching for windows and tabs using multiple criterion > (WindowQuery, TabQuery) feels pretty hard to use to me. Doing so in > history feels more natural. Doing so for bookmarks is kinda in > between. Agree. I don't think this should be a generalized concept. Some APIs will have it naturally. > I feel like for these we might want to do things that are > more optimized for the module in question. For example > windows.getTop(), windows.getCurrent(), bookmarks.getTree(), etc. I'll > start separate conversations for these, though. I personally like these specific APIs. They're much more clear. > Updating: > - We have bookmarks.setTitle(id, title) and windows.updateWindow({id, > top, left, width, height, ...}) > - My vote. I like the single update method for objects that naturally > have lots of fields you can update. For bookmarks, for example, you > will eventually be able to update both url and title. It seems natural > to me to be able to do both at the same time. It also lends itself > well to batch operations, should we ever want to do that. I like how this approach feels, and I think we should do it. That said, I have a few concerns that I think we need to keep an eye on. What if there are some properties that are heavier weight and potentially have more specific notification and error conditions with you try to change them. The side effect of grouping changes to multiple fields with different update semantics is that you'll get a weird mishmash. For example, let's say you're updating three properties in tab, and two of them could actually fail according to the underlying browser API for some reason. This means that updates might have partially succeeded (I doubt we want to try to implement transactions), and you might have multiple independent error messages. > I also > really like Rafael's idea to separate out the id parameter, so it > would look like windows.updateWindow(id, {top, left, width, height, > ...}). This elegantly solves the problem of accidentally using an old > object and clobbering more recent state. If we do this, probably we > should also split out id in the move apis, to be consistent. Would the API be: {id, window: {top, left, ...} }? Or would they be separate params as you have it above? I think I like the separate params in this case, but it does lead to some odd work when we support batching (you have to construct two separate arrays with matching indices). Update Event: > - Sometimes we pass old and new objects according to the docs, but I > think it will be difficult to actually do that. > - My vote: onUpdate(int id, {properties and values that changed}). The > second param would be an object containing the names of the properties > that changed and their new values. This feels reasonable to me. Move Event: > - We have onBookmarkOrderChanged(int folderid, int[] newchildids) and > onTabMoved(int tabid, int windowid, int fromindex, int toindex) > - My vote: I think we should go with onTabMoved(int tabid, int > newindex). In other places we have assumed that the client is keeping > state, so this should be the same. We'd still send only one move event > and assume the client either understands the implied shuffling, or > will refetch the entire list. I agree with this in principle, but this is one of those places where we'll likely have to reflect the capabilities of the underlying API. For example, BookmarkObserver has two different notifications: BookmarkNodeMoved - this is following what you're proposing (it doesn't call for each node that was moved as a side effect, just the original) BookmarkNodeChildrenReordered - this is called if there was a bulk resort of the children of a node, once for the parent, not for each node So in this case, if we expect the client to be able to keep itself up to date, then we'll need both events. > Question: Do we need to send fromIndex? 'need' is a strong word. Does it hurt to do so? I could imagine that it's useful for maintaining state to minimize the amount of re-shuffling you need to do. Incidentally, doesn't this notification also need a fromWindow? Erik --~--~---------~--~----~------------~-------~--~----~ Chromium Developers mailing list: [email protected] View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~----------~----~----~----~------~----~------~--~---
