[GitHub] cordova-android pull request: API for saving/restoring plugin and ...

[riknoll]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] cordova-android pull request: API for saving/restoring plugin and ...

[riknoll]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] cordova-android pull request: API for saving/restoring plugin and ...

[riknoll]

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: ,
pluginStatus: ,
result: 
}
}
```
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] cordova-android pull request: API for saving/restoring plugin and ...

[riknoll]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] cordova-android pull request: API for saving/restoring plugin and ...

[riknoll]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] cordova-android pull request: API for saving/restoring plugin and ...

[riknoll]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] cordova-android pull request: API for saving/restoring plugin and ...

[riknoll]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] cordova-android pull request: API for saving/restoring plugin and ...

[Bnaya]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] cordova-android pull request: API for saving/restoring plugin and ...

[riknoll]

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: ,
plugins: ,
pendingResult: {
pluginServiceName: ,
pluginStatus: ,
result: 
}
}
```


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 

[GitHub] cordova-android pull request: API for saving/restoring plugin and ...

[infil00p]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] cordova-android pull request: API for saving/restoring plugin and ...

[nikhilkh]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] cordova-android pull request: API for saving/restoring plugin and ...

[riknoll]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

Re: [GitHub] cordova-android pull request: API for saving/restoring plugin and ...

[Jesse]

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  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 infrastruct...@apache.org or file a JIRA ticket
> with INFRA.
> ---
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
> For additional commands, e-mail: dev-h...@cordova.apache.org
>
>

[GitHub] cordova-android pull request: API for saving/restoring plugin and ...

[infil00p]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] cordova-android pull request: API for saving/restoring plugin and ...

[imhotep]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] cordova-android pull request: API for saving/restoring plugin and ...

[riknoll]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

RE: [GitHub] cordova-android pull request: API for saving/restoring plugin and ...

[Richard Knoll]

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:purplecabb...@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=01%7c01%7cRIKNOLL%40exchange.microsoft.com%7c43887f38590b4cf68f2908d2dfe1b049%7c72f988bf86f141af91ab2d7cd011db47%7c1=S4Sqckds4SE%2f%2fngCXBWQxZkvofnRUIUgn0x9J0krBO0%3d

On Wed, Oct 28, 2015 at 1:52 PM, riknoll <g...@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
> =01%7c01%7cRIKNOLL%40exchange.microsoft.com%7c43887f38590b4cf68f2
> 908d2dfe1b049%7c72f988bf86f141af91ab2d7cd011db47%7c1=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=01%7c01%7cRIKNOLL%40e
> xchange.microsoft.com%7c43887f38590b4cf68f2908d2dfe1b049%7c72f988bf86f
> 141af91ab2d7cd011db47%7c1=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 
> infrastruct...@apache.org or file a JIRA ticket with INFRA.
> ---
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
> For additional commands, e-mail: dev-h...@cordova.apache.org
>
>

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

[GitHub] cordova-android pull request: API for saving/restoring plugin and ...

[jasongin]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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