Here it is: https://github.com/mmocny/incubator-cordova-android/pull/2/files
I delete IPlugin in this version, but that probably wasn't a great idea since other people may have used the symbol. Still not understanding what it's utility is though, but if we go ahead with this, I'll at *least* revert deleting IPlugin, and do that in a separate change, or more likely, just leave IPlugin alone. On Wed, Sep 26, 2012 at 2:58 PM, Andrew Grieve <agri...@chromium.org> wrote: > Aha, okay. So on iOS they happen asynchronously to the webcore thread, but > all execute in order on the UI thread, I think even moving away from having > each execute on a new thread by default would bring the behaviour closer to > iOS. > > The other big unknown to me, is the question of why the IPlugin interface > exists instead of just using the Plugin base class. Does anyone know of any > other implementations of IPlugin besides Plugin? > > I'm going to take a stab at re-writing this change to use a super-class to > have the new signature, and have the existing Plugin class extend that and > provide the shim, and will report back for everyone to review again. > Probably won't get to it today, but maybe tomorrow. > > > On Wed, Sep 26, 2012 at 2:04 PM, Simon MacDonald < > simon.macdon...@gmail.com> wrote: > >> Yeah, sorry I meant to get back to you on that. The major reason for >> switching everything to async was that iOS can only do async and this >> helped keep the code bases/API consistent. >> >> Simon Mac Donald >> http://hi.im/simonmacdonald >> >> >> On Wed, Sep 26, 2012 at 1:47 PM, Andrew Grieve <agri...@chromium.org> >> wrote: >> > Okay, so I think everyone is on the same page in terms of not breaking >> > existing plugins. >> > >> > Does anyone know what the reason was for making plugins async by >> default? >> > >> > >> > On Wed, Sep 26, 2012 at 1:38 PM, Mike Reinstein < >> reinstein.m...@gmail.com>wrote: >> > >> >> Agreed! If we can get to some kind of stability with the API exposed to >> >> plugin developers it will go a long way. >> >> >> >> >> >> >> >> On Wed, Sep 26, 2012 at 1:32 PM, Simon MacDonald >> >> <simon.macdon...@gmail.com>wrote: >> >> >> >> > Agreed. We've broken the plugins so many times that I'm more that >> sure >> >> > that 3rd party devs are sick of it. The last time we broken the >> >> > interface on Android was in 1.9.0 and then we broke it again in 2.0.0 >> >> > on the JavaScript side. I'd rather not break it again for 2.2.0. >> >> > >> >> > Also, when I say "break" I mean the code I wrote to the previous >> >> > specification will no longer compile so I need to make changes to my >> >> > plugin. Often we can get around this by adding in a shim which I >> >> > believe is the best way to go. >> >> > >> >> > Simon Mac Donald >> >> > http://hi.im/simonmacdonald >> >> > >> >> > >> >> > On Wed, Sep 26, 2012 at 3:56 AM, Brian LeRoux <b...@brian.io> wrote: >> >> > > The only concern I have is the deprecation path needs to be long >> and >> >> > > noisy---this is probably the biggest possible breaking change we >> could >> >> > > introduce to the platform. >> >> > > >> >> > > Maybe even longer than our usual 6months / but wait until 3.0 >> >> > > >> >> > > Thoughts on that? >> >> > > >> >> > > >> >> > > On Tue, Sep 25, 2012 at 4:56 PM, Andrew Grieve < >> agri...@chromium.org> >> >> > wrote: >> >> > >> Michal - Yep, good summary, that's exactly the case. >> >> > >> Simon - totally agree. I'll change what I've got to add a second >> >> > executeV2 >> >> > >> which takes in a JSONArray, and have the String-based one just >> call >> >> > that. >> >> > >> >> >> > >> The reason to need an executeV2 is threading, so I'll focus on >> that. >> >> > >> >> >> > >> My biggest gripe against the current signature, is that it >> defaults to >> >> > >> running things on a background thread. I expect most calls will be >> >> fast >> >> > >> enough to execute inline, some calls to need to run on the UI >> thread, >> >> > and >> >> > >> then only some to require doing a lot of work on a background >> thread. >> >> > >> Furthermore, those that do require a background would often >> benefit >> >> from >> >> > >> doing some param/state checking on the calling thread before >> moving to >> >> > the >> >> > >> background thread. >> >> > >> >> >> > >> I wouldn't be proposing a new signature if there was a way to >> change >> >> > >> isSync() from defaulting to false to defaulting to true, but I >> don't >> >> > think >> >> > >> that's a safe thing to change. >> >> > >> >> >> > >> On iOS, plugins execute on the calling thread and it's up to them >> to >> >> > >> dispatch background threads if they need them. >> >> > >> >> >> > >> Michal pointed out that you can't comment on a diff in github, so >> I >> >> > opened >> >> > >> a pull request with the patch to enable commenting: >> >> > >> >> >> > >> >> >> https://github.com/agrieve/incubator-cordova-android/commit/a73dffc99847b14031c1138611bb8772dc9d7b7e >> >> > >> >> >> > >> >> >> > >> >> >> > >> >> >> > >> >> >> > >> On Tue, Sep 25, 2012 at 10:51 AM, Simon MacDonald < >> >> > simon.macdon...@gmail.com >> >> > >>> wrote: >> >> > >> >> >> > >>> Here is what I was thinking on: >> >> > >>> >> >> > >>> https://issues.apache.org/jira/browse/CB-1530 >> >> > >>> >> >> > >>> In the PluginManager change the code so that is calls: >> >> > >>> >> >> > >>> plugin.execute(string, string, string); >> >> > >>> >> >> > >>> Then in the Plugin class add a new default method that does the >> >> > following: >> >> > >>> >> >> > >>> public PluginResult execute(String action, String args, String >> >> > callbackId) >> >> > >>> { >> >> > >>> return execute(action, new JSONArrary(args), callbackId); >> >> > >>> } >> >> > >>> >> >> > >>> so that all the current plugins continue to work without needing >> any >> >> > >>> changes. If someone wants to provide their own JSON parsing they >> can >> >> > >>> override the plugin.execute(string, string, string) method and >> do it >> >> > >>> themselves. >> >> > >>> >> >> > >>> Simon Mac Donald >> >> > >>> http://hi.im/simonmacdonald >> >> > >>> >> >> > >>> >> >> > >>> On Tue, Sep 25, 2012 at 10:33 AM, Michal Mocny < >> mmo...@chromium.org> >> >> > >>> wrote: >> >> > >>> > >> >> > >>> > Summarizing what I think I'm hearing: >> >> > >>> > >> >> > >>> > The current exec signature will currently: >> >> > >>> > (a) automatically parse JSON arguments, and >> >> > >>> > (b) automatically move async calls onto a background thread. >> >> > >>> > >> >> > >>> > While both of the features simplify plugin developers in most >> >> cases, >> >> > >>> > sometimes manual control is desired (ie, for the two bugs you >> link >> >> > to). >> >> > >>> > >> >> > >>> > That sounds reasonable, however, I think I'm also hearing a >> >> proposal >> >> > to >> >> > >>> > replace the existing execute signature (deprecating the current >> >> > one). If >> >> > >>> > for the majority of cases we are happy with the current >> signature, >> >> > then >> >> > >>> is >> >> > >>> > there perhaps a less intrusive solution? Or maybe we aren't >> happy >> >> > with >> >> > >>> the >> >> > >>> > current signature, and this new signature is generally more >> future >> >> > proof, >> >> > >>> > more performant, etc, giving us other reasons for changing? >> Also, >> >> > how >> >> > >>> does >> >> > >>> > this compare with other platforms? >> >> > >>> > >> >> > >>> > -Michal >> >> > >>> > >> >> > >>> > >> >> > >>> > On Mon, Sep 24, 2012 at 11:50 PM, Andrew Grieve < >> >> agri...@google.com> >> >> > >>> wrote: >> >> > >>> > >> >> > >>> > > Means to address two bugs: >> >> > >>> > > >> >> > >>> > > https://issues.apache.org/jira/browse/CB-1530 >> >> > >>> > > https://issues.apache.org/jira/browse/CB-1532 >> >> > >>> > > >> >> > >>> > > I wanted to gather some opinions from those who have been >> around >> >> > for >> >> > >>> > > longer. Here is the proposed change: >> >> > >>> > > >> https://github.com/agrieve/incubator-cordova-android/compare/ft3 >> >> > >>> > > >> >> > >>> > > My main motivation is for FileTransfer, I need to register >> the >> >> > transfer >> >> > >>> > > synchronously so that a subsequent abort() will not have a >> race >> >> > >>> condition. >> >> > >>> > > I then perform the transfer in a background thread. I *could* >> >> > implement >> >> > >>> > > this using the current signature by returning true in >> isSync() >> >> and >> >> > then >> >> > >>> > > returning a NO_RESULT result, but I think the intentions are >> >> > clearer >> >> > >>> with >> >> > >>> > > the new signature. >> >> > >>> > > >> >> > >>> >> >> > >> >> >> > >
