Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add not null checks to prevent running on destroyed activity #1148

Merged
merged 2 commits into from
Mar 27, 2021

Conversation

secondVISION
Copy link
Contributor

@secondVISION secondVISION commented Jan 18, 2021

Platforms affected

Android

Motivation and Context

See issue and sample MVP/Videos in : #1002
Fixes #1002

Description

Added null pointer checks around cordova.getActivity(), to check if the activity hosting Cordova is still alive.

Testing

Updated MVP (see issue #1002 ) locally with these changes (also updated gradle but is not pushed in this PR).

Checklist

  • I've run the tests to see all new and existing tests pass
  • I added automated test coverage as appropriate for this change
  • Commit is prefixed with (platform) if this change only applies to one platform (e.g. (android))
  • If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct keyword to close issues using keywords)
  • I've updated the documentation if necessary

Copy link
Contributor

@breautek breautek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your PR.

I left a few comments, but I'm open minded. Let me know what you think.

Generally speaking I think this PR is 👍 I'll try to give this a more in-depth test later.

@codecov-io
Copy link

Codecov Report

Merging #1148 (02bd205) into master (565106f) will increase coverage by 2.68%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1148      +/-   ##
==========================================
+ Coverage   69.06%   71.74%   +2.68%     
==========================================
  Files          20       21       +1     
  Lines        1833     1745      -88     
==========================================
- Hits         1266     1252      -14     
+ Misses        567      493      -74     
Impacted Files Coverage Δ
bin/templates/cordova/lib/Adb.js 100.00% <0.00%> (ø)
bin/templates/cordova/lib/run.js 100.00% <0.00%> (ø)
bin/templates/cordova/lib/retry.js 100.00% <0.00%> (ø)
bin/templates/cordova/lib/device.js 100.00% <0.00%> (ø)
bin/templates/cordova/lib/target.js 96.77% <0.00%> (ø)
bin/templates/cordova/lib/emulator.js 98.84% <0.00%> (+0.71%) ⬆️
...n/templates/cordova/lib/builders/ProjectBuilder.js 74.83% <0.00%> (+1.62%) ⬆️
bin/templates/cordova/lib/build.js 33.33% <0.00%> (+2.22%) ⬆️
bin/templates/cordova/lib/prepare.js 47.79% <0.00%> (+4.20%) ⬆️
bin/templates/cordova/lib/check_reqs.js 69.75% <0.00%> (+10.32%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 565106f...02bd205. Read the comment docs.

@breautek
Copy link
Contributor

breautek commented Jan 18, 2021

I think the test failure is a lie, I'll restart the checks.

@breautek breautek closed this Jan 18, 2021
@breautek breautek reopened this Jan 18, 2021
@secondVISION
Copy link
Contributor Author

Let me know if anything else is needed!

@erisu erisu changed the title (android) #1002: Add Null Pointer Checks to prevent Cordova from running on a destroyed activity fix: add not null checks to prevent running on destroyed activity Mar 26, 2021
@erisu erisu added this to the 9.1.0 milestone Mar 26, 2021
@erisu erisu added the bug label Mar 26, 2021
@breautek breautek requested a review from erisu March 27, 2021 13:47
Copy link
Member

@erisu erisu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM

@breautek
Copy link
Contributor

Thank you for your contribution @secondVISION

@breautek breautek merged commit 19a5feb into apache:master Mar 27, 2021
wedgberto pushed a commit to wedgberto/cordova-android that referenced this pull request May 17, 2022
…ache#1148)

* (android) apache#1002: Add Null Pointer Checks to prevent Cordova from running on a destroyed activity

* (android) Add logging statements if Cordova Activity does not exist anymore (i.e. is destroyed)

Co-authored-by: Habets Rick <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NullPointerException: 'void android.app.Activity.runOnUiThread(java.lang.Runnable)' on a null object reference
5 participants