You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by Carlos Santana <cs...@gmail.com> on 2016/01/28 22:23:20 UTC

[REVIEW] CB-10472 NullPointerException: org.apache.cordova.PluginManager.onSaveInstanceState

It's being a while since I have done Android native dev,
Can someone review the PR I sent
https://github.com/apache/cordova-android/pull/255

This hit us today at work, we have a complex launch of multiple Cordova
Activities to deal with security aspects and crosswalk.

It's only in cordova-android@5.1.0 due to the recent changes for saveState
and restoreState for plugins

It will be great to release a 5.1.1 next week with a couple of other bugs
that are already seating in master.

Re: [REVIEW] CB-10472 NullPointerException: org.apache.cordova.PluginManager.onSaveInstanceState

Posted by Joe Bowser <bo...@gmail.com>.
Do we have unit tests for that? Can we have unit tests for that?

I know we can test this because it's on SaveInstanceState, and we just have
to make sure that we don't crash.  Actually, I'm not sure how we'd test
this.

Thoughts?

On Fri, Jan 29, 2016 at 9:34 AM, Carlos Santana <cs...@gmail.com>
wrote:

> Thanks for looking into it, I will merge and look into adding a unit test
> to check that certain methods can be called .
> I think can add JUnit tests to cover the small use case.
> Anyone know what were the unit tests added with 5.1.0 to test the new
> feature about saving and restoring state for plugins?
> It would be easier for me to piggy back and add my condition in there
>
> Richard in the JIRA Issue I added the setup to replicate with minimal
> effort.
> My use case didn't destroy the other activity it was just stopped.
> The use case was not about having a fully initialize Cordova Activity being
> destroy or stopped. That works
>
> On Thu, Jan 28, 2016 at 8:01 PM Richard Knoll <ri...@microsoft.com>
> wrote:
>
> > I also think we should add tests for this. Carlos, do you have a setup
> for
> > this bug that you could share? It would be nice to have mobilespec tests
> > for save/restore in general, but that ends up being sort of tricky
> because
> > it requires enabling the "Don't Keep Activities" dev setting.
> >
> > Thanks,
> > Richard
> >
> > -----Original Message-----
> > From: Joe Bowser [mailto:bowserj@gmail.com]
> > Sent: Thursday, January 28, 2016 1:39 PM
> > To: dev <de...@cordova.apache.org>
> > Subject: Re: [REVIEW] CB-10472 NullPointerException:
> > org.apache.cordova.PluginManager.onSaveInstanceState
> >
> > LGTM, but we should have a unit test to cover this behaviour somehow.
> >
> > On Thu, Jan 28, 2016 at 2:23 PM, Carlos Santana <cs...@gmail.com>
> > wrote:
> >
> > > It's being a while since I have done Android native dev, Can someone
> > > review the PR I sent
> > > https://github.com/apache/cordova-android/pull/255
> > >
> > > This hit us today at work, we have a complex launch of multiple
> > > Cordova Activities to deal with security aspects and crosswalk.
> > >
> > > It's only in cordova-android@5.1.0 due to the recent changes for
> > > saveState and restoreState for plugins
> > >
> > > It will be great to release a 5.1.1 next week with a couple of other
> > > bugs that are already seating in master.
> > >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> > For additional commands, e-mail: dev-help@cordova.apache.org
> >
>

Re: [REVIEW] CB-10472 NullPointerException: org.apache.cordova.PluginManager.onSaveInstanceState

Posted by Carlos Santana <cs...@gmail.com>.
Thanks for looking into it, I will merge and look into adding a unit test
to check that certain methods can be called .
I think can add JUnit tests to cover the small use case.
Anyone know what were the unit tests added with 5.1.0 to test the new
feature about saving and restoring state for plugins?
It would be easier for me to piggy back and add my condition in there

Richard in the JIRA Issue I added the setup to replicate with minimal
effort.
My use case didn't destroy the other activity it was just stopped.
The use case was not about having a fully initialize Cordova Activity being
destroy or stopped. That works

On Thu, Jan 28, 2016 at 8:01 PM Richard Knoll <ri...@microsoft.com> wrote:

> I also think we should add tests for this. Carlos, do you have a setup for
> this bug that you could share? It would be nice to have mobilespec tests
> for save/restore in general, but that ends up being sort of tricky because
> it requires enabling the "Don't Keep Activities" dev setting.
>
> Thanks,
> Richard
>
> -----Original Message-----
> From: Joe Bowser [mailto:bowserj@gmail.com]
> Sent: Thursday, January 28, 2016 1:39 PM
> To: dev <de...@cordova.apache.org>
> Subject: Re: [REVIEW] CB-10472 NullPointerException:
> org.apache.cordova.PluginManager.onSaveInstanceState
>
> LGTM, but we should have a unit test to cover this behaviour somehow.
>
> On Thu, Jan 28, 2016 at 2:23 PM, Carlos Santana <cs...@gmail.com>
> wrote:
>
> > It's being a while since I have done Android native dev, Can someone
> > review the PR I sent
> > https://github.com/apache/cordova-android/pull/255
> >
> > This hit us today at work, we have a complex launch of multiple
> > Cordova Activities to deal with security aspects and crosswalk.
> >
> > It's only in cordova-android@5.1.0 due to the recent changes for
> > saveState and restoreState for plugins
> >
> > It will be great to release a 5.1.1 next week with a couple of other
> > bugs that are already seating in master.
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> For additional commands, e-mail: dev-help@cordova.apache.org
>

RE: [REVIEW] CB-10472 NullPointerException: org.apache.cordova.PluginManager.onSaveInstanceState

Posted by Richard Knoll <ri...@microsoft.com>.
I also think we should add tests for this. Carlos, do you have a setup for this bug that you could share? It would be nice to have mobilespec tests for save/restore in general, but that ends up being sort of tricky because it requires enabling the "Don't Keep Activities" dev setting.

Thanks,
Richard

-----Original Message-----
From: Joe Bowser [mailto:bowserj@gmail.com] 
Sent: Thursday, January 28, 2016 1:39 PM
To: dev <de...@cordova.apache.org>
Subject: Re: [REVIEW] CB-10472 NullPointerException: org.apache.cordova.PluginManager.onSaveInstanceState

LGTM, but we should have a unit test to cover this behaviour somehow.

On Thu, Jan 28, 2016 at 2:23 PM, Carlos Santana <cs...@gmail.com>
wrote:

> It's being a while since I have done Android native dev, Can someone 
> review the PR I sent
> https://github.com/apache/cordova-android/pull/255
>
> This hit us today at work, we have a complex launch of multiple 
> Cordova Activities to deal with security aspects and crosswalk.
>
> It's only in cordova-android@5.1.0 due to the recent changes for 
> saveState and restoreState for plugins
>
> It will be great to release a 5.1.1 next week with a couple of other 
> bugs that are already seating in master.
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org

Re: [REVIEW] CB-10472 NullPointerException: org.apache.cordova.PluginManager.onSaveInstanceState

Posted by Joe Bowser <bo...@gmail.com>.
LGTM, but we should have a unit test to cover this behaviour somehow.

On Thu, Jan 28, 2016 at 2:23 PM, Carlos Santana <cs...@gmail.com>
wrote:

> It's being a while since I have done Android native dev,
> Can someone review the PR I sent
> https://github.com/apache/cordova-android/pull/255
>
> This hit us today at work, we have a complex launch of multiple Cordova
> Activities to deal with security aspects and crosswalk.
>
> It's only in cordova-android@5.1.0 due to the recent changes for saveState
> and restoreState for plugins
>
> It will be great to release a 5.1.1 next week with a couple of other bugs
> that are already seating in master.
>