incubator-callback-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Simon MacDonald <simon.macdon...@gmail.com>
Subject Re: Proposal for a new Plugin.execute() signature on Android
Date Wed, 26 Sep 2012 18:04:58 GMT
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 <agrieve@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.mike@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.macdonald@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 <agrieve@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.macdonald@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 <mmocny@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 <
>> agrieve@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.
>> > >>> > >
>> > >>>
>> >
>>

Mime
View raw message