Mailing-List: contact callback-dev-help@incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: callback-dev@incubator.apache.org
Received-SPF: pass (athena.apache.org: domain of agrieve@google.com designates
 209.85.214.175 as permitted sender)
MIME-Version: 1.0
Sender: agrieve@google.com
In-Reply-To: 
 <CAM1AYQCTVFH+y6gN0GQNo_NrQXxkw-e-owGmAAiigEREyv1Ujg@mail.gmail.com>
References: 
 <CABiQX1U3n72OR0pa3Aa7T_05Ci8tVfS0Fm_OEJKKQKfmCmsJtQ@mail.gmail.com>
 <CAEeF2TexWCt72ODAbR9Rb-_cpByAVW7fzWwEot2t1WkXzSZdYQ@mail.gmail.com>
 <CAM1AYQCwCTT_=foAkSgLp1suw_Pv6xctx9NtMx=rwEWFc5615Q@mail.gmail.com>
 <CABiQX1XcDaHJ7w3c26vzTjMPdAaouU+WbzLqtvO_Tywte5VXCA@mail.gmail.com>
 <CADVgkyMnTQ=2vnJ4H6=Lx=W43eT+paeSUtXw5JNFZna2hTfbNQ@mail.gmail.com>
 <CAM1AYQA0cuU+hQAMATBULqX1k5SRN8XrE3Mio29Q=mVagz2bBQ@mail.gmail.com>
 <CAM-DUcNxgdRdKLtgig+jwGYcT7d1tTyP28OPetPttFmrokMwXg@mail.gmail.com>
 <CABiQX1VdQRoWsUx-UU-_v32332UFbRFYnL7MPwAh76Zc=HMWvw@mail.gmail.com>
 <CAM1AYQCTVFH+y6gN0GQNo_NrQXxkw-e-owGmAAiigEREyv1Ujg@mail.gmail.com>
From: Andrew Grieve <agrieve@chromium.org>
Date: Wed, 26 Sep 2012 14:58:13 -0400
Message-ID: 
 <CABiQX1XLYv9jSHVLQgQVXV7_HbxF0qvndF4D=F8CDZ+zRecJPg@mail.gmail.com>
Subject: Re: Proposal for a new Plugin.execute() signature on Android
To: callback-dev@incubator.apache.org
Content-Type: multipart/alternative; boundary=f46d0447868fc592f204ca9f68e4

--f46d0447868fc592f204ca9f68e4
Content-Type: text/plain; charset=ISO-8859-1

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.macdonald@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 <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.
> >> > >>> > >
> >> > >>>
> >> >
> >>
>

--f46d0447868fc592f204ca9f68e4--