Thanks for doing this. Comments inline. 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... > > 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. Agree. So would we split "browser" into "tabs" and "windows" modules? > > > 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. 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. Agree. My sense is that query is pretty complicated, and so far only makes sense for history and maybe bookmarks. For the rest we can start with simpler functions. > > > 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 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. I vote for Rafael's idea. It does make batch updates a bit tricker though, if we want to provide that. > > > 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. I liked the idea of passing the full new state, and a list of previous values for properties that had changed. However, if that's hard to implement, what you described seems fine. > > > 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. Question: Do we need to send fromIndex? > > Hmm, onBookmarkOrderChanged seems cleaner to me. What's the disadvantage of going with that? > > - a > > > > --~--~---------~--~----~------------~-------~--~----~ Chromium Developers mailing list: [email protected] View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~----------~----~----~----~------~----~------~--~---
