You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by riknoll <gi...@git.apache.org> on 2015/10/28 20:00:49 UTC
[GitHub] cordova-android pull request: API for saving/restoring plugin and ...
GitHub user riknoll opened a pull request:
https://github.com/apache/cordova-android/pull/236
API for saving/restoring plugin and js state
Here is my proposal for dealing with Activity destruction in Android. The code is still a little rough and might need to be finalized, but I wanted to present it for discussion. This relates to CB-8917 and CB-9189.
### Background
The issue at hand is that plugins can make calls to an external Activity and, if the device is low on memory, there is a chance that the CordovaActivity will get killed in the background causing the plugin to lose its state as well as its context for the callback. Activities in Android typically handle this situation by using `onSaveInstanceState()` and `onCreate()` methods to save and restore state respectively as the Activity is created and destroyed. This solution exposes that lifecycle to plugins as well as the js, allowing them to save state and have it restored if necessary.
### Saving js state
The js is given the ability to save its state in JSON using a new method in `navigator.app` as part of the CoreAndroid API. An application can pass an object to `navigator.app.saveState()` to save state in case of Activity destruction. This state is returned to the js as part of the resume event's payload and the js application can use it to properly restore the app. The idea is that app developers can take care of state by subscribing to the pause event to save and have it returned in the corresponding resume event. Plugins are also given the opportunity to add to the JSON to convey any information that is needed for the js to resume properly (see [1] below).
### Saving plugin state
A plugin that calls out to an external Activity is given the chance to properly restore its state before handling the result of the Activity by implementing the new `onSaveInstanceState()` and `onRestoreInstanceState()` methods that are called as part of the Activity lifecycle. The plugin is given a replacement CallbackContext as part of `onRestoreInstanceState` that can accept the result and add it to the resume event payload for use in the js (again, see [1] below).
_NOTE:_ When I mention that plugins are given the opportunity to restore state, I want to clarify that this only happens for plugins that are waiting for an external Activity result (I try to emphasize this in the `onRestoreInstanceState` method's javadoc). This makes the API a little less intuitive, but otherwise we would be conflicting with the accepted behavior that plugins currently get destroyed (i.e. lose all of their state) and are selectively rebuilt whenever a new URL is loaded into the webview. If we restore state on resume, then we can end up with some awkward cases where part of the resuming involves loading a new page so the state gets lost again and so on and so forth. My thinking is that restoring the other state is better left to app developers
### Discussion
#### Benefits:
* It requires minimal updates to existing plugins (and no updates at all if the plugin doesn't use an external Activity)
* It is a general solution/pattern that plugins can follow rather than forcing them to include platform specific methods in their APIs
* It allows the js app to save/restore its state whereas previously the app would just restart with no context
#### Downsides:
* It still requires that app developers have platform specific code in their js (unless other platforms adopt this API, but I don't know if they have the corresponding need for it)
* The resume callback will only ever get received after the initial page loads (potential page flickering)
* The pending result part of the event object doesn't provide much context (so it puts more responsibility on the app developer to keep state so they know what they're getting)
In the core plugins, this is mostly relevant to the Camera plugin which previously would crash upon receiving the Activity result if the CordovaActivity had been killed by the OS while a picture was being taken/chosen. I also updated the camera plugin to use this new API and the only necessary changes to the existing plugin code were the addition of the onSaveInstanceState and onRestoreInstanceState methods. I can post that code too if there is interest in seeing what the plugin side of this looks like
[1] Anatomy of resume event object:
```
{
action: "resume",
state: <state object passed to app.saveState>,
plugins: <objects for plugins in the form {serviceName:{pluginState}, ...}>,
pendingResult: {
pluginServiceName: <plugin service name e.g. "Camera">,
pluginStatus: <description of result' status (see PluginResult.java)>,
result: <argument(s) that would have been given to the callback>
}
}
```
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/MSOpenTech/cordova-android save-state
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-android/pull/236.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #236
----
commit 2fca689f9353ddb1058b418a26818444cace79bf
Author: riknoll <ri...@microsoft.com>
Date: 2015-10-27T19:36:43Z
Added support for Android lifecycle state saving/restoring
commit 02ef016863cc6faf722149342294566564f8055e
Author: riknoll <ri...@microsoft.com>
Date: 2015-10-28T18:03:59Z
Cleaning up the resume event object api a bit
commit 760223e076783337a8442f61237b2820f7026f1d
Author: riknoll <ri...@microsoft.com>
Date: 2015-10-28T18:09:56Z
Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/cordova-android
commit 448c69555f002378f88ff06b26dc797fe2f3d5a1
Author: riknoll <ri...@microsoft.com>
Date: 2015-10-28T18:43:15Z
Removed logging code
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-android pull request: API for saving/restoring plugin and ...
Posted by riknoll <gi...@git.apache.org>.
Github user riknoll closed the pull request at:
https://github.com/apache/cordova-android/pull/236
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-android pull request: API for saving/restoring plugin and ...
Posted by nikhilkh <gi...@git.apache.org>.
Github user nikhilkh commented on the pull request:
https://github.com/apache/cordova-android/pull/236#issuecomment-152049507
@riknoll As for cordova.js changes you can follow the instructions here: https://github.com/apache/cordova-js to generate a cordova.js. We generate the file at the time of release. To make it easier to test - you can do the generation here and add a commit to update the file that is present as part of cordova-android.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-android pull request: API for saving/restoring plugin and ...
Posted by riknoll <gi...@git.apache.org>.
Github user riknoll commented on the pull request:
https://github.com/apache/cordova-android/pull/236#issuecomment-155621633
I am closing this PR and have submitted a new proposal that incorporates feedback in #239
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-android pull request: API for saving/restoring plugin and ...
Posted by riknoll <gi...@git.apache.org>.
Github user riknoll commented on the pull request:
https://github.com/apache/cordova-android/pull/236#issuecomment-151972235
Sure, I'll push the camera branch and work on a good sample app
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-android pull request: API for saving/restoring plugin and ...
Posted by Bnaya <gi...@git.apache.org>.
Github user Bnaya commented on the pull request:
https://github.com/apache/cordova-android/pull/236#issuecomment-151969621
Looks like good a solution,
I think you need to also guarantee that handlers registered before you fired the resume event will get it, as it with device ready
https://github.com/apache/cordova-js/blob/master/src/cordova.js#L137-L155
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-android pull request: API for saving/restoring plugin and ...
Posted by infil00p <gi...@git.apache.org>.
Github user infil00p commented on the pull request:
https://github.com/apache/cordova-android/pull/236#issuecomment-152024614
I turned on "Don't keep activities" on both a Moto E (Android 4.4.1) and a Samsung Galaxy S6 Edge (Android 5.1) to test this, and I'm not able to actually get the camera to come back. What device was this tested in, and is there any other variables that we're missing here?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-android pull request: API for saving/restoring plugin and ...
Posted by riknoll <gi...@git.apache.org>.
Github user riknoll commented on the pull request:
https://github.com/apache/cordova-android/pull/236#issuecomment-151959078
@infil00p @nikhilkh please review. I was also unsure of how to handle cordova.js; do we commit that for every js change? I did not update it in my pr.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-android pull request: API for saving/restoring plugin and ...
Posted by riknoll <gi...@git.apache.org>.
Github user riknoll commented on the pull request:
https://github.com/apache/cordova-android/pull/236#issuecomment-154590431
After getting some feedback from @purplecabbage on the dev list and talking this over with @jasongin, it sounds as though we should cut everything from this resume payload besides the pending result (which means we would remove `app.saveState` as well). The burden will be on the app developer to maintain their state in local storage including the context surrounding whatever plugin calls they have pending. The new resume event payload would look like this:
```
{
action: "resume",
pendingResult: {
pluginServiceName: <plugin service name e.g. "Camera">,
pluginStatus: <description of result' status (see PluginResult.java)>,
result: <the result of the call that would be passed to the callback>
}
}
```
I agree that this is a much better idea. We should make sure we communicate the need for saving state well in the Android and plugin docs so that developers are aware of this unique Android "feature".
Are people okay with the proposed approach for allowing plugins that are waiting for external results to bundle up their state on the native side? Alternatively, we can leave plugin developers to be good about maintaining their state (using something like Android preferences). This is technically already possible assuming that we fire events correctly, but a little clunky. The advantage of the approach in this PR is that it exposes the standard Android way of doing things on the native side with the Bundle save/restore API. Does that API make sense? Should we open it up to all plugins instead of just the plugin that is receiving the Activity result?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-android pull request: API for saving/restoring plugin and ...
Posted by riknoll <gi...@git.apache.org>.
Github user riknoll commented on the pull request:
https://github.com/apache/cordova-android/pull/236#issuecomment-152026095
I am using a Nexus 6 with Marshmallow. Did you update cordova.js? I didn't check it in to the PR because I was unsure of what the protocol was since it's a built file.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-android pull request: API for saving/restoring plugin and ...
Posted by riknoll <gi...@git.apache.org>.
Github user riknoll commented on the pull request:
https://github.com/apache/cordova-android/pull/236#issuecomment-151975164
Camera code is here: https://github.com/MSOpenTech/cordova-plugin-camera/tree/save-state
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-android pull request: API for saving/restoring plugin and ...
Posted by jasongin <gi...@git.apache.org>.
Github user jasongin commented on the pull request:
https://github.com/apache/cordova-android/pull/236#issuecomment-152032481
Windows apps also have to deal with saving and restoring state like this in at least some cases. See [MSDN - Guidelines for app suspend and resume](https://msdn.microsoft.com/en-us/library/windows/apps/hh465088.aspx). Apps can be terminated by the OS while suspended and then should be capable of resuming later based on saved state. However, Windows ensures that won't happen to an app that is specifically waiting on another app for a result, such as capturing a photo.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-android pull request: API for saving/restoring plugin and ...
Posted by imhotep <gi...@git.apache.org>.
Github user imhotep commented on the pull request:
https://github.com/apache/cordova-android/pull/236#issuecomment-152031010
I believe iOS also has the same issue when transitioning from UIWebView to other native views (although I don't believe app crashes) so I vote for having a common API for this feature. /cc @shazron
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-android pull request: API for saving/restoring plugin and ...
Posted by riknoll <gi...@git.apache.org>.
Github user riknoll commented on the pull request:
https://github.com/apache/cordova-android/pull/236#issuecomment-154566865
So in terms of a common API across platforms, what part are we looking at generalizing? Obviously we should ensure that each platform properly fires its pause and resume events, but is there also a need for allowing plugins to communicate state as I have done here?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
RE: [GitHub] cordova-android pull request: API for saving/restoring
plugin and ...
Posted by Richard Knoll <ri...@microsoft.com>.
That's a pretty good point. When I was working on it, I figured that since this was a platform specific issue, mimicking the platform's API (Android's onSaveInstanceState) was the way to go. Expanding CoreAndroid's secret APIs is probably worth avoiding though.
Thanks,
Richard
-----Original Message-----
From: Jesse [mailto:purplecabbage@gmail.com]
Sent: Wednesday, October 28, 2015 2:49 PM
To: dev@cordova.apache.org
Subject: Re: [GitHub] cordova-android pull request: API for saving/restoring plugin and ...
This sounds really good!
The only reservation I have is making a new non-standard way of saving state in js.
Instead of providing navigator.app.saveState() I think we should be content to simply raise the event, and allow the app developer to choose how they wish to store it. The obvious choice would be simply dumping some data in localStorage.
@purplecabbage
https://na01.safelinks.protection.outlook.com/?url=risingj.com&data=01%7c01%7cRIKNOLL%40exchange.microsoft.com%7c43887f38590b4cf68f2908d2dfe1b049%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=S4Sqckds4SE%2f%2fngCXBWQxZkvofnRUIUgn0x9J0krBO0%3d
On Wed, Oct 28, 2015 at 1:52 PM, riknoll <gi...@git.apache.org> wrote:
> Github user riknoll commented on the pull request:
>
>
> https://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fgithu
> b.com%2fapache%2fcordova-android%2fpull%2f236%23issuecomment-151986714
> &data=01%7c01%7cRIKNOLL%40exchange.microsoft.com%7c43887f38590b4cf68f2
> 908d2dfe1b049%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=pSCIkGF0LeL
> 9R224b1tsyXH83iYwszeDAT1zxlpfQsQ%3d
>
> Here's an example index.js and index.html:
> https://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fgist.
> github.com%2friknoll%2f94a40dc147040191fd3e&data=01%7c01%7cRIKNOLL%40e
> xchange.microsoft.com%7c43887f38590b4cf68f2908d2dfe1b049%7c72f988bf86f
> 141af91ab2d7cd011db47%7c1&sdata=5CXyfzbtGMcv%2fnicW7PZffrxnykSiOo4Jk5X
> wLGKcIA%3d
>
> For anyone hoping to test it out, make sure you check "Don't keep
> activities" in your phone's developer options.
>
>
> ---
> If your project is set up for it, you can reply to this email and have
> your reply appear on GitHub as well. If your project does not have
> this feature enabled and wishes so, or if the feature is enabled but
> not working, please contact infrastructure at
> infrastructure@apache.org or file a JIRA ticket with INFRA.
> ---
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> For additional commands, e-mail: dev-help@cordova.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
Re: [GitHub] cordova-android pull request: API for saving/restoring
plugin and ...
Posted by Jesse <pu...@gmail.com>.
This sounds really good!
The only reservation I have is making a new non-standard way of saving
state in js.
Instead of providing navigator.app.saveState() I think we should be content
to simply raise the event, and allow the app developer to choose how they
wish to store it. The obvious choice would be simply dumping some data in
localStorage.
@purplecabbage
risingj.com
On Wed, Oct 28, 2015 at 1:52 PM, riknoll <gi...@git.apache.org> wrote:
> Github user riknoll commented on the pull request:
>
>
> https://github.com/apache/cordova-android/pull/236#issuecomment-151986714
>
> Here's an example index.js and index.html:
> https://gist.github.com/riknoll/94a40dc147040191fd3e
>
> For anyone hoping to test it out, make sure you check "Don't keep
> activities" in your phone's developer options.
>
>
> ---
> If your project is set up for it, you can reply to this email and have your
> reply appear on GitHub as well. If your project does not have this feature
> enabled and wishes so, or if the feature is enabled but not working, please
> contact infrastructure at infrastructure@apache.org or file a JIRA ticket
> with INFRA.
> ---
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> For additional commands, e-mail: dev-help@cordova.apache.org
>
>
[GitHub] cordova-android pull request: API for saving/restoring plugin and ...
Posted by riknoll <gi...@git.apache.org>.
Github user riknoll commented on the pull request:
https://github.com/apache/cordova-android/pull/236#issuecomment-151986714
Here's an example index.js and index.html: https://gist.github.com/riknoll/94a40dc147040191fd3e
For anyone hoping to test it out, make sure you check "Don't keep activities" in your phone's developer options.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-android pull request: API for saving/restoring plugin and ...
Posted by infil00p <gi...@git.apache.org>.
Github user infil00p commented on the pull request:
https://github.com/apache/cordova-android/pull/236#issuecomment-151959575
Can we get an updated Camera plugin branch with the same branch name so that we can work off the same code? Also, an example app for this may be a good idea as well.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org