On Fri, Aug 14, 2009 at 11:32 PM, Aaron Boodman<[email protected]> wrote: > On Fri, Aug 14, 2009 at 5:16 PM, Rafael Weinstein<[email protected]> wrote: >> Currently an extension writer has no way to discover when an async extension >> function fails (an error currently goes to console.error). >> http://code.google.com/p/chromium/issues/detail?id=17381 >> Current Plan of Action >> We discussed this earlier and decided on the path of changing the format of >> callback arguments from a list of arguments to a result object that has >> 'success' property, which if true, means the object also has properties for >> the resulting parameters. >> i.e. rather than >> chrome.tabs.create({}, function(tab) { >> // do something with tab >> }); >> this: >> chrome.tabs.create({...}, function(result) { >> if (result.success) { >> // do something with result.tab >> } >> }); >> I talked with Aaron about this today and we observed that it has the >> following problems (apart from being a breaking change to the current api): >> 1) It's a switch from ordered callback parameters to named callback >> parameters. As such it requires changes all the way to each >> ExtensionFunction which must now be bound to name of each callback param. >> I.e. It would be a pretty dramatic change in terms of number of files >> touched. >> 2) Just looking at the dispatching/marshalling mechanism that extensions use >> and chrome os, and others are considering reusing, it's a somewhat less >> general solution (returning a single value, as opposed to a list of values). >> 3) It makes the most common use case (where the caller doesn't care to >> handle the error) more a bit more verbose. >> Proposal >> We talked it through and came up with the following alternative (and a twist >> on it): Rather than return a result object with named parameters attached, >> just have the first parameter of each callback be a result object that >> always contains a 'success' property and may contain an errorMessage if >> success == false. i.e. >> chrome.tabs.create({...}, function(result, tab) { >> if (result.success) { >> // do something with tab >> } >> }); >> or if the caller doesn't want to handle the error just access |tab| without >> checking |result|. >> It's still a breaking change, but it could be implemented entirely in >> extensions_process_bindings.js and extension_api.js with minimal changes, >> and leaves the current dispatching/marshalling mechanism in tact. >> Twist >> One (somewhat windows-y) way to accomplish what we want without breaking the >> current api would be to set chrome.extension.lastError for the lifetime of >> the callback if an error has occured. i.e. >> chrome.tabs.create({...}, function(result, tab) { >> if (chrome.extension.lastError) { >> // do something with tab >> } >> }); >> Which accomplishes fixing the bug, with none of the downsides listed with >> the current plan of action, but isn't very javascripty. >> Opinions? > > It's hard for me to argue with the twist. It's better in every > dimension I can imagine: > > * It is correct -- it's easy for us to ensure that the correct error > is populated while a callback is running > * The common case of not caring about errors is still optimal > * The rare case of caring about errors is still relatively simple > * It's compatible with the current interfaces > * It's an easy modification from the current code > > That said it just smells bad. Can anyone come up with a real negative? > Otherwise, I am inclined to suggest we go with it as it meets our > needs with the least fuss.
I like the idea in general. As you say, it has some good properties. For negatives, are there any situations where it chrome.extension.lastError could become invalid during the lifetime of the callback? Random possibilities: - You make another extension API call - shouldn't cause a problem since the callback for the new call won't happen during the lifetime of this callback - You do something that can cause JS reentrancy (call out to a plugin, call out to the DOM) - I don't think this could cause another callback to be fired during this callstack, but perhaps if they run a nested modal loop? - You use one of our (few) synchronous API calls - perhaps these never set lastError since they don't use callbacks? - Could another RenderView in the same process get a callback while another RenderView is blocked? - I don't think so since they all execute on the same main thread, but again, perhaps there's some way when you call out to a plugin that this can happen. Any others? I can't say for sure whether the ones I mentioned would cause problems (my gut says no). Matt probably knows for sure. Erik --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Chromium-extensions" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/chromium-extensions?hl=en -~----------~----~----~----~------~----~------~--~---
