incubator-callback-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Brian LeRoux...@brian.io>
Subject Re: Proposal for a new Plugin.execute() signature on Android
Date Wed, 26 Sep 2012 07:56:53 GMT
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