incubator-callback-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joe Bowser <bows...@gmail.com>
Subject Re: Wiping plugins on navigation
Date Thu, 06 Sep 2012 18:55:20 GMT
I personally would prefer a state reset on the plugins rather than
destroying and recreating the plugin. The main reason for this is
because I know that the lack of constructors is not because there
aren't initialization steps, but is because it's a pain to do anything
in a constructor on an Android plugin given the current architecture.
This is from a while ago, so I might be totally out to lunch since I
recently got back, but I remember running into issues when I tried to
do some basic initialization of a crypto library in a constructor and
having to move it to another method that's called when we first used
the plugin.  I'm sure there are many plugins with this first run
loading pattern that would be hurt if we did the destroy and re-create
thing.

Seriously, if you look at a plugin, how much logic is in various
methods that should have been done in the constructor?  If you see a
lot, ask why it's there and not the constructor.

On Thu, Sep 6, 2012 at 11:35 AM, Anis KADRI <anis.kadri@gmail.com> wrote:
> I am also in favor of destroying plugins on page refreshes (not recreating
> them). If anything, just to comply with the web app architecture which is
> supposed to be somehow stateless.
> This problem also applies to javascript-based platforms like Bada. Because
> even though our implementation is only a javascript shim, the vendor's
> implementation is also using some sort of a bridge [1].
> The current mobile-spec doesn't work on Bada. The app just segfaults with a
> big core dump, which a I didn't think was possible. I am guessing it is
> because they're not doing any cleanup either, expecting people to develop
> single page apps. Not much we can do other than implementing our own bridge
> using [1] and not [2]
>
> [1]
> http://developer.bada.com/help_2.0/index.jsp?topic=/com.osp.cppappprogramming.help/html/tutorials/web_tutorial/using_javascriptbridge.htm
> [2]
> http://developer.bada.com/help_2.0/index.jsp?topic=/com.osp.webapireference.help/symbols/index/main.html
>
> On Thu, Sep 6, 2012 at 9:57 AM, Andrew Grieve <agrieve@chromium.org> wrote:
>
>> On Thu, Sep 6, 2012 at 11:41 AM, Braden Shepherdson <braden@chromium.org
>> >wrote:
>>
>> > I disagree with the latency fears for regenerating plugins. I've just
>> > sampled twenty plugins for Android and a handful for iOS (I don't know
>> > Objective-C, so I can't reliably say much about these), and the large
>> > majority of Android plugins don't define constructors at all. Those that
>> do
>> > are usually empty. I only saw one that had actual code, and it was simply
>> > storing the result of a .getInstance() call on some other class into a
>> > member variable. Similarly, I didn't see any member variables with
>> > expensive initializers.
>> >
>> > Therefore I don't think we need to worry about the cost of destroying and
>> > recreating a couple dozen classes with (nearly) empty constructors.
>> >
>> > As to Jimmy Jarvis' suggestion on the bug, I don't know if we can do IDs
>> > that persist across refreshes. We could make callback IDs include the
>> name
>> > of the plugin, but each plugin over its lifetime may make many requests
>> and
>> > have multiple outstanding requests at a time. That requires a count, and
>> we
>> > can't persist that count across page reloads since it and the map of
>> > callbacks reside in Javascript. We could use a hash function so that
>> > callback IDs are only very unlikely to collide, but that seems less than
>> > ideal.
>> >
>> Callback IDs do already include plugin names.
>> I think his suggestion was to use a count, but just start the count at a
>> random number instead of at zero.
>>
>>
>> >
>> > One advantage of the destroy-and-regenerate approach to plugins is that
>> > they cannot get themselves into a bad state on navigation, because they
>> > will always be starting fresh. As noted elsewhere, on purely
>> > Javascript-powered platforms, is it even possible for plugins to persist
>> > across navigation without playing games like using persistent storage?
>> >
>>
>> Thinking about this more, I think it probably doesn't even help things much
>> to recreate the plugin objects since that won't prevent old plugins from
>> running. Old plugins can still send JS to be executed (unless we null out
>> their webview reference maybe?), and if they have some background task
>> ongoing, it will continue to run and be annoying / drain battery.
>>
>> I think really the only good option is to make sure reset() (or dispose()
>> or whatever) is implemented on them when it needs to be. Beyond that, it
>> would be helpful to detect when plugins misbehave, so maybe nulling out
>> their webView reference and logging an error if they try to call
>> sendJavascript is the answer to this part.
>>
>>
>>
>> >
>> >
>> > On Thu, Sep 6, 2012 at 11:27 AM, Braden Shepherdson <braden@chromium.org
>> > >wrote:
>> >
>> > > Repeating here a comment on the bug:
>> > >
>> > > Jimmy Jarvis<
>> > https://issues.apache.org/jira/secure/ViewProfile.jspa?name=jiminyjarvis
>> >
>> > added
>> > > a comment - 05/Sep/12 23:02 - edited
>> > >
>> > > JSON-P uses a unique callback identifier, similar to the proposed fix
>> > > above, and is far more reliable and easier to debug than reset with
>> > > repeating IDs. It would be unfortunate overhead to have to reinitialize
>> > our
>> > > native logic every time a page reloads. The app would become far less
>> > > responsive, at least for us. If a plugin is a bunch of stubs, it's not
>> an
>> > > big deal as you say – however, if it is a longer running setup process
>> > ----
>> > > I'd prefer ignoring unmatched callbacks. I agree sending the plugin a
>> > Reset
>> > > or Terminate message would enable the plugin authors an opportunity to
>> do
>> > > proper cleanup on pending requests, but even if they do not, unique
>> ID's
>> > > (like JSON-P) are a better solution than repeating IDs.
>> > >
>> > >
>> > > On Wed, Sep 5, 2012 at 9:33 PM, Andrew Grieve <agrieve@chromium.org
>> > >wrote:
>> > >
>> > >> Plugins could probably use static fields if they wanted to maintain
>> some
>> > >> state between page changes. I think it should be extremely rare to
do
>> so
>> > >> though. e.g. For the platforms that implement their plugins in JS,
is
>> it
>> > >> even possible to maintain state between page changes?
>> > >>
>> > >> One nice outcome of this is that it will make our mobile-spec pages
>> less
>> > >> likely to affect each other.
>> > >>
>> > >>
>> > >> On Wed, Sep 5, 2012 at 6:59 PM, Brian LeRoux <b@brian.io> wrote:
>> > >>
>> > >> > Yes I do not refuting Cordova needs to work for both single page
and
>> > >> > multipage apps. Just saying there is a solution to this problem.
;P
>> > >> >
>> > >> >
>> > >> > On Wed, Sep 5, 2012 at 3:49 PM, Jesse <purplecabbage@gmail.com>
>> > wrote:
>> > >> > > Whether it is an edge case, or a common case, multi-page
apps are
>> a
>> > >> > > reality, so we definitely need to notify plugins when the
page is
>> > >> > > changing.
>> > >> > > I don't necessarily agree that the plugin should be destroyed
and
>> > >> > > recreated though, I can think of several cases where persistence
>> > would
>> > >> > > be nice to have.
>> > >> > >
>> > >> > > I also do not see this as a security issue.  Security is
already
>> > >> > > governed by the white-list, so non-trusted pages cannot access
>> > device
>> > >> > > functions.  If a plugin needs additional security, then it
should
>> be
>> > >> > > built into the plugin, and not the responsibility of the
>> framework.
>> > >> > > ... Thinking of a SuperCookie plugin which uses the domain
of the
>> > >> > > currently loaded page before deciding what to return, or
something
>> > >> > > similar.
>> > >> > >
>> > >> > > My 2 sense,
>> > >> > >   Jesse
>> > >> > >
>> > >> > >
>> > >> > > On Wed, Sep 5, 2012 at 3:30 PM, Bryce Curtis <
>> > curtis.bryce@gmail.com>
>> > >> > wrote:
>> > >> > >> Sometimes multi-page apps are needed or you navigate
from your
>> app
>> > to
>> > >> > >> another page.  One bug we ran into was that callback
ids are
>> reused
>> > >> > >> when loading a new page.  So, a plugin trying to send
data back
>> to
>> > >> the
>> > >> > >> original page could be calling a recycled plugin with
erroneous
>> > data.
>> > >> > >> In addition to the bugs, there is also a security issue
with a
>> > >> > >> subsequent page being able to access a plugin that was
used in a
>> > >> > >> previous page.
>> > >> > >>
>> > >> > >> The app/page lifecycle events are propagated to the plugins,
and
>> > the
>> > >> > >> plugins are destroyed when loading a new page.  However,
looking
>> at
>> > >> > >> the code, it appears this may be broken now.  (At least
for
>> > Android).
>> > >> > >>
>> > >> > >> On Wed, Sep 5, 2012 at 3:40 PM, Braden Shepherdson <
>> > >> braden@chromium.org>
>> > >> > wrote:
>> > >> > >>> Sure, and I'm a fan of single-page apps (I do work
for Google,
>> > after
>> > >> > >>> all...), but this causes very chaotic, hard-to-track
bugs, so it
>> > >> makes
>> > >> > >>> sense to be robust over a refresh/navigation.
>> > >> > >>>
>> > >> > >>>
>> > >> > >>> On Wed, Sep 5, 2012 at 5:25 PM, Brian LeRoux <b@brian.io>
>> wrote:
>> > >> > >>>
>> > >> > >>>> One thing to note, we tend to advise ppl author
single page web
>> > >> apps
>> > >> > >>>> which makes state visibility change an app logic
concern (and
>> > avoid
>> > >> > >>>> this issue from manifesting). Generally, we can
say a page
>> > refresh
>> > >> is
>> > >> > >>>> not a great user experience in apps.
>> > >> > >>>>
>> > >> > >>>> On Wed, Sep 5, 2012 at 1:15 PM, Braden Shepherdson
<
>> > >> > braden@chromium.org>
>> > >> > >>>> wrote:
>> > >> > >>>> > This is intended as a continuation of the
discussion started
>> in
>> > >> > >>>> > https://issues.apache.org/jira/browse/CB-1318
.
>> > >> > >>>> >
>> > >> > >>>> > The bug in question is one where one page
starts a long
>> native
>> > >> side
>> > >> > >>>> action
>> > >> > >>>> > such as a network call. Then the user navigates
the app to
>> > >> another
>> > >> > page.
>> > >> > >>>> > When the long action completes, the call
returns and the
>> > >> appropriate
>> > >> > >>>> > Javascript callback is looked up and called.
>> > >> > >>>> >
>> > >> > >>>> > However when the page is navigated, the
counter that provides
>> > >> > supposedly
>> > >> > >>>> > unique names for callbacks is reset, allowing
a callback on
>> the
>> > >> new
>> > >> > page
>> > >> > >>>> to
>> > >> > >>>> > have the same name as the callback from
the old page. It then
>> > >> gets
>> > >> > called
>> > >> > >>>> > incorrectly, potentially introducing weird
and transient
>> bugs.
>> > >> > >>>> >
>> > >> > >>>> > The proposed solution is to do the following
on navigation:
>> > >> > >>>> > - Call a destroy() call on all plugins,
which by default does
>> > >> > nothing.
>> > >> > >>>> This
>> > >> > >>>> > allows the plugins a chance to cancel any
outstanding network
>> > >> > requests or
>> > >> > >>>> > do any other cleanup work.
>> > >> > >>>> > - Delete the plugin instance and recreate
it.
>> > >> > >>>> >
>> > >> > >>>> > In the bug I also said one step would be
to wipe the callback
>> > >> table
>> > >> > in
>> > >> > >>>> the
>> > >> > >>>> > Javascript, but that isn't necessary since
it would have been
>> > >> wiped
>> > >> > by
>> > >> > >>>> the
>> > >> > >>>> > navigation anyway.
>> > >> > >>>> >
>> > >> > >>>> > This issue is cross-platform-ish. It (probably)
doesn't apply
>> > to
>> > >> > >>>> web-based
>> > >> > >>>> > platforms like WebOS or Bada, because the
plugins are
>> > Javascript
>> > >> > shims
>> > >> > >>>> > rather than native code, and are wiped on
navigation like any
>> > >> other
>> > >> > >>>> > Javascript. However this issue does exist
on at least Android
>> > and
>> > >> > iOS,
>> > >> > >>>> and
>> > >> > >>>> > probably a few others as well.
>> > >> > >>>> >
>> > >> > >>>> > I'm proposing to implement the solution
outlined above on
>> > Android
>> > >> > and
>> > >> > >>>> iOS.
>> > >> > >>>> > I don't have the devices or environment
to do any other
>> > >> platforms,
>> > >> > nor
>> > >> > >>>> am I
>> > >> > >>>> > sure which are necessary. The maintainers
of other platforms
>> > will
>> > >> > have to
>> > >> > >>>> > consider this problem for their platform.
I would also update
>> > the
>> > >> > core
>> > >> > >>>> > plugins to define a destroy() method if
they have relevant
>> > >> cleanups
>> > >> > to
>> > >> > >>>> make.
>> > >> > >>>> >
>> > >> > >>>> > Thoughts on the approach, things I'm missing?
>> > >> > >>>> >
>> > >> > >>>> > Braden
>> > >> > >>>>
>> > >> > >
>> > >> > >
>> > >> > >
>> > >> > > --
>> > >> > > @purplecabbage
>> > >> > > risingj.com
>> > >> >
>> > >>
>> > >
>> > >
>> >
>>

Mime
View raw message